Page MenuHomePhabricator

[ELF] - Linkerscript: Add `~` as separate math token.
ClosedPublic

Authored by grimar on Aug 9 2017, 3:59 AM.

Details

Summary

Previously we did not support following:
foo = ~0xFF;
and had to add space before numeric value:
foo = ~ 0xFF

That was constistent with ld.bfd < 2.30, which shows:
script.txt:3: undefined symbol `~2' referenced in expression,
but inconsistent with gold.

It was fixed for ld.bfd 2.30 as well:
https://sourceware.org/bugzilla/show_bug.cgi?id=22267

Diff Detail

Event Timeline

grimar created this revision.Aug 9 2017, 3:59 AM
ruiu edited edge metadata.Aug 9 2017, 5:10 AM

bfd's behavior seems more conservative than gold, and if GNU linkers are inconsistent, I'd make lld compatible with conservative one, so I wouldn't make this change.

grimar added a comment.Aug 9 2017, 5:13 AM
In D36508#836495, @ruiu wrote:

bfd's behavior seems more conservative than gold, and if GNU linkers are inconsistent, I'd make lld compatible with conservative one, so I wouldn't make this change.

I see, though it looks more like a bug of bfd. It accepts !foo but does not like ~foo for probably no reason.

ruiu added a comment.Aug 9 2017, 5:18 AM

Well, I wouldn't say that that is a bug of bfd, as there's no specification on which we can say whether something is a bug or not.

grimar added a comment.EditedAug 9 2017, 5:27 AM
In D36508#836506, @ruiu wrote:

Well, I wouldn't say that that is a bug of bfd, as there's no specification on which we can say whether something is a bug or not.

Ok, not a bug but unexpected and confusing behavior probably.
btw, It is safe to make it user friendly I think, because scripts used with bfd will not contain ~a since bfd fails to parse them.
And gold's script expect behavior supported by patch.

ruiu added a comment.Aug 9 2017, 6:39 AM

You actually don't want to mimic the gold's behavior as they probably accidentally relaxed the grammar. With that, it is now easy to write linker scripts that are only by consumed by gold. They introduced an unnecessary compatibility. We don't want to do the same thing.

grimar abandoned this revision.Aug 9 2017, 6:54 AM

Ok.

grimar reclaimed this revision.Oct 9 2017, 6:02 AM
grimar added a comment.EditedOct 9 2017, 6:05 AM

ld.bfd behavior was fixed and going to be changed in upcoming 2.30 version:
https://sourceware.org/bugzilla/show_bug.cgi?id=22267

So I am resurrecting this review and suggest to land this again.

ruiu added a comment.Oct 9 2017, 11:55 AM

Ah, sorry, I missed the first line of your last comment.

ELF/ScriptLexer.cpp
167

I'd think you should spend a little more time on each patch. Apparently, you need to update the following comment that assumes +-*/ is a list of operators.

grimar updated this revision to Diff 118556.Oct 11 2017, 1:24 AM
grimar edited the summary of this revision. (Show Details)
  • Rebased.
ELF/ScriptLexer.cpp
167

It was already out of sync, fixed in rL315442

ruiu accepted this revision.Oct 11 2017, 11:48 AM

LGTM

This revision is now accepted and ready to land.Oct 11 2017, 11:48 AM
This revision was automatically updated to reflect the committed changes.