Ah fair. That looks like many cases we've unfortunately seen before (see SHF_LINK and SHF_COMPRESSED) that are not always respected (and thus we must support the old case as well). So disregard my comment about using the type. That's what I get for blindly reading the spec.<div><br></div><div>l'm still waiting to hear back from Roland about the sensibilities of this but what's sensible probably doesn't matter the more I think about it. I'll give a +2 later tomorrow if I can't think of a better way to resolve this in a practical manner. As of yet this seems like the best option to me.<br><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 29, 2018, 3:21 PM Jordan Rupprecht via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rupprecht added inline comments.<br>
<br>
<br>
================<br>
Comment at: test/tools/llvm-objcopy/localize.test:44<br>
+    - Name:     GlobalCommon<br>
+      Type:     STT_OBJECT<br>
+      Index:    SHN_COMMON<br>
----------------<br>
jakehehrlich wrote:<br>
> 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. <br>
It doesn't seem to be required that STT_COMMON is used for common symbols:<br>
<br>
```<br>
$ echo 'int foo;' | bin/clang -xc - -c -o /tmp/common.o; bin/llvm-readobj -symbols /tmp/common.o | grep -B1 -A7 foo<br>
  Symbol {<br>
    Name: foo (16)<br>
    Value: 0x4<br>
    Size: 4<br>
    Binding: Global (0x1)<br>
    Type: Object (0x1)<br>
    Other: 0<br>
    Section: Common (0xFFF2)<br>
  }<br>
```<br>
<br>
However, there seems to be a `--elf-stt-common=yes` flag that some GNU tools (including objcopy) support:<br>
<br>
```<br>
$ 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<br>
  Symbol {<br>
    Name: foo (3)<br>
    Value: 0x4<br>
    Size: 4<br>
    Binding: Global (0x1)<br>
    Type: Common (0x5)<br>
    Other: 0<br>
    Section: Common (0xFFF2)<br>
  }<br>
```<br>
<br>
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. <a href="https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Object/ELFTypes.h#L219" rel="noreferrer" target="_blank">https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Object/ELFTypes.h#L219</a>)<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D53782" rel="noreferrer" target="_blank">https://reviews.llvm.org/D53782</a><br>
<br>
<br>
<br>
</blockquote></div></div>