[PATCH] D53782: [llvm-objcopy] Don't apply --localize flags to common symbols

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: test/tools/llvm-objcopy/localize.test:44
+    - Name:     GlobalCommon
+      Type:     STT_OBJECT
+      Index:    SHN_COMMON
----------------
jakehehrlich wrote:
> Slight oddity that doesn't really matter. If Index is SHN_COMMON then the Type should always by STT_COMMON. In relocatable files if the type STT_COMMON then the index must be SHN_COMMON. In fully linked executables it needs to be something specific but the type should still be STT_COMMON. You should never see a symbol with index SHN_COMMON without STT_COMMON. 
It doesn't seem to be required that STT_COMMON is used for common symbols:

```
$ echo 'int foo;' | bin/clang -xc - -c -o /tmp/common.o; bin/llvm-readobj -symbols /tmp/common.o | grep -B1 -A7 foo
  Symbol {
    Name: foo (16)
    Value: 0x4
    Size: 4
    Binding: Global (0x1)
    Type: Object (0x1)
    Other: 0
    Section: Common (0xFFF2)
  }
```

However, there seems to be a `--elf-stt-common=yes` flag that some GNU tools (including objcopy) support:

```
$ echo 'int foo;' | bin/clang -xc - -c -o /tmp/common.o; objcopy --elf-stt-common=yes /tmp/common.o; bin/llvm-readobj -symbols /tmp/common.o | grep -B1 -A7 foo
  Symbol {
    Name: foo (3)
    Value: 0x4
    Size: 4
    Binding: Global (0x1)
    Type: Common (0x5)
    Other: 0
    Section: Common (0xFFF2)
  }
```

It looks like STT_COMMON was introduced later, and so it might be the direction that we want to eventually be in, but at least for now tools should be able to handle common objects that aren't explicitly STT_COMMON. Checking both type and section index seems to be standard practice, so I'll do that. (e.g. https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Object/ELFTypes.h#L219)


Repository:
  rL LLVM

https://reviews.llvm.org/D53782