pc32 -> plt32 conversion and related questions

[H.J. Lu]

On Tue, Jun 15, 2021 at 6:42 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> H.J.,
>
> alongside the question of what non-zero addends mean for certain
> relocation types, I came to notice that this conversion may be
> done in cases where it would better be avoided. I have the draft
> patch below (still lacking a test case and ChangeLog entry), and
> I particularly wonder whether the information propagation
> approach to md_estimate_size_before_relax() is acceptable, or
> whether you have other (better) suggestions.
>
> While playing with this (and in particular the code that I mean
> to convert into a testcase) I came to further notice that there
> is an inconsistency in whether relocations for branches get
> emitted for locally defined symbols:
>
>         .text
>         .global _start, func
> _start:
>         call    func    # gets converted to PLT32 reloc (and would be subject to preemption even when left as PC32), ...
>         jc      func    # ... while this and ...
>         jmp     func    # ... this get resolved locally, unless ...
>
>         # ... this directive is enabled (or -mshared gets passed)
>         #.section .text.lib, "ax", @progbits
> func:
>         ret
>         .type func, @function
>         .size func, .-func
>
> In 2.30 (i.e. before the pc32 -> plt32 conversion) the CALL
> yields a PC32 reloc, so the issue isn't directly related to that
> change. But given the effect -mshared has on JMP and Jcc, do you
> agree that CALL should be handled consistently with JMP/Jcc here?
> Not the least to match the documentation of -mshared:
>
> "On ELF target, the assembler normally optimizes out non-PLT
>  relocations against defined non-weak global branch targets with
>  default visibility."

Yes, CALL should be handled like JMP/Jcc.

> It doesn't mention that there's a difference between different
> kinds of branches.
>
> And then, still in this same context, I did also notice that the
> linker complains about e.g. PC32 relocs when building a shared
> library even when that's a x32 one (suggesting to rebuild with
> -fPIC). 32-bit relocations can't overflow in that ABI, so wouldn't
> such errors better be suppressed there?

It can overflow in x32 since x32 is totally implemented on software
and there is no address wraparound.

> Thanks, Jan
>
> x86-64: restrict PC32 -> PLT32 conversion
>
> Neither non-64-bit code nor uses with a non-zero offset from a symbol
> should be converted to PLT32, as an eventual PLT entry would not express
> what was requested.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -8765,6 +8765,8 @@ output_branch (void)
>        off = 0;
>      }
>
> +  frag_now->tc_frag_data.code64 = flag_code == CODE_64BIT;
> +
>    /* 1 possible extra opcode + 4 byte displacement go in var part.
>       Pass reloc in fr_var.  */
>    frag_var (rs_machine_dependent, 5, i.reloc[0], subtype, sym, off, p);
> @@ -8893,8 +8895,8 @@ output_jump (void)
>      }
>
>  #if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
> -  if (size == 4
> -      && jump_reloc == NO_RELOC
> +  if (flag_code == CODE_64BIT && size == 4
> +      && jump_reloc == NO_RELOC && i.op[0].disps->X_add_number == 0
>        && need_plt32_p (i.op[0].disps->X_add_symbol))
>      jump_reloc = BFD_RELOC_X86_64_PLT32;
>  #endif
> @@ -12191,7 +12193,8 @@ md_estimate_size_before_relax (fragS *fr
>        else if (size == 2)
>         reloc_type = BFD_RELOC_16_PCREL;
>  #if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
> -      else if (need_plt32_p (fragP->fr_symbol))
> +      else if (fragP->tc_frag_data.code64 && fragP->fr_offset == 0
> +              && need_plt32_p (fragP->fr_symbol))
>         reloc_type = BFD_RELOC_X86_64_PLT32;
>  #endif
>        else
> --- a/gas/config/tc-i386.h
> +++ b/gas/config/tc-i386.h
> @@ -280,6 +280,7 @@ struct i386_tc_frag_data
>    unsigned int mf_type : 3;
>    unsigned int classified : 1;
>    unsigned int branch_type : 3;
> +  unsigned int code64 : 1; /* Only set by output_branch for now.  */
>  };
>
>  /* We need to emit the right NOP pattern in .align frags.  This is
>


-- 
H.J.