read/coff: SymbolKind::Common detection was too broad (#592)

Common symbols should be IMAGE_SYM_CLASS_EXTERNAL.

We were also matching on IMAGE_SYM_CLASS_SECTION, which
can occur for section symbols in import libraries where
the section is defined in another archive member.

Author: philipc

crates/examples/testfiles/coff/import_msvc.lib.objdump

@@ -89,8 +89,8 @@ Symbols
 1: Symbol { name: "__IMPORT_DESCRIPTOR_test_x64", address: 0, size: 0, kind: Data, section: Section(SectionIndex(2)), scope: Linkage, weak: false, flags: None }
 2: Symbol { name: ".idata$2", address: 0, size: 0, kind: Section, section: Section(SectionIndex(2)), scope: Compilation, weak: false, flags: None }
 3: Symbol { name: ".idata$6", address: 0, size: 0, kind: Data, section: Section(SectionIndex(3)), scope: Compilation, weak: false, flags: None }
-4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None }
-5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None }
+4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Undefined, scope: Compilation, weak: false, flags: None }
+5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Undefined, scope: Compilation, weak: false, flags: None }
 6: Symbol { name: "__NULL_IMPORT_DESCRIPTOR", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None }
 7: Symbol { name: "\u{7f}test_x64_NULL_THUNK_DATA", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None }
 
@@ -166,8 +166,8 @@ Symbols
 1: Symbol { name: "__IMPORT_DESCRIPTOR_test_x86", address: 0, size: 0, kind: Data, section: Section(SectionIndex(2)), scope: Linkage, weak: false, flags: None }
 2: Symbol { name: ".idata$2", address: 0, size: 0, kind: Section, section: Section(SectionIndex(2)), scope: Compilation, weak: false, flags: None }
 3: Symbol { name: ".idata$6", address: 0, size: 0, kind: Data, section: Section(SectionIndex(3)), scope: Compilation, weak: false, flags: None }
-4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None }
-5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None }
+4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Undefined, scope: Compilation, weak: false, flags: None }
+5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Undefined, scope: Compilation, weak: false, flags: None }
 6: Symbol { name: "__NULL_IMPORT_DESCRIPTOR", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None }
 7: Symbol { name: "\u{7f}test_x86_NULL_THUNK_DATA", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None }
 
@@ -243,8 +243,8 @@ Symbols
 1: Symbol { name: "__IMPORT_DESCRIPTOR_test_arm64ec", address: 0, size: 0, kind: Data, section: Section(SectionIndex(2)), scope: Linkage, weak: false, flags: None }
 2: Symbol { name: ".idata$2", address: 0, size: 0, kind: Section, section: Section(SectionIndex(2)), scope: Compilation, weak: false, flags: None }
 3: Symbol { name: ".idata$6", address: 0, size: 0, kind: Data, section: Section(SectionIndex(3)), scope: Compilation, weak: false, flags: None }
-4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None }
-5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None }
+4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Undefined, scope: Compilation, weak: false, flags: None }
+5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Undefined, scope: Compilation, weak: false, flags: None }
 6: Symbol { name: "__NULL_IMPORT_DESCRIPTOR", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None }
 7: Symbol { name: "\u{7f}test_arm64ec_NULL_THUNK_DATA", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None }
 

src/read/coff/symbol.rs

@@ -401,12 +401,16 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> ObjectSymbol<'data>
     fn section(&self) -> SymbolSection {
         match self.symbol.section_number() {
             pe::IMAGE_SYM_UNDEFINED => {
-                if self.symbol.storage_class() == pe::IMAGE_SYM_CLASS_EXTERNAL
-                    && self.symbol.value() == 0
-                {
+                if self.symbol.storage_class() == pe::IMAGE_SYM_CLASS_EXTERNAL {
+                    if self.symbol.value() == 0 {
+                        SymbolSection::Undefined
+                    } else {
+                        SymbolSection::Common
+                    }
+                } else if self.symbol.storage_class() == pe::IMAGE_SYM_CLASS_SECTION {
                     SymbolSection::Undefined
                 } else {
-                    SymbolSection::Common
+                    SymbolSection::Unknown
                 }
             }
             pe::IMAGE_SYM_ABSOLUTE => SymbolSection::Absolute,