Handle numbers followed by ":" in linker scripts

[dmikulin]

Summary

This is a fix for Bugzilla 31813.
The problem is that the tokenizer does not create a separate token for ":" unless there's white space before it. Changed it to always create a token for ":" and reworked some logic that relied on ":" being attached to some tokens like "global:" and "local:".

[ruiu]

If there's a bare token containing ":", this patch regress, but I believe that is unlikely. LGTM.

[davide]

LGTM

[dmikulin]

Committed as r294006

ELF/LinkerScript.cpp

unsigned ScriptParser::readPhdrType() {
   }
   return Ret;
 }
 
 // Reads a list of symbols, e.g. "{ global: foo; bar; local: *; };".
 void ScriptParser::readAnonymousDeclaration() {
   // Read global symbols first. "global:" is default, so if there's
   // no label, we assume global symbols.
-  if (consume("global:") || peek() != "local:")
+  if (peek() != "local") {
+    if (consume("global"))
+      expect(":");
     Config->VersionScriptGlobals = readSymbols();
-
+  }
   readLocals();
   expect("}");
   expect(";");
 }
 
 void ScriptParser::readLocals() {
-  if (!consume("local:"))
+  if (!consume("local"))
     return;
+  expect(":");
   std::vector<SymbolVersion> Locals = readSymbols();
   for (SymbolVersion V : Locals) {
     if (V.Name == "*") {
       Config->DefaultSymbolVersion = VER_NDX_LOCAL;
       continue;
     }
     Config->VersionScriptLocals.push_back(V);
   }
 }
 
 // Reads a list of symbols, e.g. "VerStr { global: foo; bar; local: *; };".
 void ScriptParser::readVersionDeclaration(StringRef VerStr) {
   // Identifiers start at 2 because 0 and 1 are reserved
   // for VER_NDX_LOCAL and VER_NDX_GLOBAL constants.
   uint16_t VersionId = Config->VersionDefinitions.size() + 2;
   Config->VersionDefinitions.push_back({VerStr, VersionId});
 
   // Read global symbols.
-  if (consume("global:") || peek() != "local:")
+  if (peek() != "local") {
+    if (consume("global"))
+      expect(":");
     Config->VersionDefinitions.back().Globals = readSymbols();
-
+  }
   readLocals();
   expect("}");
 
   // Each version may have a parent version. For example, "Ver2"
   // defined as "Ver2 { global: foo; local: *; } Ver1;" has "Ver1"
   // as a parent. This version hierarchy is, probably against your
   // instinct, purely for hint; the runtime doesn't care about it
   // at all. In LLD, we simply ignore it.
   if (peek() != ";")
     skip();
   expect(";");
 }
 
 // Reads a list of symbols for a versions cript.
 std::vector<SymbolVersion> ScriptParser::readSymbols() {
   std::vector<SymbolVersion> Ret;
   for (;;) {
     if (consume("extern")) {
       for (SymbolVersion V : readVersionExtern())
         Ret.push_back(V);
       continue;
     }
 
-    if (peek() == "}" || peek() == "local:" || Error)
+    if (peek() == "}" || peek() == "local" || Error)
       break;
     StringRef Tok = next();
     Ret.push_back({unquote(Tok), false, hasWildcard(Tok)});
     expect(";");
   }
   return Ret;
 }
 

ELF/ScriptParser.cpp

if (S.startswith("\"")) {
       S = S.substr(E + 1);
       continue;
     }
 
     // Unquoted token. This is more relaxed than tokens in C-like language,
     // so that you can write "file-name.cpp" as one bare token, for example.
     size_t Pos = S.find_first_not_of(
         "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
-        "0123456789_.$/\\~=+[]*?-:!<>^");
+        "0123456789_.$/\\~=+[]*?-!<>^");
 
     // A character that cannot start a word (which is usually a
     // punctuation) forms a single character token.
     if (Pos == 0)
       Pos = 1;
     Vec.push_back(S.substr(0, Pos));
     S = S.substr(Pos);
   }

test/ELF/linkerscript/numbers.s

 
 # RUN: echo "SECTIONS { \
 # RUN:  . = 0x11m; \
 # RUN: }" > %t4.script
 # RUN: not ld.lld %t --script %t4.script -o %t5 2>&1 | \
 # RUN:  FileCheck --check-prefix=ERR3 %s
 # ERR3: malformed number: 0x11m
 
+## Make sure that numbers can be followed by a ":" with and without a space,
+## e.g. "0x100 :" or "0x100:"
+# RUN: echo "SECTIONS { \
+# RUN:  .hex1 0x400 : { *(.hex.1) } \
+# RUN:  .hex2 0x500:{ *(.hex.2) } \
+# RUN: }" > %t5.script
+# RUN: ld.lld %t --script %t5.script -o %t6
+# RUN: llvm-objdump -section-headers %t6 | FileCheck -check-prefix=SECADDR %s
+# SECADDR:     Sections:
+# SECADDR-NEXT: Idx Name          Size      Address
+# SECADDR-NEXT:   0               00000000 0000000000000000
+# SECADDR-NEXT:   1 .hex1         00000008 0000000000000400
+# SECADDR-NEXT:   2 .hex2         00000008 0000000000000500
+
 .globl _start
 _start:
 nop
 
 .section .hex.1, "a"
 .quad 0
 
 .section .kilo.1, "a"

test/ELF/version-script.s

 # REQUIRES: x86
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/shared.s -o %t2.o
 # RUN: ld.lld -shared %t2.o -soname shared -o %t2.so
 
 # RUN: echo "{ global: foo1; foo3; local: *; };" > %t.script
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 # RUN: ld.lld --version-script %t.script -shared %t.o %t2.so -o %t.so
 # RUN: llvm-readobj -dyn-symbols %t.so | FileCheck --check-prefix=DSO %s
 
 # RUN: echo "# comment" > %t3.script
 # RUN: echo "{ local: *; # comment" >> %t3.script
 # RUN: echo -n "}; # comment" >> %t3.script
 # RUN: ld.lld --version-script %t3.script -shared %t.o %t2.so -o %t3.so
 # RUN: llvm-readobj -dyn-symbols %t3.so | FileCheck --check-prefix=DSO2 %s
 
-# RUN: echo "VERSION_1.0 { global: foo1; local: *; };" > %t4.script
+## Also check that both "global:" and "global :" forms are accepted
+# RUN: echo "VERSION_1.0 { global : foo1; local : *; };" > %t4.script
 # RUN: echo "VERSION_2.0 { global: foo3; local: *; };" >> %t4.script
 # RUN: ld.lld --version-script %t4.script -shared %t.o %t2.so -o %t4.so
 # RUN: llvm-readobj -dyn-symbols %t4.so | FileCheck --check-prefix=VERDSO %s
 
 # RUN: echo "VERSION_1.0 { global: foo1; local: *; };" > %t5.script
 # RUN: echo "{ global: foo3; local: *; };" >> %t5.script
 # RUN: not ld.lld --version-script %t5.script -shared %t.o %t2.so -o %t5.so 2>&1 | \
 # RUN:   FileCheck -check-prefix=ERR1 %s