On 26/09/2024 19:21, Ramana Radhakrishnan wrote:
> On Mon, Mar 4, 2024 at 1:43 PM Fangrui Song <mask...@google.com> wrote:
>>
>> From: Fangrui Song <mask...@gcc.gnu.org>
>>
>> -fno-pic -mfdpic generated code is like regular -fno-pic, not suitable
>> for FDPIC (absolute addressing for symbol references and no function
>> descriptor).  The sh port simply upgrades -fno-pic to -fpie by setting
>> flag_pic.  Let's follow suit.
>>
>> Link: 
>> https://inbox.sourceware.org/gcc-patches/20150913165303.gc17...@brightrain.aerifal.cx/
>>
>> gcc/ChangeLog:
>>
>>         * config/arm/arm.cc (arm_option_override): Set flag_pic if
>>           TARGET_FDPIC.
>>
>> gcc/testsuite/ChangeLog:
>>
>>         * gcc.target/arm/fdpic-pie.c: New test.
>> ---
>>  gcc/config/arm/arm.cc                    |  6 +++++
>>  gcc/testsuite/gcc.target/arm/fdpic-pie.c | 30 ++++++++++++++++++++++++
>>  2 files changed, 36 insertions(+)
>>  create mode 100644 gcc/testsuite/gcc.target/arm/fdpic-pie.c
>>
>> diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
>> index 1cd69268ee9..f2fd3cce48c 100644
>> --- a/gcc/config/arm/arm.cc
>> +++ b/gcc/config/arm/arm.cc
>> @@ -3682,6 +3682,12 @@ arm_option_override (void)
>>        arm_pic_register = FDPIC_REGNUM;
>>        if (TARGET_THUMB1)
>>         sorry ("FDPIC mode is not supported in Thumb-1 mode");
>> +
>> +      /* FDPIC code is a special form of PIC, and the vast majority of code
>> +        generation constraints that apply to PIC also apply to FDPIC, so we
>> +         set flag_pic to avoid the need to check TARGET_FDPIC everywhere
>> +         flag_pic is checked. */
>> +      flag_pic = 2;
>>      }
> 
> Been a while since I looked at this stuff but should this not be
> flag_pie being set rather than flag_pic here if the expectation is to
> turn -fno-PIC -mfdpic into fPIE ?

-fPIE implies -fPIC, but has the added implication that any definition of an 
object we see is the one true definition and cannot be pre-empted during 
loading (in a shared library, a definition of X may be pre-empted by another 
definition of X in either the application itself or another shared library that 
was loaded first).

Part of the confusion comes from the manual, though:

Select the FDPIC ABI, which uses 64-bit function descriptors to
represent pointers to functions.  When the compiler is configured for
@code{arm-*-uclinuxfdpiceabi} targets, this option is on by default
and implies @option{-fPIE} if none of the PIC/PIE-related options is
provided.  On other targets, it only enables the FDPIC-specific code
generation features, and the user should explicitly provide the
PIC/PIE-related options as needed.

Which conflates things relating to the option flag and the compiler 
configuration.  I think that needs clearing up as well.  Something like

Select the FDPIC ABI, which uses 64-bit function descriptors to
represent pointers to functions.  @option{-mfdpic} implies @option{-fPIC}.

When the compiler is configured for @code{arm-*-uclinuxfdpiceabi} targets, 
this option is on by default and the compiler defaults to @option{-fPIE}, 
unless @option{-fPIC} is explicitly specified.

might cover it, but I'm not sure I've fully untangled the web of option 
permutations here.  Perhaps someone could tabulate the expected options 
somewhere for clarity.

The other option would be to error if flag_pic is not set, when -mfdpic is set, 
which would force the user to be explicit as to which pic options they want 
(technically the explicit combination -mno-pic -mfdpic has no meaning).

R.

> 
> 
>>
>>    if (arm_pic_register_string != NULL)
>> diff --git a/gcc/testsuite/gcc.target/arm/fdpic-pie.c 
>> b/gcc/testsuite/gcc.target/arm/fdpic-pie.c
>> new file mode 100644
>> index 00000000000..909db8bce74
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/fdpic-pie.c
>> @@ -0,0 +1,30 @@
>> +// { dg-do compile }
>> +// { dg-options "-O2 -fno-pic -mfdpic" }
>> +// { dg-skip-if "-mpure-code and -fPIC incompatible" { *-*-* } { 
>> "-mpure-code" } }
>> +
>> +__attribute__((visibility("hidden"))) void hidden_fun(void);
>> +void fun(void);
>> +__attribute__((visibility("hidden"))) extern int hidden_var;
>> +extern int var;
>> +__attribute__((visibility("hidden"))) const int ro_hidden_var = 42;
>> +
>> +// { dg-final { scan-assembler "hidden_fun\\(GOTOFFFUNCDESC\\)" } }
>> +void *addr_hidden_fun(void) { return hidden_fun; }
>> +
>> +// { dg-final { scan-assembler "fun\\(GOTFUNCDESC\\)" } }
>> +void *addr_fun(void) { return fun; }
>> +
>> +// { dg-final { scan-assembler "hidden_var\\(GOT\\)" } }
>> +void *addr_hidden_var(void) { return &hidden_var; }
>> +
>> +// { dg-final { scan-assembler "var\\(GOT\\)" } }
>> +void *addr_var(void) { return &var; }
>> +
>> +// { dg-final { scan-assembler ".LANCHOR0\\(GOT\\)" } }
>> +const int *addr_ro_hidden_var(void) { return &ro_hidden_var; }
>> +
>> +// { dg-final { scan-assembler "hidden_var\\(GOT\\)" } }
>> +int read_hidden_var(void) { return hidden_var; }
>> +
>> +// { dg-final { scan-assembler "var\\(GOT\\)" } }
>> +int read_var(void) { return var; }
>> --
>> 2.44.0.rc1.240.g4c46232300-goog
>>

Reply via email to