Re: [PATCH] Fix default_binds_local_p_2 for extern protected data

[Szabolcs Nagy <szabolcs.nagy@arm.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").
>
>
>
>
>