public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] bfd/elfnn-aarch64.c: Fix calculation of DT_RELASZ
@ 2014-03-20 15:46 Will Newton
  2014-03-20 17:57 ` Jiong Wang
  2014-03-25  7:39 ` Marcus Shawcroft
  0 siblings, 2 replies; 6+ messages in thread
From: Will Newton @ 2014-03-20 15:46 UTC (permalink / raw)
  To: binutils

The current code subtracts the size of the output section containing
relplt from RELASZ. In some cases this will be the same output
section as the dynamic relocs causing a value of zero to be output.
Calculating the size from input sections seems to make more sense.

bfd/ChangeLog:

2014-03-20  Will Newton  <will.newton@linaro.org>

	 * elfnn-aarch64.c (elfNN_aarch64_finish_dynamic_sections):
	 Set value of DT_PLTRELSZ and DT_RELASZ based on the size
	 of input sections rather than output sections.

ld/testsuite/ChangeLog:

2014-03-20  Will Newton  <will.newton@linaro.org>

	 * ld-aarch64/aarch64-elf.exp: Add relasz dump test.
	 * ld-aarch64/relasz.d: New file.
	 * ld-aarch64/relasz.s: Likewise.
---
 bfd/elfnn-aarch64.c                     |  4 ++--
 ld/testsuite/ld-aarch64/aarch64-elf.exp |  2 ++
 ld/testsuite/ld-aarch64/relasz.d        | 18 ++++++++++++++++++
 ld/testsuite/ld-aarch64/relasz.s        |  9 +++++++++
 4 files changed, 31 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-aarch64/relasz.d
 create mode 100644 ld/testsuite/ld-aarch64/relasz.s

