[lld] Make ELFLinkingContext own LinkerScript buffers

[rafaelauler]

Summary

Currently, no one owns script::Parser buffers, but yet ELFLinkingContext gets
updated with StringRef pointers to data inside Parser buffers. Since this buffer
is locally owned inside GnuLdDriver::evalLinkerScript(), as soon as this
function finishes, all pointers in ELFLinkingContext that comes from linker
scripts get invalid. The problem is that we need someone to own linker scripts
data structures and, since ELFLinkingContext transports references to linker
scripts data, we can simply make it also own all linker scripts data.

[rafaelauler]

forgot to subscribe to llvm-commits. opened a new one in D7323

[shankarke]

Adding a test might give more information on the functionality that you are trying to add too.

include/lld/ReaderWriter/ELFLinkingContext.h

311

Why do you recognize the master linker script from the rest ? The linker can use more than one -T option to specify more than one linker script too.

Input files that are linker scripts are positional too.

For example :

ld 1.o -lc

Where libc.so is a linker script that needs to be handled in command line order.

[rafaelauler]

Hi Shankar,

Yes, but positional linker scripts are different because they do not
override the default linker script and are typically used just to add a
symbol assignment, a new search path or INPUT/GROUP commands (according to
https://sourceware.org/binutils/docs/ld/Implicit-Linker-Scripts.html#Implicit-Linker-Scripts
).

Is it possible to specify more than one -T? I didn't know. I was expecting
that a single script would override the default linker script. If there can
be multiple -T, I need to change this implementation and just add a bool to
tell "overides default linker script?".

[• shankar.easwaran]

All linker scripts with -T are concatenated. I am not totally certain on what gnu tries to solve here.

[ruiu]

I agree that probably in the future we need to add a linker script to the linking context, so that later passes can interpret the linker scripts for their needs. Maybe it's not bad to do that.

But still the way that this patch is trying to do doesn't look good to me. We don't have to distinguish "master" and "implicit" linker scripts as far as I know. The signature of the addLinkerScriptInstance function looks a bit odd -- it always returns a given object to the caller, although the caller is of course knows what is passed to the function. Can we just add one function to the linking context, which takes a linker script and returns void, to transfer the ownership of a linker script?

Can we also eliminate ScriptInstance class? That's a wrapper for Lexer and Parser. If we modify Parser to own a lexer, we can directly use Parser instead of ScriptInstance, no?

[rafaelauler]

I agree, we don't need a master linker script, Shankar was right in that we
can have multiple linker scripts with the -T option. I'll work on your
suggestions, they sound good to me.

include/lld/ReaderWriter/ELFLinkingContext.h

 //===- lld/ReaderWriter/ELFLinkingContext.h -------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_READER_WRITER_ELF_LINKER_CONTEXT_H
 #define LLD_READER_WRITER_ELF_LINKER_CONTEXT_H
 
+#include "LinkerScript.h"
 #include "lld/Core/LinkingContext.h"
 #include "lld/Core/Pass.h"
 #include "lld/Core/PassManager.h"
 #include "lld/Core/STDExtras.h"
 #include "lld/Core/range.h"
 #include "lld/Core/Reader.h"
 #include "lld/Core/Writer.h"
 #include "llvm/ADT/StringSet.h"
public:
   std::string demangle(StringRef symbolName) const override;
   bool demangleSymbols() const { return _demangle; }
   void setDemangleSymbols(bool d) { _demangle = d; }
 
   /// \brief Align segments.
   bool alignSegments() const { return _alignSegments; }
   void setAlignSegments(bool align) { _alignSegments = align; }
 
+  // We can parse several "implicit" linker scripts, which are all input files
+  // that are not recognized as objects or archives. These are stored in the
+  // ELFLinkingContext with addLinkerScriptInstance(). However, only one
+  // "master" linker script can override the default linker script, specified
+  // with -T in the command line and, here, by calling
+  // addMasterLinkerScript().
+  script::ScriptInstance &
+  addLinkerScriptInstance(std::unique_ptr<script::ScriptInstance> script) {
+    _scripts.push_back(std::move(script));
+    return *_scripts.back().get();
+  }
+  script::ScriptInstance &
+  addMasterLinkerScript(std::unique_ptr<script::ScriptInstance> script) {
+    _scripts.push_back(std::move(script));
+    _masterLinkerScript = _scripts.back().get();
+    return *_masterLinkerScript;
+  }
 private:
   ELFLinkingContext() LLVM_DELETED_FUNCTION;
 
 protected:
   ELFLinkingContext(llvm::Triple, std::unique_ptr<TargetHandlerBase>);
 
   Writer &writer() const override;
 
protected:
   StringRef _initFunction;
   StringRef _finiFunction;
   StringRef _sysrootPath;
   StringRef _soname;
   StringRefVector _rpathList;
   StringRefVector _rpathLinkList;
   std::map<std::string, uint64_t> _absoluteSymbols;
   llvm::StringSet<> _dynamicallyExportedSymbols;
