[PECOFF] Parse .drectve section before reading other file contents.

Currently .drectve section contents are parsed after other sections are parsed.
That order may result in wrong results if other sections depend on command line
options in the directive section.

For example, if a weak symbol is defined using /alternatename option in the
directive section, we have to read it first and then read the text section
contents. Otherwise the weak symbol won't be defined.

This patch changes the order to fix the issue.

llvm-svn: 198071

lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp

@@ -64,8 +64,10 @@ private:
 public:
   typedef const std::map<std::string, std::string> StringMap;
 
-  FileCOFF(std::unique_ptr<MemoryBuffer> mb, StringMap &altNames,
-           error_code &ec);
+  FileCOFF(std::unique_ptr<MemoryBuffer> mb, error_code &ec);
+
+  error_code parse(StringMap &altNames);
+  StringRef getLinkerDirectives() const { return _directives; }
 
   virtual const atom_collection<DefinedAtom> &defined() const {
     return _definedAtoms;
@@ -83,8 +85,6 @@ public:
     return _absoluteAtoms;
   }
 
-  StringRef getLinkerDirectives() const { return _directives; }
-
 private:
   error_code readSymbolTable(vector<const coff_symbol *> &result);
 
@@ -122,7 +122,6 @@ private:
   error_code addRelocationReferenceToAtoms();
   error_code findSection(StringRef name, const coff_section *&result);
   StringRef ArrayRefToString(ArrayRef<uint8_t> array);
-  error_code maybeReadLinkerDirectives();
 
   std::unique_ptr<const llvm::object::COFFObjectFile> _obj;
   atom_collection_vector<DefinedAtom> _definedAtoms;
@@ -252,8 +251,7 @@ DefinedAtom::Merge getMerge(const coff_aux_section_definition *auxsym) {
   }
 }
 
-FileCOFF::FileCOFF(std::unique_ptr<MemoryBuffer> mb, StringMap &altNames,
-                   error_code &ec)
+FileCOFF::FileCOFF(std::unique_ptr<MemoryBuffer> mb, error_code &ec)
     : File(mb->getBufferIdentifier(), kindObject), _ordinal(0) {
   OwningPtr<llvm::object::Binary> bin;
   ec = llvm::object::createBinary(mb.release(), bin);
@@ -267,24 +265,34 @@ FileCOFF::FileCOFF(std::unique_ptr<MemoryBuffer> mb, StringMap &altNames,
   }
   bin.take();
 
+  // Read .drectve section if exists.
+  const coff_section *section = nullptr;
+  if ((ec = findSection(".drectve", section)))
+    return;
+  if (section != nullptr) {
+    ArrayRef<uint8_t> contents;
+    if ((ec = _obj->getSectionContents(section, contents)))
+      return;
+    _directives = ArrayRefToString(contents);
+  }
+}
+
+error_code FileCOFF::parse(StringMap &altNames) {
   // Read the symbol table and atomize them if possible. Defined atoms
   // cannot be atomized in one pass, so they will be not be atomized but
   // added to symbolToAtom.
   SymbolVectorT symbols;
-  if ((ec = readSymbolTable(symbols)))
-    return;
+  if (error_code ec = readSymbolTable(symbols))
+    return ec;
 
   createAbsoluteAtoms(symbols, _absoluteAtoms._atoms);
-  if ((ec = createUndefinedAtoms(symbols, _undefinedAtoms._atoms)))
-    return;
-  if ((ec = createDefinedSymbols(symbols, altNames, _definedAtoms._atoms)))
-    return;
-
-  if ((ec = addRelocationReferenceToAtoms()))
-    return;
-
-  // Read .drectve section if exists.
-  ec = maybeReadLinkerDirectives();
+  if (error_code ec = createUndefinedAtoms(symbols, _undefinedAtoms._atoms))
+    return ec;
+  if (error_code ec = createDefinedSymbols(symbols, altNames, _definedAtoms._atoms))
+    return ec;
+  if (error_code ec = addRelocationReferenceToAtoms())
+    return ec;
+  return error_code::success();
 }
 
 /// Iterate over the symbol table to retrieve all symbols.
@@ -778,20 +786,6 @@ StringRef FileCOFF::ArrayRefToString(ArrayRef<uint8_t> array) {
   return StringRef(*contents).trim();
 }
 
