[cfe-dev] different gcc/clang behavior for Internal visibility

[Andrii Nakryiko via cfe-dev]

On Tue, May 11, 2021 at 11:05 AM Fāng-ruì Sòng <maskray at google.com> wrote:
>
> > rnk: IIRC, internal visibility is a guarantee that a function is not escaped and called indirectly from another DSO. This allows the compiler to avoid setting up a TOC or PIC base register in the prologue for architectures that have one. This was notably expensive for x86_32 because of the call/pop code sequence required to materialize EIP. There are other RISC-y architectures (PPC? 64? not sure) with PIC base registers that could benefit from internal visibility support, but I think most of them are considered legacy architectures at this point, so it's not top priority.
>
> I vaguely recall that I have seen something similar to "a function is
> not escaped", but I just searched generic-abi and could not find any
> proof...
>
> I just checked i386 and PowerPC64 ELFv2 ABIs - they don't define STV_INTERNAL.
>
> On Tue, May 11, 2021 at 10:45 AM Andrii Nakryiko
> <andrii.nakryiko at gmail.com> wrote:
> >
> > On Mon, May 10, 2021 at 12:52 PM Fangrui Song <maskray at google.com> wrote:
> > >
> > > On 2021-05-10, Y Song via cfe-dev wrote:
> > > >Hi,
> > > >
> > > >The bpf linker project tries to explore to use INTERNAL visibility as in
> > > >   https://lore.kernel.org/bpf/20210507054119.270888-1-andrii@kernel.org/
> > > >
> > > >But we found clang actually changed user "internal" visibility to "hidden".
> > > >For example, I have the following example,
> > > >
> > > >$ cat t.c
> > > >int __attribute__((visibility("internal"))) foo() { return 0; }
> > > >$ clang -c t.c && llvm-readelf -s t.o | grep foo
> > > >     3: 0000000000000000     8 FUNC    GLOBAL HIDDEN      2 foo
> > > >$ gcc -c t.c && llvm-readelf -s t.o | grep foo
> > > >     8: 0000000000000000    11 FUNC    GLOBAL INTERNAL    1 foo
> > > >$
> > > >
> > > >Looks like this is caused by clang Attr.td,
> > > >
> > > >diff --git a/clang/include/clang/Basic/Attr.td
> > > >b/clang/include/clang/Basic/Attr.td
> > > >index 5e04f32187cd..4559a1bcfe42 100644
> > > >--- a/clang/include/clang/Basic/Attr.td
> > > >+++ b/clang/include/clang/Basic/Attr.td
> > > >@@ -2776,7 +2776,7 @@ def Visibility : InheritableAttr {
> > > >   let Spellings = [GCC<"visibility">];
> > > >   let Args = [EnumArgument<"Visibility", "VisibilityType",
> > > >                            ["default", "hidden", "internal", "protected"],
> > > >-                           ["Default", "Hidden", "Hidden", "Protected"]>];
> > > >+                           ["Default", "Hidden", "Internal", "Protected"]>];
> > > >   let MeaningfulToClassTemplateDefinition = 1;
> > > >   let Documentation = [Undocumented];
> > > > }
> > > >@@ -2786,7 +2786,7 @@ def TypeVisibility : InheritableAttr {
> > > >   let Spellings = [Clang<"type_visibility">];
> > > >   let Args = [EnumArgument<"Visibility", "VisibilityType",
> > > >                            ["default", "hidden", "internal", "protected"],
> > > >-                           ["Default", "Hidden", "Hidden", "Protected"]>];
> > > >+                           ["Default", "Hidden", "Internal", "Protected"]>];
> > > > //  let Subjects = [Tag, ObjCInterface, Namespace];
> > > >   let Documentation = [Undocumented];
> > > > }
> > > >
> > > >One of early commits,
> > > >
> > > >commit 570024a8d9b4a4aa4a35f077a0a65003dc7b71fe
> > > >Author: Eli Friedman <eli.friedman at gmail.com>
> > > >Date:   Thu Aug 5 06:57:20 2010 +0000
> > > >
> > > >    Implement #pragma GCC visibility.
> > > >
> > > >    llvm-svn: 110315
> > > >
> > > >I see
> > > >
> > > >+    else if (VisType->isStr("internal"))
> > > >+      type = VisibilityAttr::HiddenVisibility; // FIXME
> > > >
> > > >Do we have any plan to support Internal visibility?
> > >
> > > I don't think there is value supporting STV_INTERNAL.
> > >
> > > STV_INTERNAL as we see today in the ELF specification was requested by SGI.
> > >
> > > > The meaning of this visibility attribute may be defined by processor
> > > > supplements to further constrain hidden symbols. A processor
> > > > supplement's definition should be such that generic tools can safely
> > > > treat internal symbols as hidden.  An internal symbol contained in a
> > > > relocatable object must be either removed or converted to STB_LOCAL
> > > > binding by the link-editor when the relocatable object is included in an
> > > > executable file or shared object.
> > >
> > > I recall from reading somewhere that it is used by its propritery
> > > compiler for some LTO like optimizations. STV_INTERNAL is identical to
> > > STV_HIDDEN in GNU/Solaris (and likely HP-UX).
> > >
> > > For the Linux kernel, my suggestion is to just use __attribute__((visibility("hidden"))).
> > > Then there will be no confusion that "internal" translates to STV_HIDDEN.
> >
> > We are considering using both STV_HIDDEN and STV_INTERNAL for BPF
> > target and we want to be able to distinguish between the two. Given
> > ELF defines all four (STV_DEFAULT, STV_PROTECTED, STV_HIDDEN,
> > STV_INTERNAL), it would be great to be able to actually express
> > STV_INTERNAL. Are there any specific problems with passing-through
> > STV_INTERNAL down to ELF symbol visibility?
>
> Since the ELF specification delegates the further definition of
> STV_INTERNAL to processor supplements, BPF as a processor has the
> right to further define STV_INTERNAL.
> I'd like to know more arguments favoring a distinguished
> STV_INTERNAL/STV_HIDDEN, since the two bits are ubiquitously
> equivalent in GNU ABI and Solaris (and likely HP-UX; only SGI does
> something different but it is difficult to find the evidence now).
>

It's quite BPF specific, but you can check this patch with some
discussion around STV_HIDDEN vs STV_INTERNAL.\

  [0] https://lore.kernel.org/bpf/20210507054119.270888-8-andrii@kernel.org/

>
> --
> 宋方睿