Skip to content

Commit c53faf6

Browse files
thurstondthurstond
authored
Revert "[LLD] [COFF] Fix linking MSVC generated implib header objects" (#123877)
Reverts llvm/llvm-project#122811 due to buildbot breakage e.g., https://lab.llvm.org/buildbot/#/builders/52/builds/5421/steps/11/logs/stdio ASan output from local re-run: ``` ==2780289==ERROR: AddressSanitizer: use-after-poison on address 0x7e0b87e28d28 at pc 0x55a979a99e7e bp 0x7ffe4b18f0b0 sp 0x7ffe4b18f0a8 READ of size 1 at 0x7e0b87e28d28 thread T0 #0 0x55a979a99e7d in getStorageClass /usr/local/google/home/thurston/buildbot_repro/llvm-project/llvm/include/llvm/Object/COFF.h:344 #1 0x55a979a99e7d in isSectionDefinition /usr/local/google/home/thurston/buildbot_repro/llvm-project/llvm/include/llvm/Object/COFF.h:429:9 #2 0x55a979a99e7d in getSymbols /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/COFF/LLDMapFile.cpp:54:42 #3 0x55a979a99e7d in lld::coff::writeLLDMapFile(lld::coff::COFFLinkerContext const&) /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/COFF/LLDMapFile.cpp:103:40 #4 0x55a979a16879 in (anonymous namespace)::Writer::run() /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/COFF/Writer.cpp:810:3 #5 0x55a979a00aac in lld::coff::writeResult(lld::coff::COFFLinkerContext&) /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/COFF/Writer.cpp:354:15 #6 0x55a97985f7ed in lld::coff::LinkerDriver::linkerMain(llvm::ArrayRef<char const*>) /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/COFF/Driver.cpp:2826:3 #7 0x55a97984cdd3 in lld::coff::link(llvm::ArrayRef<char const*>, llvm::raw_ostream&, llvm::raw_ostream&, bool, bool) /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/COFF/Driver.cpp:97:15 #8 0x55a9797f9793 in lld::unsafeLldMain(llvm::ArrayRef<char const*>, llvm::raw_ostream&, llvm::raw_ostream&, llvm::ArrayRef<lld::DriverDef>, bool) /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/Common/DriverDispatcher.cpp:163:12 #9 0x55a9797fa3b6 in operator() /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/Common/DriverDispatcher.cpp:188:15 #10 0x55a9797fa3b6 in void llvm::function_ref<void ()>::callback_fn<lld::lldMain(llvm::ArrayRef<char const*>, llvm::raw_ostream&, llvm::raw_ostream&, llvm::ArrayRef<lld::DriverDef>)::$_0>(long) /usr/local/google/home/thurston/buildbot_repro/llvm-project/llvm/include/llvm/ADT/STLFunctionalExtras.h:46:12 #11 0x55a97966cb93 in operator() /usr/local/google/home/thurston/buildbot_repro/llvm-project/llvm/include/llvm/ADT/STLFunctionalExtras.h:69:12 #12 0x55a97966cb93 in llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) /usr/local/google/home/thurston/buildbot_repro/llvm-project/llvm/lib/Support/CrashRecoveryContext.cpp:426:3 #13 0x55a9797f9dc3 in lld::lldMain(llvm::ArrayRef<char const*>, llvm::raw_ostream&, llvm::raw_ostream&, llvm::ArrayRef<lld::DriverDef>) /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/Common/DriverDispatcher.cpp:187:14 #14 0x55a979627512 in lld_main(int, char**, llvm::ToolContext const&) /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/tools/lld/lld.cpp:103:14 #15 0x55a979628731 in main /usr/local/google/home/thurston/buildbot_repro/llvm_build_asan/tools/lld/tools/lld/lld-driver.cpp:17:10 #16 0x7ffb8b202c89 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 #17 0x7ffb8b202d44 in __libc_start_main csu/../csu/libc-start.c:360:3 #18 0x55a97953ef60 in _start (/usr/local/google/home/thurston/buildbot_repro/llvm_build_asan/bin/lld+0x8fd1f60) ```
1 parent 05861b3 commit c53faf6

File tree

4 files changed

+37
-34
lines changed

4 files changed

+37
-34
lines changed

‎lld/COFF/InputFiles.cpp‎

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -458,16 +458,9 @@ Symbol *ObjFile::createRegular(COFFSymbolRef sym) {
458458
return nullptr;
459459
return symtab.addUndefined(name, this, false);
460460
}
461-
if (sc) {
462-
const coff_symbol_generic *symGen = sym.getGeneric();
463-
if (sym.isSection()) {
464-
auto *customSymGen = make<coff_symbol_generic>(*symGen);
465-
customSymGen->Value = 0;
466-
symGen = customSymGen;
467-
}
461+
if (sc)
468462
return make<DefinedRegular>(this, /*Name*/ "", /*IsCOMDAT*/ false,
469-
/*IsExternal*/ false, symGen, sc);
470-
}
463+
/*IsExternal*/ false, sym.getGeneric(), sc);
471464
return nullptr;
472465
}
473466