-// Read .drectve section contents if exists, and store it to _directives.
-error_code FileCOFF::maybeReadLinkerDirectives() {
-  const coff_section *section = nullptr;
-  if (error_code ec = findSection(".drectve", section))
-    return ec;
-  if (section != nullptr) {
-    ArrayRef<uint8_t> contents;
-    if (error_code ec = _obj->getSectionContents(section, contents))
-      return ec;
-    _directives = ArrayRefToString(contents);
-  }
-  return error_code::success();
-}
-
 // Convert .res file to .coff file and then parse it. Resource file is a file
 // containing various types of data, such as icons, translation texts,
 // etc. "cvtres.exe" command reads an RC file to create a COFF file which
@@ -821,11 +815,12 @@ public:
       return ec;
     std::unique_ptr<MemoryBuffer> newmb(opmb.take());
     error_code ec;
-    FileCOFF::StringMap emptyMap;
-    std::unique_ptr<FileCOFF> file(
-        new FileCOFF(std::move(newmb), emptyMap, ec));
+    std::unique_ptr<FileCOFF> file(new FileCOFF(std::move(newmb), ec));
     if (ec)
       return ec;
+    FileCOFF::StringMap emptyMap;
+    if (error_code ec = file->parse(emptyMap))
+      return ec;
     result.push_back(std::move(file));
     return error_code::success();
   }
@@ -912,17 +907,18 @@ public:
             std::vector<std::unique_ptr<File>> &result) const {
     // Parse the memory buffer as PECOFF file.
     error_code ec;
-    std::unique_ptr<FileCOFF> file(
-        new FileCOFF(std::move(mb), _context.alternateNames(), ec));
+    std::unique_ptr<FileCOFF> file(new FileCOFF(std::move(mb), ec));
     if (ec)
       return ec;
 
     // Interpret .drectve section if the section has contents.
     StringRef directives = file->getLinkerDirectives();
     if (!directives.empty())
-      if (error_code ec2 = handleDirectiveSection(registry, directives))
-        return ec2;
+      if (error_code ec = handleDirectiveSection(registry, directives))
+        return ec;
 
+    if (error_code ec = file->parse(_context.alternateNames()))
+      return ec;
     result.push_back(std::move(file));
     return error_code::success();
   }

lld/test/pecoff/Inputs/alternatename3.obj.yaml

@@ -0,0 +1,35 @@
+---
+header:
+  Machine:         IMAGE_FILE_MACHINE_I386
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     90909090
+  - Name:            .drectve
+    Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ]
+    Alignment:       2147483648
+    SectionData:     2F616C7465726E6174656E616D653A5F6D61696E3D5F666F6F00
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            _foo
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            .drectve
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    NumberOfAuxSymbols: 1
+    AuxiliaryData:   0D0000000000000000000000000000000000
+...

lld/test/pecoff/alternatename.test

@@ -1,11 +1,15 @@
 # RUN: yaml2obj %p/Inputs/alternatename1.obj.yaml > %t1.obj
 # RUN: yaml2obj %p/Inputs/alternatename2.obj.yaml > %t2.obj
+# RUN: yaml2obj %p/Inputs/alternatename3.obj.yaml > %t3.obj
 #
 # RUN: lld -flavor link /out:%t1.exe /alternatename:_main=_foo -- %t1.obj
 # RUN: llvm-objdump -d %t1.exe | FileCheck -check-prefix=CHECK1 %s
 #
 # RUN: lld -flavor link /out:%t2.exe /alternatename:_main=_foo  -- %t1.obj %t2.obj
 # RUN: llvm-objdump -d %t2.exe | FileCheck -check-prefix=CHECK2 %s
+#
+# RUN: lld -flavor link /out:%t3.exe -- %t3.obj
+# RUN: llvm-objdump -d %t3.exe | FileCheck -check-prefix=CHECK3 %s
 
 CHECK1:      nop
 CHECK1-NEXT: nop
@@ -18,3 +22,9 @@ CHECK2:      int3
 CHECK2-NEXT: int3
 CHECK2-NEXT: int3
 CHECK2-NEXT: int3
+
+CHECK3:      nop
+CHECK3-NEXT: nop
+CHECK3-NEXT: nop
+CHECK3-NEXT: nop
+CHECK3-NOT:  int3