Revert "[LLD] [COFF] Fix linking MSVC generated implib header objects" #123877
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thurstond
added
the
skip-precommit-approval
|
@llvm/pr-subscribers-llvm-binary-utilities @llvm/pr-subscribers-lld-coff Author: Thurston Dang (thurstond) ChangesReverts 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: Full diff: https://github.com/llvm/llvm-project/pull/123877.diff 4 Files Affected:
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 74883ac9e7578c..5ee73d4dc4f8b7 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -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;
}
@@ -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))
diff --git a/lld/test/COFF/empty-section-decl.yaml b/lld/test/COFF/empty-section-decl.yaml
index 12fe6d44ebb832..320df340000289 100644
--- a/lld/test/COFF/empty-section-decl.yaml
+++ b/lld/test/COFF/empty-section-decl.yaml
@@ -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
@@ -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
@@ -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
diff --git a/llvm/include/llvm/Object/COFF.h b/llvm/include/llvm/Object/COFF.h
index 3d0738c4090497..4de2c680f57b1a 100644
--- a/llvm/include/llvm/Object/COFF.h
+++ b/llvm/include/llvm/Object/COFF.h
@@ -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 {
@@ -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 {
diff --git a/llvm/test/Object/coff-sec-sym.test b/llvm/test/Object/coff-sec-sym.test
new file mode 100644
index 00000000000000..0b7117250150de
--- /dev/null
+++ b/llvm/test/Object/coff-sec-sym.test
@@ -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
+...
|
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #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: