Re: [PATCH] Fix default_binds_local_p_2 for extern protected data

[Szabolcs Nagy <szabolcs dot nagy at arm dot com>]

On 30/09/15 20:23, Andreas Krebbel wrote:
> On 09/30/2015 06:21 PM, Szabolcs Nagy wrote:
> > On 30/09/15 14:47, Bernd Schmidt wrote:
> > > On 09/17/2015 11:15 AM, Szabolcs Nagy wrote:
> > > > ping 2.
> > > >
> > > > this patch is needed for working visibility ("protected")
> > > > attribute for extern data on targets using default_binds_local_p_2.
> > > > https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01871.html
> > >
> > > I hesitate to review this one since I don't think I understand the
> > > issues on the various affected arches well enough. It looks like Jakub
> > > had some input on the earlier changes, maybe he could take a look? Or
> > > maybe rth knows best. Adding Ccs.
> > >
> > > It would help to have examples of code generation demonstrating the
> > > problem and how you would solve it. Input from the s390 maintainers
> > > whether this is correct for their port would also be appreciated.
>
> We are having the same problem on S/390. I think the GCC change is correct for S/390 as well.
>
> -Andreas-

i think the approvals of arm and aarch64 maintainers
are needed to apply this fix for pr target/66912.

(only s390, arm and aarch64 use this predicate.)

> > >
> >
> > consider the TU
> >
> >     __attribute__((visibility("protected"))) int n;
> >
> >     int f () { return n; }
> >
> > if n "binds_local" then gcc -O -fpic -S is like
> >
> >           .text
> >           .align  2
> >           .global f
> >           .arch armv8-a+fp+simd
> >           .type   f, %function
> > f:
> >           adrp    x0, n
> >           ldr     w0, [x0, #:lo12:n]
> >           ret
> >           .size   f, .-f
> >           .protected      n
> >           .comm   n,4,4
> >
> > so 'n' is a direct reference, not accessed through
> > the GOT ('n' will be in the .bss of the dso).
> > this is the current behavior.
> >
> > if i remove the protected visibility attribute
> > then the access goes through GOT:
> >
> >           .text
> >           .align  2
> >           .global f
> >           .arch armv8-a+fp+simd
> >           .type   f, %function
> > f:
> >           adrp    x0, _GLOBAL_OFFSET_TABLE_
> >           ldr     x0, [x0, #:gotpage_lo15:n]
> >           ldr     w0, [x0]
> >           ret
> >           .size   f, .-f
> >           .comm   n,4,4
> >
> > protected visibility means the definition cannot
> > be overridden by another module, but it should
> > still allow extern references.
> >
> > if the main module references such an object then
> > (as an implementation detail) it may use copy
> > relocation against it, which places 'n' in the
> > main module and the dynamic linker should make
> > sure that references to 'n' point there.
> >
> > this is only possible if references to 'n' go
> > through the GOT (i.e. it should not be "binds_local").