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

[grimar]

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

[ruiu]

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]

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]

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]

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]

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]

Ok.

[grimar]

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]

Why?

[ruiu]

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

ELF/ScriptLexer.cpp

171 ↗

(On Diff #110345)

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]

• Rebased.

ELF/ScriptLexer.cpp

171 ↗

(On Diff #110345)

It was already out of sync, fixed in rL315442

[ruiu]

LGTM

lld/trunk/ELF/ScriptLexer.cpp

 }
 
 // An erroneous token is handled as if it were the last token before EOF.
 bool ScriptLexer::atEOF() { return ErrorCount || Tokens.size() == Pos; }
 
 // Split a given string as an expression.
 // This function returns "3", "*" and "5" for "3*5" for example.
 static std::vector<StringRef> tokenizeExpr(StringRef S) {
-  StringRef Ops = "+-*/:!"; // List of operators
+  StringRef Ops = "+-*/:!~"; // List of operators
 
   // Quoted strings are literal strings, so we don't want to split it.
   if (S.startswith("\""))
     return {S};
 
   // Split S with operators as separators.
   std::vector<StringRef> Ret;
   while (!S.empty()) {

lld/trunk/test/ELF/linkerscript/symbol-assignexpr.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 
 # RUN: echo "SECTIONS { \
 # RUN:         symbol = CONSTANT(MAXPAGESIZE); \
 # RUN:         symbol2 = symbol + 0x1234; \
 # RUN:         symbol3 = symbol2; \
 # RUN:         symbol4 = symbol + -4; \
-# RUN:         symbol5 = symbol - ~ 0xfffb; \
+# RUN:         symbol5 = symbol - ~0xfffb; \
 # RUN:         symbol6 = symbol - ~(0xfff0 + 0xb); \
 # RUN:         symbol7 = symbol - ~ 0xfffb + 4; \
 # RUN:         symbol8 = ~ 0xffff + 4; \
 # RUN:         symbol9 = - 4; \
 # RUN:         symbol10 = 0xfedcba9876543210; \
 # RUN:         symbol11 = ((0x28000 + 0x1fff) & ~(0x1000 + -1)); \
 # RUN:         symbol12 = 0x1234; \
 # RUN:         symbol12 += 1; \