Re: RFC: Should AArch64 *_NC relocs complain on overflow ?

[Jiong Wang <jiong dot wang at foss dot arm dot com>]

On 08/02/16 10:15, Nick Clifton wrote:
> Hi Richard, Hi Marcus,
>
>    Should AArch64 *_NC relocs complain if they encounter an overflow ?  I
>    thought that the _NC suffix meant "no complain", but when I tested a
>    local patch[1] to address a problem related to these relocs, I found a
>    new failure in the linker testsuite:
>
>      FAIL: ld-aarch64/emit-relocs-286-bad
>
>    This test appears to expect some "relocation truncated to fit" warning
>    messages from the R_AARCH64_LDST64_ABS_LO12_NC reloc.  Is the test
>    wrong, or am I misunderstanding about when *_NC should generate
>    overflow warnings ?
>
> Cheers
>    Nick
>
> [1] This is the patch that I am testing:
>
> diff --git a/bfd/elfxx-aarch64.c b/bfd/elfxx-aarch64.c
> index 498171c..7d8fa3b 100644
> --- a/bfd/elfxx-aarch64.c
> +++ b/bfd/elfxx-aarch64.c
> @@ -224,7 +224,8 @@ _bfd_aarch64_elf_put_addend (bfd *abfd,
>       case BFD_RELOC_AARCH64_LD_LO19_PCREL:
>       case BFD_RELOC_AARCH64_TLSDESC_LD_PREL19:
>       case BFD_RELOC_AARCH64_TLSIE_LD_GOTTPREL_PREL19:
> -      if (old_addend & ((1 << howto->rightshift) - 1))
> +      if (howto->complain_on_overflow != complain_overflow_dont
> +	  && (old_addend & ((1 << howto->rightshift) - 1)))
>   	return bfd_reloc_overflow;
>         contents = reencode_ld_lit_ofs_19 (contents, addend);
>         break;
> @@ -285,7 +286,8 @@ _bfd_aarch64_elf_put_addend (bfd *abfd,
>       case BFD_RELOC_AARCH64_TLSLD_LDST64_DTPREL_LO12_NC:
>       case BFD_RELOC_AARCH64_TLSLD_LDST8_DTPREL_LO12:
>       case BFD_RELOC_AARCH64_TLSLD_LDST8_DTPREL_LO12_NC:
> -      if (old_addend & ((1 << howto->rightshift) - 1))
> +      if (howto->complain_on_overflow != complain_overflow_dont
> +	  && (old_addend & ((1 << howto->rightshift) - 1)))
>   	return bfd_reloc_overflow;
>         /* Used for ldr*|str* rt, [rn, #uimm12] to provide the low order
>            12 bits of the page offset following BFD_RELOC_AARCH64_ADR_HI21_PCREL
>
> This is to address a problem building the sipp package on AArch64
> Fedora:
>
>    https://bugzilla.redhat.com/show_bug.cgi?id=1300543
>
> Part of the problem appears to be related to the fact that a global
> variable is declared with different types in different source files, but
> the same code compliles without problems on other architectures, and I
> was intrigued as to why the AArch64 was generating warnings for "no
> complain" relocs...

Hi Nick,

 This looks to me is a C code bug.

 It can be reproduced by the following simple testcase.

gcc -O2 -o bug.out 1.c 2.c 3.c --save-temps

1.c
===
extern char var_2 ;

char
cal ()
{
  return var_2;
}

2.c
===
extern int var_2 ;

int
dec (int a)
{
  return var_2;
}

3.c
===
char var_1;
char var_2;
char var_3;

int
main ()
{
  return 0;
}


 The variable type is inconsistent between different modules, for
module A, it's assume to be a four byte type, for module B, it's
assume to be a one byte type, then finally, relocation types require
different alignments are generated. for example, LDST8_ABS_LO12_NC
for one byte, LDST32_ABS_LO12_NC for four byte.

 Then if the underline data is really one byte, then it the address
may not be 4byte aligned, thus such error will happen for four byte
relocation.

 I am afraid your patch will hiding such alignment errors and evetually
tolerating wrong binary generated if the hardware don't support unaligned
load/store, because the place you modified is actually checking unalignment
issue.

 On AArch64, overflow message is given for unalignment issue as well I 
guess
it's because there is no proper enumeration in bfd_reloc_status, maybe a
bfd_reloc_unaligned should be added.

 IMO, the code itself should be fixed. like this one:

   https://github.com/sipcapture/sipgrep/issues/21