Skip to content

Commit

Permalink
Revert "[LLD] [COFF] Fix linking MSVC generated implib header objects" (
Browse files Browse the repository at this point in the history
llvm#123877)

Reverts llvm#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
    llvm#2 0x55a979a99e7d in getSymbols /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/COFF/LLDMapFile.cpp:54:42
    llvm#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
    llvm#4 0x55a979a16879 in (anonymous namespace)::Writer::run() /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/COFF/Writer.cpp:810:3
    llvm#5 0x55a979a00aac in lld::coff::writeResult(lld::coff::COFFLinkerContext&) /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/COFF/Writer.cpp:354:15
    llvm#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
    llvm#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
    llvm#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
    llvm#9 0x55a9797fa3b6 in operator() /usr/local/google/home/thurston/buildbot_repro/llvm-project/lld/Common/DriverDispatcher.cpp:188:15
    llvm#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
    llvm#11 0x55a97966cb93 in operator() /usr/local/google/home/thurston/buildbot_repro/llvm-project/llvm/include/llvm/ADT/STLFunctionalExtras.h:69:12
    llvm#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
    llvm#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
    llvm#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
    llvm#15 0x55a979628731 in main /usr/local/google/home/thurston/buildbot_repro/llvm_build_asan/tools/lld/tools/lld/lld-driver.cpp:17:10
    llvm#16 0x7ffb8b202c89 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    llvm#17 0x7ffb8b202d44 in __libc_start_main csu/../csu/libc-start.c:360:3
    llvm#18 0x55a97953ef60 in _start (/usr/local/google/home/thurston/buildbot_repro/llvm_build_asan/bin/lld+0x8fd1f60)
```
  • Loading branch information
thurstond authored Jan 22, 2025
1 parent 05861b3 commit c53faf6
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 34 deletions.
31 changes: 8 additions & 23 deletions lld/COFF/InputFiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,16 +458,9 @@ Symbol *ObjFile::createRegular(COFFSymbolRef sym) {
return nullptr;
return symtab.addUndefined(name, this, false);
}
if (sc) {
const coff_symbol_generic *symGen = sym.getGeneric();
if (sym.isSection()) {
auto *customSymGen = make<coff_symbol_generic>(*symGen);
customSymGen->Value = 0;
symGen = customSymGen;
}
if (sc)
return make<DefinedRegular>(this, /*Name*/ "", /*IsCOMDAT*/ false,
/*IsExternal*/ false, symGen, sc);
}
/*IsExternal*/ false, sym.getGeneric(), sc);
return nullptr;
}

Expand Down Expand Up @@ -762,23 +755,15 @@ std::optional<Symbol *> ObjFile::createDefined(
memset(hdr, 0, sizeof(*hdr));
strncpy(hdr->Name, name.data(),
std::min(name.size(), (size_t)COFF::NameSize));
// The Value field in a section symbol may contain the characteristics,
// or it may be zero, where we make something up (that matches what is
// used in .idata sections in the regular object files in import libraries).
if (sym.getValue())
hdr->Characteristics = sym.getValue() | IMAGE_SCN_ALIGN_4BYTES;
else
hdr->Characteristics = IMAGE_SCN_CNT_INITIALIZED_DATA |
IMAGE_SCN_MEM_READ | IMAGE_SCN_MEM_WRITE |
IMAGE_SCN_ALIGN_4BYTES;
// We have no idea what characteristics should be assumed here; pick
// a default. This matches what is used for .idata sections in the regular
// object files in import libraries.
hdr->Characteristics = IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ |
IMAGE_SCN_MEM_WRITE | IMAGE_SCN_ALIGN_4BYTES;
auto *sc = make<SectionChunk>(this, hdr);
chunks.push_back(sc);

coff_symbol_generic *symGen = make<coff_symbol_generic>(*sym.getGeneric());
// Ignore the Value offset of these symbols, as it may be a bitmask.
symGen->Value = 0;
return make<DefinedRegular>(this, /*name=*/"", /*isCOMDAT=*/false,
/*isExternal=*/false, symGen, sc);
/*isExternal=*/false, sym.getGeneric(), sc);
}

if (llvm::COFF::isReservedSectionNumber(sectionNumber))
Expand Down
13 changes: 5 additions & 8 deletions lld/test/COFF/empty-section-decl.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# RUN: FileCheck %s --check-prefix=MAP < %t.map

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

# MAP: 00001000 0000000a 4 {{.*}}:(.itest$2)
# MAP: 00001000 00000000 0 .itest$2
Expand All @@ -28,10 +28,7 @@ sections:
Relocations:
- VirtualAddress: 0
SymbolName: '.itest$4'
Type: IMAGE_REL_AMD64_ADDR32NB
- VirtualAddress: 4
SymbolName: '.itest$6'
Type: IMAGE_REL_AMD64_ADDR32NB
Type: IMAGE_REL_AMD64_ADDR64
- Name: '.itest$6'
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
Alignment: 2
Expand All @@ -45,13 +42,13 @@ symbols:
ComplexType: IMAGE_SYM_DTYPE_NULL
StorageClass: IMAGE_SYM_CLASS_SECTION
- Name: '.itest$6'
Value: 3221225536
Value: 0
SectionNumber: 2
SimpleType: IMAGE_SYM_TYPE_NULL
ComplexType: IMAGE_SYM_DTYPE_NULL
StorageClass: IMAGE_SYM_CLASS_SECTION
StorageClass: IMAGE_SYM_CLASS_STATIC
- Name: '.itest$4'
Value: 3221225536
Value: 0
SectionNumber: 0
SimpleType: IMAGE_SYM_TYPE_NULL
ComplexType: IMAGE_SYM_DTYPE_NULL
Expand Down
7 changes: 4 additions & 3 deletions llvm/include/llvm/Object/COFF.h
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ class COFFSymbolRef {
}

bool isCommon() const {
return isExternal() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED &&
getValue() != 0;
return (isExternal() || isSection()) &&
getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED && getValue() != 0;
}

bool isUndefined() const {
Expand All @@ -393,7 +393,8 @@ class COFFSymbolRef {
}

bool isEmptySectionDeclaration() const {
return isSection() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED;
return isSection() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED &&
getValue() == 0;
}

bool isWeakExternal() const {
Expand Down
20 changes: 20 additions & 0 deletions llvm/test/Object/coff-sec-sym.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Check that section symbol (IMAGE_SYM_CLASS_SECTION) is listed as common symbol.

# RUN: yaml2obj %s -o %t.obj
# RUN: llvm-nm %t.obj | FileCheck %s

# CHECK: 00000001 C foo

--- !COFF
header:
Machine: IMAGE_FILE_MACHINE_AMD64
Characteristics: [ ]
sections:
symbols:
- Name: foo
Value: 1
SectionNumber: 0
SimpleType: IMAGE_SYM_TYPE_NULL
ComplexType: IMAGE_SYM_DTYPE_NULL
StorageClass: IMAGE_SYM_CLASS_SECTION
...

0 comments on commit c53faf6

Please sign in to comment.