+  std::vector<std::unique_ptr<script::ScriptInstance>> _scripts;
+  script::ScriptInstance *_masterLinkerScript;
 };
 } // end namespace lld
 
 #endif

include/lld/ReaderWriter/LinkerScript.h

 /// parser/recursive descent implementation.
 ///
 /// Based on the linker script documentation available at
 /// https://sourceware.org/binutils/docs/ld/Scripts.html
 class Parser {
 public:
   explicit Parser(Lexer &lex) : _lex(lex), _peekAvailable(false) {}
 
+  /// Let's not allow copying of Parser class because it would be expensive
+  /// to update all the AST pointers to a new buffer.
+  Parser(const Parser &instance) LLVM_DELETED_FUNCTION;
+
   LinkerScript *parse();
 
 private:
   /// Advances to the next token, either asking the Lexer to lex the next token
   /// or obtaining it from the look ahead buffer.
   void consumeToken() {
     // First check if the look ahead buffer cached the next token
     if (_peekAvailable) {
private:
 
   // Current token being analyzed
   Token _tok;
 
   // Annotate whether we buffered the next token to allow peeking
   bool _peekAvailable;
   Token _bufferedToken;
 };
+
+/// Encapsulates a linker script reading session, including lexer, parser and
+/// semantic analysis.
+class ScriptInstance {
+public:
+  ScriptInstance(std::unique_ptr<MemoryBuffer> mb)
+      : _lex(llvm::make_unique<Lexer>(std::move(mb))),
+        _parser(llvm::make_unique<Parser>(*_lex)) {
+    _topLevelNode = _parser->parse();
+  }
+
+  LinkerScript *getTopLevelNode();
+
+private:
+  std::unique_ptr<Lexer> _lex;
+  std::unique_ptr<Parser> _parser;
+  LinkerScript *_topLevelNode;
+};
 } // end namespace script
 } // end namespace lld
 
 #endif

lib/Driver/GnuLdDriver.cpp

 }
 
 std::error_code
 GnuLdDriver::evalLinkerScript(ELFLinkingContext &ctx,
                               std::unique_ptr<MemoryBuffer> mb,
                               raw_ostream &diag) {
   // Read the script file from disk and parse.
   StringRef path = mb->getBufferIdentifier();
-  auto lexer = llvm::make_unique<script::Lexer>(std::move(mb));
-  auto parser = llvm::make_unique<script::Parser>(*lexer);
-  script::LinkerScript *script = parser->parse();
+  script::LinkerScript *script =
+      ctx.addLinkerScriptInstance(llvm::make_unique<script::ScriptInstance>(
+                                      std::move(mb))).getTopLevelNode();
   if (!script)
     return LinkerScriptReaderError::parse_error;
   // Evaluate script commands.
   // Currently we only recognize this subset of linker script commands:
   // - GROUP()
   // - SEARCH_DIR()
   for (const script::Command *c : script->_commands) {
     if (auto *group = dyn_cast<script::Group>(c))

lib/ReaderWriter/ELF/ELFLinkingContext.cpp

ELFLinkingContext::ELFLinkingContext(
     llvm::Triple triple, std::unique_ptr<TargetHandlerBase> targetHandler)
     : _outputELFType(llvm::ELF::ET_EXEC), _triple(triple),
       _targetHandler(std::move(targetHandler)), _baseAddress(0),
       _isStaticExecutable(false), _noInhibitExec(false), _exportDynamic(false),
       _mergeCommonStrings(false), _useShlibUndefines(true),
       _dynamicLinkerArg(false), _noAllowDynamicLibraries(false),
       _mergeRODataToTextSegment(true), _demangle(true), _alignSegments(true),
       _outputMagic(OutputMagic::DEFAULT), _initFunction("_init"),
-      _finiFunction("_fini"), _sysrootPath("") {}
+      _finiFunction("_fini"), _sysrootPath(""), _masterLinkerScript(nullptr) {}
 
 void ELFLinkingContext::addPasses(PassManager &pm) {
   pm.add(std::unique_ptr<Pass>(new elf::OrderPass()));
 }
 
 uint16_t ELFLinkingContext::getOutputMachine() const {
   switch (getTriple().getArch()) {
   case llvm::Triple::x86:

lib/ReaderWriter/LinkerScript.cpp

Sections *Parser::parseSections() {
   if (!expectAndConsume(
           Token::r_brace,
           "expected symbol assignment, entry, overlay or output section name."))
     return nullptr;
 
   return new (_alloc) Sections(sectionsCommands);
 }
 
+// ScriptInstance functions
+LinkerScript *ScriptInstance::getTopLevelNode() {
+  return _topLevelNode;
+}
+
 } // end namespace script
 } // end namespace lld