llvm dev - Aug 2018 - GNU objcopy compatibility for --globalize-symbol and --keep-global-symbol (-G)

In attempting to add support for -G/--keep-global-symbol, I played around
with GNU objcopy to see how it handles various situations, especially in
interacting with --globalize-symbol. I'd like to see if anyone has
objections to the behavior proposed in https://reviews.llvm.org/D50589.
Long, gory details below:

These two "global" flags are named similarly, but can have very
different
effects:
* --globalize-symbol makes a symbol global.
 +  According to man pages: "Give symbol symbolname global scoping so that
it is visible outside of the file in which it is defined."
* --keep-global-symbol will make all symbols local *except* for any symbols
specified via --keep-global-symbol.
 + According to man pages: "Keep only symbol symbolname global.  Make all
other symbols local to the file, so that they are not visible externally."

(Note: "--keep-global-symbol LocalSymbol" does not promote it to being
a
global symbol)

However, the actual thing GNU sources for objcopy.c does (simplified to
just relevant bits) is this:

  for (auto sym : symbols) {
    auto flags = sym->flags;
    auto name = sym->name;

    // If flag is global/weak, and --keep-global-symbol includes it,
    // make it local
    if ((flags & (GLOBAL | WEAK)) &&
        (keep_globals.size() != 0 && !list_contains(keep_globals,
name))) {
      sym->flags &= ~(GLOBAL | WEAK);
      sym->flags |= LOCAL;
    }

    // If flag is local and --globalize-symbol is set, make it global
    // Note: checks flags, not sym->flags
    if ((flags & LOCAL) && list_contains(globalize, name)) {
      sym->flags &= ~LOCAL;
      sym->flags |= GLOBAL;
    }
  }

This seems strange for a few reasons:

* Weak symbols are not globalized. The help for --globalize-symbol never
mention it doesn't handle weak symbols, so this seems like a bug in GNU
objcopy.

* If one were to do "--globalize-symbol SomeGlobal --keep-global-symbol
SomeOtherGlobal", you might expect that both SomeGlobal and SomeOtherGlobal
are global in the output file... but it isn't. (Admittedly, this is a weird
command line usage). Because --keep-global-symbol is set and doesn't
include SomeGlobal, SomeGlobal will be demoted to a local symbol. And
because the check to see if we should apply the --globalize-symbol flag
checks "flags" (the original flag set), and not
"sym->flags", it decides
not to do anything, so SomeGlobal remains a local symbol.

The proposed behavior in https://reviews.llvm.org/D50589 is to break from
GNU here, so that --globalize-symbol will always make a symbol global,
including when it's a weak symbol, and including when it isn't included
in
--keep-global-symbol.

(Actually, --globalize-symbol is already implemented and promotes weak
symbols to global, this message is more of a "hey, we're already
different
from GNU, is that OK?" for that part)

Thanks -- Jordan
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180815/8762e516/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4849 bytes
Desc: S/MIME Cryptographic Signature
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180815/8762e516/attachment.bin>

Hi Jordan,

No objections per se, however:

However, the actual thing GNU sources for objcopy.c does (simplified
to
> just relevant bits) is this:
>
>   for (auto sym : symbols) {
>     auto flags = sym->flags;
>     auto name = sym->name;
>
>     // If flag is global/weak, and --keep-global-symbol includes it,
>     // make it local
>     if ((flags & (GLOBAL | WEAK)) &&
>         (keep_globals.size() != 0 && !list_contains(keep_globals,
name))) {
>       sym->flags &= ~(GLOBAL | WEAK);
>       sym->flags |= LOCAL;
>     }
>
>     // If flag is local and --globalize-symbol is set, make it global
>     // Note: checks flags, not sym->flags
>     if ((flags & LOCAL) && list_contains(globalize, name)) {
>       sym->flags &= ~LOCAL;
>       sym->flags |= GLOBAL;
>     }
>   }
>
> This seems strange for a few reasons:
>
> * Weak symbols are not globalized. The help for --globalize-symbol never
> mention it doesn't handle weak symbols, so this seems like a bug in GNU
> objcopy.
>
> * If one were to do "--globalize-symbol SomeGlobal
--keep-global-symbol
> SomeOtherGlobal", you might expect that both SomeGlobal and
SomeOtherGlobal
> are global in the output file... but it isn't. (Admittedly, this is a
weird
> command line usage). Because --keep-global-symbol is set and doesn't
> include SomeGlobal, SomeGlobal will be demoted to a local symbol. And
> because the check to see if we should apply the --globalize-symbol flag
> checks "flags" (the original flag set), and not
"sym->flags", it decides
> not to do anything, so SomeGlobal remains a local symbol.
>
>
I'd probably send a message to the binutils list with this (happy to be
cc'd on it) with "hey, this behavior and the documentation doesn't
match.
Do we know of anything that depends on this behavior?"

Or something like that and see who responds and what they say. :)

Thoughts?

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180904/e831f129/attachment.html>