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

Rui Ueyama ruiu at google.com
Thu Dec 26 23:05:04 PST 2013


Author: ruiu
Date: Fri Dec 27 01:05:04 2013
New Revision: 198071

URL: http://llvm.org/viewvc/llvm-project?rev=198071&view=rev
Log:
[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.

Added:
    lld/trunk/test/pecoff/Inputs/alternatename3.obj.yaml
Modified:
    lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
    lld/trunk/test/pecoff/alternatename.test

Modified: lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp?rev=198071&r1=198070&r2=198071&view=diff
==============================================================================
--- lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp (original)
+++ lld/trunk/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp Fri Dec 27 01:05:04 2013
@@ -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_a
   }
 }
 
-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<Memor
   }
   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(Arr
   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();
   }

Added: lld/trunk/test/pecoff/Inputs/alternatename3.obj.yaml
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/Inputs/alternatename3.obj.yaml?rev=198071&view=auto
==============================================================================
--- lld/trunk/test/pecoff/Inputs/alternatename3.obj.yaml (added)
+++ lld/trunk/test/pecoff/Inputs/alternatename3.obj.yaml Fri Dec 27 01:05:04 2013
@@ -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
+...

Modified: lld/trunk/test/pecoff/alternatename.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/alternatename.test?rev=198071&r1=198070&r2=198071&view=diff
==============================================================================
--- lld/trunk/test/pecoff/alternatename.test (original)
+++ lld/trunk/test/pecoff/alternatename.test Fri Dec 27 01:05:04 2013
@@ -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





More information about the llvm-commits mailing list