Page MenuHomePhabricator

[ELF] - Eliminate AssertCommand.
ClosedPublic

Authored by grimar on Apr 9 2018, 4:27 AM.

Details

Summary

Currently, LLD supports ASSERT as a separate command.

We support two forms now.

  1. Assign expression-view: . = ASSERT(0x100)

(old GNU ld required it and some scripts in the wild are still using
something like . = ASSERT((_end - _text <= (512 * 1024 * 1024)), "kernel image bigger than KERNEL_IMAGE_SIZE");

  1. Nowadays above is not a mandatory form and command-like form is commonly used:

ASSERT(<expr>, "text);

The return value of the ASSERT is Dot. That was implemented in D30171.
It looks like (2) is just a short version of (1) then.

GNU ld does *not* list ASSERT as a SECTIONS command:
https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS

Given above I think we can change ASSERT to be an assignment to Dot.
That makes the rest of the code much simpler.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Apr 9 2018, 4:27 AM
ruiu added inline comments.Apr 9 2018, 11:16 AM
ELF/ScriptParser.cpp
764 ↗(On Diff #141614)

This change might be okay, but this function name has to be changed.

espindola accepted this revision.Apr 9 2018, 4:17 PM

I am OK with it once a name is agreed on. I would also be OK with readProvideOrAssignment since ASSERT can be viewed as a odd form of assignment.

ELF/ScriptParser.cpp
765 ↗(On Diff #141614)

Add a comment saying that the Assert expression returns ., so this is just ". = .".

This revision is now accepted and ready to land.Apr 9 2018, 4:17 PM
grimar updated this revision to Diff 141815.Apr 10 2018, 3:09 AM
grimar marked 2 inline comments as done.
  • Addressed review comments.
ELF/ScriptParser.cpp
764 ↗(On Diff #141614)

What about readAssignment? Since PROVIDE, HIDDEN and ASSERT are an a variants of assignment command.

grimar added a comment.EditedApr 23 2018, 7:08 AM

Ping. Rui?

LGTM

It turns out there was one readAssignment already. Not sure how that happened when I posted the patch :/
I'll rename the original readAssignment to readSymbolAssignment as readAssignment introduced in this patch is a wider thing.

This revision was automatically updated to reflect the committed changes.