@@ -762,23 +755,15 @@ std::optional<Symbol *> ObjFile::createDefined(
762755
memset(hdr, 0, sizeof(*hdr));
763756
strncpy(hdr->Name, name.data(),
764757
std::min(name.size(), (size_t)COFF::NameSize));
765-
// The Value field in a section symbol may contain the characteristics,
766-
// or it may be zero, where we make something up (that matches what is
767-
// used in .idata sections in the regular object files in import libraries).
768-
if (sym.getValue())
769-
hdr->Characteristics = sym.getValue() | IMAGE_SCN_ALIGN_4BYTES;
770-
else
771-
hdr->Characteristics = IMAGE_SCN_CNT_INITIALIZED_DATA |
772-
IMAGE_SCN_MEM_READ | IMAGE_SCN_MEM_WRITE |
773-
IMAGE_SCN_ALIGN_4BYTES;
758+
// We have no idea what characteristics should be assumed here; pick
759+
// a default. This matches what is used for .idata sections in the regular
760+
// object files in import libraries.
761+
hdr->Characteristics = IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ |
762+
IMAGE_SCN_MEM_WRITE | IMAGE_SCN_ALIGN_4BYTES;
774763
auto *sc = make<SectionChunk>(this, hdr);
775764
chunks.push_back(sc);
776-
777-
coff_symbol_generic *symGen = make<coff_symbol_generic>(*sym.getGeneric());
778-
// Ignore the Value offset of these symbols, as it may be a bitmask.
779-
symGen->Value = 0;
780765
return make<DefinedRegular>(this, /*name=*/"", /*isCOMDAT=*/false,
781-
/*isExternal=*/false, symGen, sc);
766+
/*isExternal=*/false, sym.getGeneric(), sc);
782767
}
783768

784769
if (llvm::COFF::isReservedSectionNumber(sectionNumber))

‎lld/test/COFF/empty-section-decl.yaml‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
# RUN: FileCheck %s --check-prefix=MAP < %t.map
77

88
# CHECK: Contents of section .itest:
9-
# CHECK-NEXT: 180001000 0c100000 0c100000 00000000 01000000
9+
# CHECK-NEXT: 180001000 0c100080 01000000 00000000 01000000
1010

1111
# MAP: 00001000 0000000a 4 {{.*}}:(.itest$2)
1212
# MAP: 00001000 00000000 0 .itest$2
@@ -28,10 +28,7 @@ sections:
2828
Relocations:
2929
- VirtualAddress: 0
3030
SymbolName: '.itest$4'
31-
Type: IMAGE_REL_AMD64_ADDR32NB
32-
- VirtualAddress: 4
33-
SymbolName: '.itest$6'
34-
Type: IMAGE_REL_AMD64_ADDR32NB
31+
Type: IMAGE_REL_AMD64_ADDR64
3532
- Name: '.itest$6'
3633
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
3734
Alignment: 2
@@ -45,13 +42,13 @@ symbols:
4542
ComplexType: IMAGE_SYM_DTYPE_NULL
4643
StorageClass: IMAGE_SYM_CLASS_SECTION
4744
- Name: '.itest$6'
48-
Value: 3221225536
45+
Value: 0
4946
SectionNumber: 2
5047
SimpleType: IMAGE_SYM_TYPE_NULL
5148
ComplexType: IMAGE_SYM_DTYPE_NULL
52-
StorageClass: IMAGE_SYM_CLASS_SECTION
49+
StorageClass: IMAGE_SYM_CLASS_STATIC
5350
- Name: '.itest$4'
54-
Value: 3221225536
51+
Value: 0
5552
SectionNumber: 0
5653
SimpleType: IMAGE_SYM_TYPE_NULL
5754
ComplexType: IMAGE_SYM_DTYPE_NULL

‎llvm/include/llvm/Object/COFF.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,8 @@ class COFFSymbolRef {
383383
}
384384

385385
bool isCommon() const {
386-
return isExternal() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED &&
387-
getValue() != 0;
386+
return (isExternal() || isSection()) &&
387+
getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED && getValue() != 0;
388388
}
389389

390390
bool isUndefined() const {
@@ -393,7 +393,8 @@ class COFFSymbolRef {
393393
}
394394

395395
bool isEmptySectionDeclaration() const {
396-
return isSection() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED;
396+
return isSection() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED &&
397+
getValue() == 0;
397398
}
398399

399400
bool isWeakExternal() const {

‎llvm/test/Object/coff-sec-sym.test‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Check that section symbol (IMAGE_SYM_CLASS_SECTION) is listed as common symbol.
2+
3+
# RUN: yaml2obj %s -o %t.obj
4+
# RUN: llvm-nm %t.obj | FileCheck %s
5+
6+
# CHECK: 00000001 C foo
7+
8+
--- !COFF
9+
header:
10+
Machine: IMAGE_FILE_MACHINE_AMD64
11+
Characteristics: [ ]
12+
sections:
13+
symbols:
14+
- Name: foo
15+
Value: 1
16+
SectionNumber: 0
17+
SimpleType: IMAGE_SYM_TYPE_NULL
18+
ComplexType: IMAGE_SYM_DTYPE_NULL
19+
StorageClass: IMAGE_SYM_CLASS_SECTION
20+
...

0 commit comments

Comments
 (0)