Fix handling of "{ global : local; local: *; };" in version scripts

[dmikulin]

Summary

This patch is related to https://reviews.llvm.org/D29434, an attempt to fix bug 31813.
Currently the following case is not handled correctly:
{ global : local; local: *; };
Added look-ahead one more token to tell a variable named 'local' from a keyword 'local:'.

[davide]

ELF/LinkerScript.cpp

1999–2001

This patch looks alright, but it substantially diverges from what we did here.
In particular, we look ahead 2 tokens instead of one.
This is Rui's parser, so if he has no problems with that, I'm fine with it.

ELF/ScriptParser.cpp

161

Ditto.

ELF/ScriptParser.h

31

why size_t and not unsigned ?

[• rafael]

I think this is fine. We do have to improve the lexer, but it is not clear if we want the lexer to return a single token for "local :".

Rui, what do you think?

[dmikulin]

ELF/ScriptParser.h

31

Why indeed...

[ruiu]

It is unfortunate that we change the grammar to LL(2) by this patch, but that's not necessarily a bad thing. We need to redesign the lexer as we had discussed in the linking Linux kernel thread, but until then, we need to improve this lexer.

ELF/ScriptParser.cpp

161–167

Capitalize the variables.

I think it is more natural to use a for loop here.

for (size_t I = 0; I <= N; ++I) {
  Tok = next();
  if (Error)
    return "";
}
Pos = Pos - N - 1;

(Note that ++I is more C++-ish than I++.)

ELF/ScriptParser.h

31

n -> N

[dmikulin]

Updated patch

[davide]

LGTM now.

[dmikulin]

Committed r294343.

ELF/LinkerScript.cpp

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" && peek(1) == ":") || Error)
       break;
     StringRef Tok = next();
     Ret.push_back({unquote(Tok), false, hasWildcard(Tok)});
     expect(";");
   }
   return Ret;
 }
 
 // Reads an "extern C++" directive, e.g.,
 // "extern "C++" { ns::*; "f(int, double)"; };"

ELF/ScriptParser.h

 public:
   explicit ScriptParserBase(MemoryBufferRef MB);
 
   void setError(const Twine &Msg);
   void tokenize(MemoryBufferRef MB);
   static StringRef skipSpace(StringRef S);
   bool atEOF();
   StringRef next();
-  StringRef peek();
+  StringRef peek(unsigned N = 0);
   void skip();
   bool consume(StringRef Tok);
   void expect(StringRef Expect);
   std::string getCurrentLocation();
 
   std::vector<MemoryBufferRef> MBs;
   std::vector<StringRef> Tokens;
   size_t Pos = 0;

ELF/ScriptParser.cpp

if (Error)
     return "";
   if (atEOF()) {
     setError("unexpected EOF");
     return "";
   }
   return Tokens[Pos++];
 }
 
-StringRef ScriptParserBase::peek() {
-  StringRef Tok = next();
+StringRef ScriptParserBase::peek(unsigned N) {
+  StringRef Tok;
+  for (unsigned I = 0; I <= N; ++I) {
+    Tok = next();
     if (Error)
       return "";
-  --Pos;
+  }
+  Pos = Pos - N - 1;
   return Tok;
 }
 
 bool ScriptParserBase::consume(StringRef Tok) {
   if (peek() == Tok) {
     skip();
     return true;
   }
   return false;

test/ELF/version-script-glob.s

 # REQUIRES: x86
 
 # RUN: echo "{ global: foo*; bar*; local: *; };" > %t.script
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: ld.lld -shared --version-script %t.script %t.o -o %t.so
 # RUN: llvm-readobj -dyn-symbols %t.so | FileCheck %s
 
         .globl foo1
 foo1:
 
         .globl bar1
 bar1:
 
         .globl zed1
 zed1:
 
+        .globl local
+local:
+
 # CHECK:      DynamicSymbols [
 # CHECK-NEXT:   Symbol {
 # CHECK-NEXT:     Name:
 # CHECK-NEXT:     Value: 0x0
 # CHECK-NEXT:     Size: 0
 # CHECK-NEXT:     Binding: Local
 # CHECK-NEXT:     Type: None
 # CHECK-NEXT:     Other: 0

 # CHECK-NEXT:     Value: 0x1000
 # CHECK-NEXT:     Size: 0
 # CHECK-NEXT:     Binding: Global
 # CHECK-NEXT:     Type: None
 # CHECK-NEXT:     Other: 0
 # CHECK-NEXT:     Section: .text
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
+
+# RUN: echo "{ global : local; local: *; };" > %t1.script
+# RUN: ld.lld -shared --version-script %t1.script %t.o -o %t1.so
+
+# LOCAL:      DynamicSymbols [
+# LOCAL-NEXT:   Symbol {
+# LOCAL-NEXT:     Name:
+# LOCAL-NEXT:     Value: 0x0
+# LOCAL-NEXT:     Size: 0
+# LOCAL-NEXT:     Binding: Local
+# LOCAL-NEXT:     Type: None
+# LOCAL-NEXT:     Other: 0
+# LOCAL-NEXT:     Section: Undefined
+# LOCAL-NEXT:   }
+# LOCAL-NEXT:   Symbol {
+# LOCAL-NEXT:     Name: local
+# LOCAL-NEXT:     Value: 0x1000
+# LOCAL-NEXT:     Size: 0
+# LOCAL-NEXT:     Binding: Global
+# LOCAL-NEXT:     Type: None
+# LOCAL-NEXT:     Other: 0
+# LOCAL-NEXT:     Section: .text
+# LOCAL-NEXT:   }
+# LOCAL-NEXT: ]