This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: RFC: Should AArch64 *_NC relocs complain on overflow ?
- From: "Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>
- To: Nick Clifton <nickc at redhat dot com>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, Jiong Wang <jiong dot wang at foss dot arm dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Mon, 8 Feb 2016 13:13:01 +0000
- Subject: Re: RFC: Should AArch64 *_NC relocs complain on overflow ?
- Authentication-results: sourceware.org; auth=none
- References: <87a8nb7bk8 dot fsf at redhat dot com> <ADC64823-9296-45C9-BCED-FFDC03CA29BB at arm dot com> <56B88C97 dot 6090308 at redhat dot com>
On 08/02/16 12:39, Nick Clifton wrote:
> Hi Marcus, Hi Jiong,
>
>> NC relocations do complain if the relocation requires some minimum alignment
>> and the final relocated value does not meet that requirement. Consider, for
>> example, instructions with an implicit scaling of an address offset,
>> relocations used for those instruction, including the NC form will gripe if
>> the offset being inserted into the instruction is not a multiple of the
>> implicit scaling implied.
>
> OK that makes sense,
>
> It would be nice if we could generate a more helpful warning message for the user.
> What do you think of a patch like the one below ? With this applied we get output
> like the following:
>
> 1.c:(.text+0x4): relocation truncated to fit: R_AARCH64_LDST32_ABS_LO12_NC against symbol `var_2' defined in COMMON section in tmpdir/dump1.o
> 1.c:(.text+0x4): warning: Possibly the symbol is being referenced via a differently aligned type to its declaration>, no expected output
>
> Cheers
> Nick
>
>
> aarch64-reloc-overflow.patch
>
>
> diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
> index 292470df..be0ddb5 100644
> --- a/bfd/elfnn-aarch64.c
> +++ b/bfd/elfnn-aarch64.c
> @@ -6405,10 +6405,6 @@ elfNN_aarch64_relocate_section (bfd *output_bfd,
> break;
> }
>
> - if (!save_addend)
> - addend = 0;
> -
> -
> /* Dynamic relocs are not propagated for SEC_DEBUGGING sections
> because such sections are not SEC_ALLOC and thus ld.so will
> not process them. */
> @@ -6448,6 +6444,31 @@ elfNN_aarch64_relocate_section (bfd *output_bfd,
> name, input_bfd, input_section, rel->r_offset);
> return FALSE;
> }
> + /* Overflow can occur when a variable is referenced with a type
> + that has a larger alignment than the type with which it was
> + declared. eg:
> + file1.c: extern int foo; int a (void) { return foo; }
> + file2.c: char bar, foo, baz;
> + If the variable is placed into a data section at an offset
> + that is incompatible with the larger alignment requirement
> + overflow will occur. (Strictly speaking this is not overflow
> + but rather an alignment problem, but the bfd_reloc_ error
> + enum does not have a value to cover that situation).
> +
> + Try to catch this situation here and provide a more helpful
> + error message to the user. */
> + if (addend & ((1 << howto->rightshift) - 1)
> + /* FIXME: Are we testing all of the appropriate reloc
> + types here ? */
> + && (real_r_type == BFD_RELOC_AARCH64_LDST16_LO12
> + || real_r_type == BFD_RELOC_AARCH64_LDST32_LO12
> + || real_r_type == BFD_RELOC_AARCH64_LDST64_LO12
> + || real_r_type == BFD_RELOC_AARCH64_LDST128_LO12))
Those are checking relocations (don't have _NC in the name), so I'd
expect that they already check alignment as part of their standard
overflow test (if they don't that's probably a different bug).
For the _NC versions, those would probably the most common, but there is
quite a long list of relocations that have alignment checks. It would
be better if we could have an alignment check field in the standard
RELOC descriptor field that is distinct from the overflow check test.
> + {
> + info->callbacks->warning
> + (info, _("Possibly the symbol is being referenced via a differently aligned type to its declaration"),
> + name, input_bfd, input_section, rel->r_offset);
> + }
> break;
>
> case bfd_reloc_undefined:
> @@ -6482,6 +6503,9 @@ elfNN_aarch64_relocate_section (bfd *output_bfd,
> break;
> }
> }
> +
> + if (!save_addend)
> + addend = 0;
> }
>
> return TRUE;
> diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
> index 939539e..d0b33cf 100644
> --- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
> +++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
> @@ -159,6 +159,8 @@ run_dump_test "emit-relocs-537"
> run_dump_test "emit-relocs-537-overflow"
> run_dump_test "emit-relocs-538"
>
> +run_dump_test "reloc-overflow-bad"
> +
> # test addend correctness when --emit-relocs specified for non-relocatable obj.
> run_dump_test "emit-relocs-local-addend"
> # test addend correctness when -r specified.
> --- /dev/null 2016-02-08 07:59:08.480962732 +0000
> +++ ld/testsuite/ld-aarch64/reloc-overflow-bad.d 2016-02-08 12:34:55.696124085 +0000
> @@ -0,0 +1,7 @@
> +#source: reloc-overflow-1.s
> +#source: reloc-overflow-2.s
> +#ld: -e0
> +#error: .*Possibly the symbol.*
> +
> +
> +
> --- /dev/null 2016-02-08 07:59:08.480962732 +0000
> +++ ld/testsuite/ld-aarch64/reloc-overflow-1.s 2016-02-08 12:30:59.584834873 +0000
> @@ -0,0 +1,14 @@
> + .file "1.c"
> + .text
> + .align 2
> + .p2align 3,,7
> + .global dec
> + .arch armv8-a+fp+simd
> + //.tune generic
> + .type dec, %function
> +dec:
> + adrp x0, var_2
> + ldr w0, [x0, #:lo12:var_2]
> + ret
> + .size dec, .-dec
> + .ident "GCC: (GNU) 6.0.0 20160208 (experimental) [trunk revision 233206]"
> --- /dev/null 2016-02-08 07:59:08.480962732 +0000
> +++ ld/testsuite/ld-aarch64/reloc-overflow-2.s 2016-02-08 12:31:36.679037435 +0000
> @@ -0,0 +1,5 @@
> + .file "2.c"
> + .comm var_3,1,1
> + .comm var_2,1,1
> + .comm var_1,1,1
> + .ident "GCC: (GNU) 6.0.0 20160208 (experimental) [trunk revision 233206]"
>