Changes in v2:
 - Add testcase

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index c2adcc9..638938d 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -6993,7 +6993,7 @@ elfNN_aarch64_finish_dynamic_sections (bfd *output_bfd,
 	      break;
 
 	    case DT_PLTRELSZ:
-	      s = htab->root.srelplt->output_section;
+	      s = htab->root.srelplt;
 	      dyn.d_un.d_val = s->size;
 	      break;
 
@@ -7007,7 +7007,7 @@ elfNN_aarch64_finish_dynamic_sections (bfd *output_bfd,
 		 about changing the DT_RELA entry.  */
 	      if (htab->root.srelplt != NULL)
 		{
-		  s = htab->root.srelplt->output_section;
+		  s = htab->root.srelplt;
 		  dyn.d_un.d_val -= s->size;
 		}
 	      break;
diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
index 32cf73a..845ea20 100644
--- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
+++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
@@ -157,3 +157,5 @@ run_dump_test "ifunc-19b"
 run_dump_test "ifunc-20"
 run_dump_test "ifunc-21"
 run_dump_test "ifunc-22"
+
+run_dump_test "relasz"
diff --git a/ld/testsuite/ld-aarch64/relasz.d b/ld/testsuite/ld-aarch64/relasz.d
new file mode 100644
index 0000000..5cc5595
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/relasz.d
@@ -0,0 +1,18 @@
+#source: relasz.s
+#ld: -shared -Taarch64.ld
+#readelf: -d
+# Check that the RELASZ section has the correct size even if we are
+# using a non-default linker script that merges .rela.dyn and .rela.plt
+# in the output.
+
+Dynamic section at offset 0x[0-9a-f]+ contains 9 entries:
+  Tag        Type                         Name/Value
+ 0x0000000000000004 \(HASH\)               0x[0-9a-f]+
+ 0x0000000000000005 \(STRTAB\)             0x[0-9a-f]+
+ 0x0000000000000006 \(SYMTAB\)             0x[0-9a-f]+
+ 0x000000000000000a \(STRSZ\)              [0-9]+ \(bytes\)
+ 0x000000000000000b \(SYMENT\)             [0-9]+ \(bytes\)
+ 0x0000000000000007 \(RELA\)               0x[0-9a-f]+
+ 0x0000000000000008 \(RELASZ\)             24 \(bytes\)
+ 0x0000000000000009 \(RELAENT\)            24 \(bytes\)
+ 0x0000000000000000 \(NULL\)               0x0
diff --git a/ld/testsuite/ld-aarch64/relasz.s b/ld/testsuite/ld-aarch64/relasz.s
new file mode 100644
index 0000000..8c7f891
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/relasz.s
@@ -0,0 +1,9 @@
+	.text
+	.global	func
+	.type	func, %function
+func:
+	adrp	x0, :got:foo
+	ldr	x0, [x0, #:got_lo12:foo]
+	ldr	w0, [x0]
+	ret
+	.size	func, .-func
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] bfd/elfnn-aarch64.c: Fix calculation of DT_RELASZ
  2014-03-20 15:46 [PATCH v2] bfd/elfnn-aarch64.c: Fix calculation of DT_RELASZ Will Newton
@ 2014-03-20 17:57 ` Jiong Wang
  2014-03-24  8:49   ` Will Newton
  2014-03-25  7:39 ` Marcus Shawcroft
  1 sibling, 1 reply; 6+ messages in thread
From: Jiong Wang @ 2014-03-20 17:57 UTC (permalink / raw)
  To: Will Newton, binutils

On 20/03/14 15:46, Will Newton wrote:
> The current code subtracts the size of the output section containing
> relplt from RELASZ. In some cases this will be the same output
> section

Hi Will,

   I guess you can only trigger this "same output section" issue when 
using "ld-aarch64/aarch64.ld", right?

--
Jiong

> as the dynamic relocs causing a value of zero to be output.
> Calculating the size from input sections seems to make more sense.
>
> bfd/ChangeLog:
>
> 2014-03-20  Will Newton  <will.newton@linaro.org>
>
> 	 * elfnn-aarch64.c (elfNN_aarch64_finish_dynamic_sections):
> 	 Set value of DT_PLTRELSZ and DT_RELASZ based on the size
> 	 of input sections rather than output sections.
>
> ld/testsuite/ChangeLog:
>
> 2014-03-20  Will Newton  <will.newton@linaro.org>
>
> 	 * ld-aarch64/aarch64-elf.exp: Add relasz dump test.
> 	 * ld-aarch64/relasz.d: New file.
> 	 * ld-aarch64/relasz.s: Likewise.
> ---
>   bfd/elfnn-aarch64.c                     |  4 ++--
>   ld/testsuite/ld-aarch64/aarch64-elf.exp |  2 ++
>   ld/testsuite/ld-aarch64/relasz.d        | 18 ++++++++++++++++++
>   ld/testsuite/ld-aarch64/relasz.s        |  9 +++++++++
>   4 files changed, 31 insertions(+), 2 deletions(-)
>   create mode 100644 ld/testsuite/ld-aarch64/relasz.d
>   create mode 100644 ld/testsuite/ld-aarch64/relasz.s
>
> Changes in v2:
>   - Add testcase
>
> diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
> index c2adcc9..638938d 100644
> --- a/bfd/elfnn-aarch64.c
> +++ b/bfd/elfnn-aarch64.c
> @@ -6993,7 +6993,7 @@ elfNN_aarch64_finish_dynamic_sections (bfd *output_bfd,
>   	      break;
>   
>   	    case DT_PLTRELSZ:
> -	      s = htab->root.srelplt->output_section;
> +	      s = htab->root.srelplt;
>   	      dyn.d_un.d_val = s->size;
>   	      break;
>   
> @@ -7007,7 +7007,7 @@ elfNN_aarch64_finish_dynamic_sections (bfd *output_bfd,
>   		 about changing the DT_RELA entry.  */
>   	      if (htab->root.srelplt != NULL)
>   		{
> -		  s = htab->root.srelplt->output_section;
> +		  s = htab->root.srelplt;
>   		  dyn.d_un.d_val -= s->size;
>   		}
>   	      break;
> diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
> index 32cf73a..845ea20 100644
> --- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
> +++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
> @@ -157,3 +157,5 @@ run_dump_test "ifunc-19b"
>   run_dump_test "ifunc-20"
>   run_dump_test "ifunc-21"
>   run_dump_test "ifunc-22"
> +
> +run_dump_test "relasz"
> diff --git a/ld/testsuite/ld-aarch64/relasz.d b/ld/testsuite/ld-aarch64/relasz.d
> new file mode 100644
> index 0000000..5cc5595
> --- /dev/null
> +++ b/ld/testsuite/ld-aarch64/relasz.d
> @@ -0,0 +1,18 @@
> +#source: relasz.s
> +#ld: -shared -Taarch64.ld
> +#readelf: -d
> +# Check that the RELASZ section has the correct size even if we are
> +# using a non-default linker script that merges .rela.dyn and .rela.plt
> +# in the output.
> +
> +Dynamic section at offset 0x[0-9a-f]+ contains 9 entries:
> +  Tag        Type                         Name/Value
> + 0x0000000000000004 \(HASH\)               0x[0-9a-f]+
> + 0x0000000000000005 \(STRTAB\)             0x[0-9a-f]+
> + 0x0000000000000006 \(SYMTAB\)             0x[0-9a-f]+
> + 0x000000000000000a \(STRSZ\)              [0-9]+ \(bytes\)
> + 0x000000000000000b \(SYMENT\)             [0-9]+ \(bytes\)
> + 0x0000000000000007 \(RELA\)               0x[0-9a-f]+
> + 0x0000000000000008 \(RELASZ\)             24 \(bytes\)
> + 0x0000000000000009 \(RELAENT\)            24 \(bytes\)
> + 0x0000000000000000 \(NULL\)               0x0
> diff --git a/ld/testsuite/ld-aarch64/relasz.s b/ld/testsuite/ld-aarch64/relasz.s
> new file mode 100644
> index 0000000..8c7f891
> --- /dev/null
> +++ b/ld/testsuite/ld-aarch64/relasz.s
> @@ -0,0 +1,9 @@
> +	.text
> +	.global	func
> +	.type	func, %function
> +func:
> +	adrp	x0, :got:foo
> +	ldr	x0, [x0, #:got_lo12:foo]
> +	ldr	w0, [x0]
> +	ret
> +	.size	func, .-func

-- 
Jiong


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] bfd/elfnn-aarch64.c: Fix calculation of DT_RELASZ
  2014-03-20 17:57 ` Jiong Wang
@ 2014-03-24  8:49   ` Will Newton
  2014-03-24 12:26     ` Marcus Shawcroft
  0 siblings, 1 reply; 6+ messages in thread
From: Will Newton @ 2014-03-24  8:49 UTC (permalink / raw)
  To: Jiong Wang; +Cc: binutils

On 20 March 2014 17:57, Jiong Wang <jiong.wang@arm.com> wrote:
> On 20/03/14 15:46, Will Newton wrote:
>>
>> The current code subtracts the size of the output section containing
>> relplt from RELASZ. In some cases this will be the same output
>> section
>
>
> Hi Will,
>
>   I guess you can only trigger this "same output section" issue when using
> "ld-aarch64/aarch64.ld", right?

Yes, it requires a custom linker script. The problem arose building
OSv for aarch64:

https://github.com/cloudius-systems/osv

You could say the linker script is incorrect not to lay out the
dynamic reloc sections explicitly but it seems counter intuitive
behaviour to set RELASZ to zero when it clearly isn't.

A number of ports base the size on the size of the output sections:

lm32
m32r
nds32
sh-32
xtensa
s390-64
x86_64

But the rest use the size of input sections.

>
> --
> Jiong
>
>
>> as the dynamic relocs causing a value of zero to be output.
>> Calculating the size from input sections seems to make more sense.
>>
>> bfd/ChangeLog:
>>
>> 2014-03-20  Will Newton  <will.newton@linaro.org>
>>
>>          * elfnn-aarch64.c (elfNN_aarch64_finish_dynamic_sections):
>>          Set value of DT_PLTRELSZ and DT_RELASZ based on the size
>>          of input sections rather than output sections.
>>
>> ld/testsuite/ChangeLog:
>>
>> 2014-03-20  Will Newton  <will.newton@linaro.org>
>>
>>          * ld-aarch64/aarch64-elf.exp: Add relasz dump test.
>>          * ld-aarch64/relasz.d: New file.
>>          * ld-aarch64/relasz.s: Likewise.
>> ---
>>   bfd/elfnn-aarch64.c                     |  4 ++--
>>   ld/testsuite/ld-aarch64/aarch64-elf.exp |  2 ++
>>   ld/testsuite/ld-aarch64/relasz.d        | 18 ++++++++++++++++++
>>   ld/testsuite/ld-aarch64/relasz.s        |  9 +++++++++
>>   4 files changed, 31 insertions(+), 2 deletions(-)
>>   create mode 100644 ld/testsuite/ld-aarch64/relasz.d
>>   create mode 100644 ld/testsuite/ld-aarch64/relasz.s
>>
>> Changes in v2:
>>   - Add testcase
>>
>> diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
>> index c2adcc9..638938d 100644
>> --- a/bfd/elfnn-aarch64.c
>> +++ b/bfd/elfnn-aarch64.c
>> @@ -6993,7 +6993,7 @@ elfNN_aarch64_finish_dynamic_sections (bfd
>> *output_bfd,
>>               break;
>>             case DT_PLTRELSZ:
>> -             s = htab->root.srelplt->output_section;
>> +             s = htab->root.srelplt;
>>               dyn.d_un.d_val = s->size;
>>               break;
>>   @@ -7007,7 +7007,7 @@ elfNN_aarch64_finish_dynamic_sections (bfd
>> *output_bfd,
>>                  about changing the DT_RELA entry.  */
>>               if (htab->root.srelplt != NULL)
>>                 {
>> -                 s = htab->root.srelplt->output_section;
>> +                 s = htab->root.srelplt;
>>                   dyn.d_un.d_val -= s->size;
>>                 }
>>               break;
>> diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp
>> b/ld/testsuite/ld-aarch64/aarch64-elf.exp
>> index 32cf73a..845ea20 100644
>> --- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
>> +++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
>> @@ -157,3 +157,5 @@ run_dump_test "ifunc-19b"
>>   run_dump_test "ifunc-20"
>>   run_dump_test "ifunc-21"
>>   run_dump_test "ifunc-22"
>> +
>> +run_dump_test "relasz"
>> diff --git a/ld/testsuite/ld-aarch64/relasz.d
>> b/ld/testsuite/ld-aarch64/relasz.d
>> new file mode 100644
>> index 0000000..5cc5595
>> --- /dev/null
>> +++ b/ld/testsuite/ld-aarch64/relasz.d
>> @@ -0,0 +1,18 @@
>> +#source: relasz.s
>> +#ld: -shared -Taarch64.ld
>> +#readelf: -d
>> +# Check that the RELASZ section has the correct size even if we are
>> +# using a non-default linker script that merges .rela.dyn and .rela.plt
>> +# in the output.
>> +
>> +Dynamic section at offset 0x[0-9a-f]+ contains 9 entries:
>> +  Tag        Type                         Name/Value
>> + 0x0000000000000004 \(HASH\)               0x[0-9a-f]+
>> + 0x0000000000000005 \(STRTAB\)             0x[0-9a-f]+
>> + 0x0000000000000006 \(SYMTAB\)             0x[0-9a-f]+
>> + 0x000000000000000a \(STRSZ\)              [0-9]+ \(bytes\)
>> + 0x000000000000000b \(SYMENT\)             [0-9]+ \(bytes\)
>> + 0x0000000000000007 \(RELA\)               0x[0-9a-f]+
>> + 0x0000000000000008 \(RELASZ\)             24 \(bytes\)
>> + 0x0000000000000009 \(RELAENT\)            24 \(bytes\)
>> + 0x0000000000000000 \(NULL\)               0x0
>> diff --git a/ld/testsuite/ld-aarch64/relasz.s
>> b/ld/testsuite/ld-aarch64/relasz.s
>> new file mode 100644
>> index 0000000..8c7f891
>> --- /dev/null
>> +++ b/ld/testsuite/ld-aarch64/relasz.s
>> @@ -0,0 +1,9 @@
>> +       .text
>> +       .global func
>> +       .type   func, %function
>> +func:
>> +       adrp    x0, :got:foo
>> +       ldr     x0, [x0, #:got_lo12:foo]
>> +       ldr     w0, [x0]
>> +       ret
>> +       .size   func, .-func
>
>
> --
> Jiong
>
>



-- 
Will Newton
Toolchain Working Group, Linaro

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] bfd/elfnn-aarch64.c: Fix calculation of DT_RELASZ
  2014-03-24  8:49   ` Will Newton
@ 2014-03-24 12:26     ` Marcus Shawcroft
  2014-03-24 13:39       ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: Marcus Shawcroft @ 2014-03-24 12:26 UTC (permalink / raw)
  To: Will Newton; +Cc: Jiong Wang, binutils

On 24 March 2014 08:49, Will Newton <will.newton@linaro.org> wrote:
> On 20 March 2014 17:57, Jiong Wang <jiong.wang@arm.com> wrote:
>> On 20/03/14 15:46, Will Newton wrote:
>>>
>>> The current code subtracts the size of the output section containing
>>> relplt from RELASZ. In some cases this will be the same output
>>> section
>>
>>
>> Hi Will,
>>
>>   I guess you can only trigger this "same output section" issue when using
>> "ld-aarch64/aarch64.ld", right?
>
> Yes, it requires a custom linker script. The problem arose building
> OSv for aarch64:
>
> https://github.com/cloudius-systems/osv
>
> You could say the linker script is incorrect not to lay out the
> dynamic reloc sections explicitly but it seems counter intuitive
> behaviour to set RELASZ to zero when it clearly isn't.
>
> A number of ports base the size on the size of the output sections:
>
> lm32
> m32r
> nds32
> sh-32
> xtensa
> s390-64
> x86_64

which implies these ports are all broken in the same way ? or do they
address this issue in  a different way?

Cheers
/Marcus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] bfd/elfnn-aarch64.c: Fix calculation of DT_RELASZ
  2014-03-24 12:26     ` Marcus Shawcroft
@ 2014-03-24 13:39       ` Alan Modra
  0 siblings, 0 replies; 6+ messages in thread
From: Alan Modra @ 2014-03-24 13:39 UTC (permalink / raw)
  To: Marcus Shawcroft; +Cc: Will Newton, Jiong Wang, binutils

On Mon, Mar 24, 2014 at 12:26:28PM +0000, Marcus Shawcroft wrote:
> On 24 March 2014 08:49, Will Newton <will.newton@linaro.org> wrote:
[Re DT_PLTRELSZ and DT_RELASZ]
> > A number of ports base the size on the size of the output sections:
> >
> > lm32
> > m32r
> > nds32
> > sh-32
> > xtensa
> > s390-64
> > x86_64
> 
> which implies these ports are all broken in the same way ?

Yes, I think so.  The patch looks good to me.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] bfd/elfnn-aarch64.c: Fix calculation of DT_RELASZ
  2014-03-20 15:46 [PATCH v2] bfd/elfnn-aarch64.c: Fix calculation of DT_RELASZ Will Newton
  2014-03-20 17:57 ` Jiong Wang
@ 2014-03-25  7:39 ` Marcus Shawcroft
  1 sibling, 0 replies; 6+ messages in thread
From: Marcus Shawcroft @ 2014-03-25  7:39 UTC (permalink / raw)
  To: Will Newton; +Cc: binutils

On 20 March 2014 15:46, Will Newton <will.newton@linaro.org> wrote:
> The current code subtracts the size of the output section containing
> relplt from RELASZ. In some cases this will be the same output
> section as the dynamic relocs causing a value of zero to be output.
> Calculating the size from input sections seems to make more sense.
>
> bfd/ChangeLog:
>
> 2014-03-20  Will Newton  <will.newton@linaro.org>
>
>          * elfnn-aarch64.c (elfNN_aarch64_finish_dynamic_sections):
>          Set value of DT_PLTRELSZ and DT_RELASZ based on the size
>          of input sections rather than output sections.


OK, go ahead, thanks Will.  /Marcus

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-03-25  7:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-20 15:46 [PATCH v2] bfd/elfnn-aarch64.c: Fix calculation of DT_RELASZ Will Newton
2014-03-20 17:57 ` Jiong Wang
2014-03-24  8:49   ` Will Newton
2014-03-24 12:26     ` Marcus Shawcroft
2014-03-24 13:39       ` Alan Modra
2014-03-25  7:39 ` Marcus Shawcroft

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).