[Bug target/104117] [9,10,11,12 Regression] Darwin ppc64 uses invalid non-PIC address to access constants (in PIC code).

iains at gcc dot gnu.org gcc-bugzilla@gcc.gnu.org
Sun Feb 6 10:34:32 GMT 2022


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104117

--- Comment #15 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Vladimir Makarov from comment #13)
> I think there are two code spots whose pitfalls resulted in the PR.
> 
> The first one is in rs6000.cc::legitimate_lo_sum_address_p which permits
> wrong pic low-sum address.

Ack, 

> Another one is in lra-constraints.cc::process_address_1 which permits put
> wrong low-sum address in reg and use the reg in memory.
> 
> The following patch solves the problem:
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 5404fb18755..306f67f26c4 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -8202,7 +8202,7 @@ legitimate_lo_sum_address_p (machine_mode mode, rtx x,
> int strict)
>      {
>        bool large_toc_ok;
> 
> -      if (DEFAULT_ABI == ABI_V4 && flag_pic)
> +      if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_DARWIN) && flag_pic)
>         return false;
>        /* LRA doesn't use LEGITIMIZE_RELOAD_ADDRESS as it usually calls
>          push_reload from reload pass code.  LEGITIMIZE_RELOAD_ADDRESS

I have a somewhat wider fix which accounts for the case that UNSPEC [
SYMBOL_REF ] MACHOPIC_UNSPEC_OFFSET *is* valid, where SYMBOL_REF is not (but
the change you have made could be sufficient - will try to get that into a
retest soon).

> diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
> index 30d088afbca..998e82be54f 100644
> --- a/gcc/lra-constraints.c
> +++ b/gcc/lra-constraints.c
> @@ -3517,21 +3517,8 @@ process_address_1 (int nop, bool check_only_p,
>                   *ad.inner = gen_rtx_LO_SUM (Pmode, new_reg, addr);
>                   if (!valid_address_p (op, &ad, cn))
>                     {
> -                     /* Try to put lo_sum into register.  */
> -                     insn = emit_insn (gen_rtx_SET
> -                                       (new_reg,
> -                                        gen_rtx_LO_SUM (Pmode, new_reg,
> addr)));
> -                     code = recog_memoized (insn);
> -                     if (code >= 0)
> -                       {
> -                         *ad.inner = new_reg;
> -                         if (!valid_address_p (op, &ad, cn))
> -                           {
> -                             *ad.inner = addr;
> -                             code = -1;
> -                           }
> -                       }
> -
> +                     *ad.inner = addr;
> +                     code = -1;
>                     }
>                 }
>               if (code < 0)
> 
> The patch was successfully tested on x86-64/ppc64 under Linux.

The second part of the patch is the same as what I was testing.

How to proceed on this (testing an LRA change widely enough)?


More information about the Gcc-bugs mailing list