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

[Jake Ehrlich via llvm-commits]

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.

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.

On Mon, Oct 29, 2018, 3:21 PM Jordan Rupprecht via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181030/e793b943/attachment.html>