[llvm-objcopy] Fix --keep-global-symbol/--globalize-symbol for undefined symbols.

[rupprecht]

Summary

--keep-global-symbol and --globalize-symbol don't make sense for undefined symbols, so it should be ignored for those symbols. This matches GNU objcopy behavior.

[jhenderson]

My feeling is that a single separate test is not the right way to test this. I think it makes more sense for an undefined symbol to be added to globalize.test and --keep-global-symbols.test. What do you think?

test/tools/llvm-objcopy/globalize-undefined-sym.test

11

Are you sure this is Foo? That's a symbol that doesn't even exist. I assume you meant it to be Weak or Global?

[rupprecht]

test/tools/llvm-objcopy/globalize-undefined-sym.test

11

Foo is intentional, sorry for not explaining in a test comment why -- fixed.

[rupprecht]

• Update test

[jakehehrlich]

This seems correct to me. LGTM.

[rupprecht]

• Move test cases to already existing tests

[rupprecht]

• typo

[rupprecht]

In D53733#1278600, @jhenderson wrote:

My feeling is that a single separate test is not the right way to test this. I think it makes more sense for an undefined symbol to be added to globalize.test and --keep-global-symbols.test. What do you think?

Makes sense to me -- done.

[jhenderson]

LGTM.

test/tools/llvm-objcopy/globalize-undefined-sym.test

+# RUN: yaml2obj %s > %t.o
+# RUN: cp %t.o %t-copy.o
+
+# This test ensures that --globalize-symbol and --keep-global-symbol are noops
+# for undefined symbols.
+
+# RUN: llvm-objcopy --globalize-symbol Weak --globalize-symbol Global %t.o %t.2.o
+# RUN: llvm-readobj -symbols %t.2.o | FileCheck %s
+# RUN: cmp %t.o %t-copy.o
+
+# RUN: llvm-objcopy --keep-global-symbol Foo %t.o %t.3.o
+# RUN: llvm-readobj -symbols %t.3.o | FileCheck %s
+# RUN: cmp %t.o %t-copy.o
+
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:          .text
+    Type:          SHT_PROGBITS
+    Flags:         [ SHF_ALLOC, SHF_EXECINSTR ]
+Symbols:
+  Weak:
+    - Name:     Weak
+  Global:
+    - Name:     Global
+
+# CHECK:      Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name:
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Local (0x0)
+# CHECK-NEXT:     Type: None (0x0)
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined (0x0)
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: Global
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global (0x1)
+# CHECK-NEXT:     Type: None (0x0)
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined (0x0)
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: Weak
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Weak (0x2)
+# CHECK-NEXT:     Type: None (0x0)
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined (0x0)
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]

tools/llvm-objcopy/llvm-objcopy.cpp

Obj.SymbolTable->updateSymbols([&](Symbol &Sym) {
       // --globalize-symbol: promote a symbol to global
       // --keep-global-symbol: all symbols except for these should be made local
       //
       // If --globalize-symbol is specified for a given symbol, it will be
       // global in the output file even if it is not included via
       // --keep-global-symbol. Because of that, make sure to check
       // --globalize-symbol second.
       if (!Config.SymbolsToKeepGlobal.empty() &&
-          !is_contained(Config.SymbolsToKeepGlobal, Sym.Name))
+          !is_contained(Config.SymbolsToKeepGlobal, Sym.Name) &&
+          Sym.getShndx() != SHN_UNDEF)
         Sym.Binding = STB_LOCAL;
 
       if (!Config.SymbolsToGlobalize.empty() &&
-          is_contained(Config.SymbolsToGlobalize, Sym.Name))
+          is_contained(Config.SymbolsToGlobalize, Sym.Name) &&
+          Sym.getShndx() != SHN_UNDEF)
         Sym.Binding = STB_GLOBAL;
 
       if (!Config.SymbolsToWeaken.empty() &&
           is_contained(Config.SymbolsToWeaken, Sym.Name) &&
           Sym.Binding == STB_GLOBAL)
         Sym.Binding = STB_WEAK;
 
       if (Config.Weaken && Sym.Binding == STB_GLOBAL &&