[llvm] ecb5ea6 - [Object][COFF] Allow section symbol to be common symbol

[Pengxuan Zheng via llvm-commits]

Author: Pengxuan Zheng
Date: 2022-09-13T18:07:02-07:00
New Revision: ecb5ea6a266d5cc4e05252f6db4c73613b73cc3b

URL: https://github.com/llvm/llvm-project/commit/ecb5ea6a266d5cc4e05252f6db4c73613b73cc3b
DIFF: https://github.com/llvm/llvm-project/commit/ecb5ea6a266d5cc4e05252f6db4c73613b73cc3b.diff

LOG: [Object][COFF] Allow section symbol to be common symbol

I ran into an lld-link error due to a symbol named ".idata$4" coming from some
static library:
  .idata$4 should not refer to special section 0.

Here is the symbol table entry for .idata$4:

  Symbol {
      Name: .idata$4
      Value: 3221225536
      Section: IMAGE_SYM_UNDEFINED (0)
      BaseType: Null (0x0)
      ComplexType: Null (0x0)
      StorageClass: Section (0x68)
      AuxSymbolCount: 0
  }

The symbol .idata$4 is a section symbol (IMAGE_SYM_CLASS_SECTION) and LLD
currently handles it as a regular defined symbol since isCommon() returns false
for this symbol. This results in the error ".idata$4 should not refer to special
section 0" because lld-link asserts that regular defined symbols should not
refer to section 0.

Should this symbol be handled as a common symbol instead? LLVM currently only
allows external symbols (IMAGE_SYM_CLASS_EXTERNAL) to be common
symbols. However, the PE/COFF spec (see section "Section Number Values") does
not seem to mention this restriction. Any thoughts?

Reviewed By: thakis

Differential Revision: https://reviews.llvm.org/D133627

Added: 
    llvm/test/Object/coff-sec-sym.test

Modified: 
    llvm/include/llvm/Object/COFF.h

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Object/COFF.h b/llvm/include/llvm/Object/COFF.h
index 8a110078ffdfd..7aa9eaf2287d0 100644
--- a/llvm/include/llvm/Object/COFF.h
+++ b/llvm/include/llvm/Object/COFF.h
@@ -379,8 +379,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 {

diff  --git a/llvm/test/Object/coff-sec-sym.test b/llvm/test/Object/coff-sec-sym.test
new file mode 100644
index 0000000000000..0b7117250150d
--- /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
+...