Revert "[LLD] [COFF] Fix linking MSVC generated implib header objects"

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))

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

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 {

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
+...