[lld] r240216 - COFF: Fix a common symbol bug.
Rui Ueyama
ruiu at google.com
Sat Jun 20 00:21:57 PDT 2015
Author: ruiu
Date: Sat Jun 20 02:21:57 2015
New Revision: 240216
URL: http://llvm.org/viewvc/llvm-project?rev=240216&view=rev
Log:
COFF: Fix a common symbol bug.
This is a case that one mistake caused a very mysterious bug.
I made a mistake to calculate addresses of common symbols, so
each common symbol pointed not to the beginning of its location
but to the end of its location. (Ouch!)
Common symbols are aligned on 16 byte boundaries. If a common
symbol is small enough to fit between the end of its real
location and whatever comes next, this bug didn't cause any harm.
However, if a common symbol is larger than that, its memory
naturally overlapped with other symbols. That means some
uninitialized variables accidentally shared memory. Because
totally unrelated memory writes mutated other varaibles, it was
hard to debug.
It's surprising that LLD was able to link itself and all LLD
tests except gunit tests passed with this nasty bug.
With this fix, the new COFF linker is able to pass all tests
for LLVM, Clang and LLD if I use MSVC cl.exe as a compiler.
Only three tests are failing when used with clang-cl.
Added:
lld/trunk/test/COFF/Inputs/common.yaml
lld/trunk/test/COFF/common.test
Modified:
lld/trunk/COFF/InputFiles.cpp
lld/trunk/COFF/Symbols.cpp
lld/trunk/COFF/Symbols.h
Modified: lld/trunk/COFF/InputFiles.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=240216&r1=240215&r2=240216&view=diff
==============================================================================
--- lld/trunk/COFF/InputFiles.cpp (original)
+++ lld/trunk/COFF/InputFiles.cpp Sat Jun 20 02:21:57 2015
@@ -184,7 +184,7 @@ SymbolBody *ObjectFile::createSymbolBody
if (Sym.isCommon()) {
Chunk *C = new (Alloc) CommonChunk(Sym);
Chunks.push_back(C);
- return new (Alloc) DefinedRegular(COFFObj.get(), Sym, C);
+ return new (Alloc) DefinedCommon(COFFObj.get(), Sym, C);
}
if (Sym.isAbsolute()) {
COFFObj->getSymbolName(Sym, Name);
Modified: lld/trunk/COFF/Symbols.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Symbols.cpp?rev=240216&r1=240215&r2=240216&view=diff
==============================================================================
--- lld/trunk/COFF/Symbols.cpp (original)
+++ lld/trunk/COFF/Symbols.cpp Sat Jun 20 02:21:57 2015
@@ -29,21 +29,21 @@ int DefinedRegular::compare(SymbolBody *
auto *R = dyn_cast<DefinedRegular>(Other);
if (!R)
return 1;
-
- // Common symbols are weaker than other types of defined symbols.
- if (isCommon() && R->isCommon())
- return (getCommonSize() < R->getCommonSize()) ? -1 : 1;
- // TODO: we are not sure if regular defined symbol and common
- // symbols are allowed to have the same name.
- if (isCommon())
- return -1;
- if (R->isCommon())
- return 1;
if (isCOMDAT() && R->isCOMDAT())
return 1;
return 0;
}
+int DefinedCommon::compare(SymbolBody *Other) {
+ if (Other->kind() < kind())
+ return -Other->compare(this);
+ if (auto *D = dyn_cast<DefinedCommon>(Other))
+ return getSize() > D->getSize() ? 1 : -1;
+ if (isa<DefinedRegular>(Other))
+ return -1;
+ return 1;
+}
+
int DefinedBitcode::compare(SymbolBody *Other) {
assert(Other->kind() >= kind());
if (!isa<Defined>(Other))
@@ -63,10 +63,12 @@ int DefinedBitcode::compare(SymbolBody *
// resolution will be done accurately after lowering bitcode symbols
// to regular symbols in addCombinedLTOObject().
if (auto *R = dyn_cast<DefinedRegular>(Other)) {
- if (!R->isCommon() && !R->isCOMDAT() && !Replaceable)
+ if (!R->isCOMDAT() && !Replaceable)
return 0;
return -1;
}
+ if (isa<DefinedCommon>(Other))
+ return -1;
return 0;
}
@@ -111,6 +113,12 @@ StringRef DefinedRegular::getName() {
if (Name.empty())
COFFFile->getSymbolName(Sym, Name);
return Name;
+}
+
+StringRef DefinedCommon::getName() {
+ if (Name.empty())
+ COFFFile->getSymbolName(Sym, Name);
+ return Name;
}
ErrorOr<std::unique_ptr<InputFile>> Lazy::getMember() {
Modified: lld/trunk/COFF/Symbols.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Symbols.h?rev=240216&r1=240215&r2=240216&view=diff
==============================================================================
--- lld/trunk/COFF/Symbols.h (original)
+++ lld/trunk/COFF/Symbols.h Sat Jun 20 02:21:57 2015
@@ -49,6 +49,7 @@ public:
DefinedAbsoluteKind,
DefinedImportDataKind,
DefinedImportThunkKind,
+ DefinedCommonKind,
DefinedRegularKind,
DefinedLast,
LazyKind,
@@ -130,11 +131,32 @@ public:
bool isCOMDAT() const { return Data->isCOMDAT(); }
int compare(SymbolBody *Other) override;
- // Returns true if this is a common symbol.
- bool isCommon() const { return Sym.isCommon(); }
- uint32_t getCommonSize() const { return Sym.getValue(); }
+private:
+ StringRef Name;
+ COFFObjectFile *COFFFile;
+ COFFSymbolRef Sym;
+ Chunk *Data;
+};
+
+class DefinedCommon : public Defined {
+public:
+ DefinedCommon(COFFObjectFile *F, COFFSymbolRef S, Chunk *C)
+ : Defined(DefinedCommonKind), COFFFile(F), Sym(S), Data(C) {}
+
+ static bool classof(const SymbolBody *S) {
+ return S->kind() == DefinedCommonKind;
+ }
+
+ StringRef getName() override;
+ uint64_t getRVA() override { return Data->getRVA(); }
+ bool isExternal() override { return Sym.isExternal(); }
+ void markLive() override { Data->markLive(); }
+ uint64_t getFileOff() override { return Data->getFileOff(); }
+ int compare(SymbolBody *Other) override;
private:
+ uint64_t getSize() { return Sym.getValue(); }
+
StringRef Name;
COFFObjectFile *COFFFile;
COFFSymbolRef Sym;
Added: lld/trunk/test/COFF/Inputs/common.yaml
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/Inputs/common.yaml?rev=240216&view=auto
==============================================================================
--- lld/trunk/test/COFF/Inputs/common.yaml (added)
+++ lld/trunk/test/COFF/Inputs/common.yaml Sat Jun 20 02:21:57 2015
@@ -0,0 +1,91 @@
+---
+header:
+ Machine: IMAGE_FILE_MACHINE_AMD64
+ Characteristics: []
+sections:
+ - Name: .text
+ Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+ Alignment: 4
+ SectionData: b800000000b800000000b800000000b800000000b800000000
+ Relocations:
+ - VirtualAddress: 1
+ SymbolName: bssdata4
+ Type: IMAGE_REL_AMD64_ADDR32
+ - VirtualAddress: 6
+ SymbolName: bsspad1
+ Type: IMAGE_REL_AMD64_ADDR32
+ - VirtualAddress: 11
+ SymbolName: bssdata64
+ Type: IMAGE_REL_AMD64_ADDR32
+ - VirtualAddress: 16
+ SymbolName: bsspad2
+ Type: IMAGE_REL_AMD64_ADDR32
+ - VirtualAddress: 21
+ SymbolName: bssdata16
+ Type: IMAGE_REL_AMD64_ADDR32
+ - Name: .data
+ Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+ Alignment: 4
+ SectionData: 03000000
+symbols:
+ - Name: .text
+ Value: 0
+ SectionNumber: 1
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_STATIC
+ SectionDefinition:
+ Length: 0
+ NumberOfRelocations: 5
+ NumberOfLinenumbers: 0
+ CheckSum: 0
+ Number: 0
+ - Name: .data
+ Value: 0
+ SectionNumber: 2
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_STATIC
+ SectionDefinition:
+ Length: 4
+ NumberOfRelocations: 0
+ NumberOfLinenumbers: 0
+ CheckSum: 0
+ Number: 0
+ - Name: mainCRTStartup
+ Value: 0
+ SectionNumber: 1
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+ - Name: bssdata4
+ Value: 4
+ SectionNumber: 0
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+ - Name: bsspad1
+ Value: 1
+ SectionNumber: 0
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+ - Name: bssdata64
+ Value: 64
+ SectionNumber: 0
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+ - Name: bsspad2
+ Value: 1
+ SectionNumber: 0
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+ - Name: bssdata16
+ Value: 16
+ SectionNumber: 0
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+...
Added: lld/trunk/test/COFF/common.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/common.test?rev=240216&view=auto
==============================================================================
--- lld/trunk/test/COFF/common.test (added)
+++ lld/trunk/test/COFF/common.test Sat Jun 20 02:21:57 2015
@@ -0,0 +1,10 @@
+# RUN: yaml2obj %p/Inputs/common.yaml > %t.obj
+# RUN: lld -flavor link2 /out:%t.exe %t.obj %t.obj
+# RUN: llvm-objdump -d %t.exe | FileCheck %s
+
+# Operands of B8 (MOV EAX) are common symbols
+CHECK: 3000: b8 00 10 00 40
+CHECK: 3005: b8 10 10 00 40
+CHECK: 300a: b8 20 10 00 40
+CHECK: 300f: b8 60 10 00 40
+CHECK: 3014: b8 70 10 00 40
More information about the llvm-commits
mailing list