[GOLD] Fix PR22233 gold segfault in relocate_erratum_stub on aarch64-linux-gnu

Message ID CAEt-8LD2expg_e1Qt0xnZEoOk6317VAD0tWQ3ONFkLugksJD_Q@mail.gmail.com
State New
Headers show
Series
  • [GOLD] Fix PR22233 gold segfault in relocate_erratum_stub on aarch64-linux-gnu
Related show

Commit Message

Peter Smith Nov. 13, 2017, 3:07 p.m.
Hello,

I've recently investigated
https://sourceware.org/bugzilla/show_bug.cgi?id=22233 which is a
segfault in gold with the --fix-cortex-a53-843419 applied.

The full details are in the PR, to summarise:
- The fix for PR21868 (an internal error when --fix-cortex-a53-843419
is applied) has a small mistake in it.
- When the stub_owner section needs an erratum fix an incorrect
address for the stubs for the section is given to
relocate_erratum_stub.
- If we are lucky we will get a segfault, if we aren't an incorrect
patch or data corruption is possible. The error is visible in PR21868,
but the side-effects aren't fatal.
- The fix is a one line change to add the view_offset to pview.address
when doing the calculation to find the stub address.

This is a first time posting a fix to this mailing list so my
apologies for missing anything out.

Peter

Comments

Matthias Klose Nov. 14, 2017, 10:58 a.m. | #1
On 13.11.2017 16:07, Peter Smith wrote:
> Hello,

> 

> I've recently investigated

> https://sourceware.org/bugzilla/show_bug.cgi?id=22233 which is a

> segfault in gold with the --fix-cortex-a53-843419 applied.

> 

> The full details are in the PR, to summarise:

> - The fix for PR21868 (an internal error when --fix-cortex-a53-843419

> is applied) has a small mistake in it.

> - When the stub_owner section needs an erratum fix an incorrect

> address for the stubs for the section is given to

> relocate_erratum_stub.

> - If we are lucky we will get a segfault, if we aren't an incorrect

> patch or data corruption is possible. The error is visible in PR21868,

> but the side-effects aren't fatal.

> - The fix is a one line change to add the view_offset to pview.address

> when doing the calculation to find the stub address.

> 

> This is a first time posting a fix to this mailing list so my

> apologies for missing anything out.


I can confirm that this fixes the linking of the testcases (haskell libraries).
Please could that be applied to the trunk and the 2.29 branch?

Matthias
Peter Smith Nov. 30, 2017, 12:01 p.m. | #2
>> This is a first time posting a fix to this mailing list so my

>> apologies for missing anything out.

>

> I can confirm that this fixes the linking of the testcases (haskell libraries).

> Please could that be applied to the trunk and the 2.29 branch?

>

> Matthias


General ping for this patch. The fix is one line, and together with
the fixes for PR20765 should fix the known problems with the
--fix-cortex-a53-843419 option.

Peter
Cary Coutant Nov. 30, 2017, 9:56 p.m. | #3
> General ping for this patch. The fix is one line, and together with

> the fixes for PR20765 should fix the known problems with the

> --fix-cortex-a53-843419 option.


This is OK, but I prefer it without the + and - of view_offset.

>> This is a first time posting a fix to this mailing list so my

>> apologies for missing anything out.


No problems -- you're just missing a ChangeLog entry. I've committed
the modified patch with the following ChangeLog:

2017-11-30  Peter Smith  <peter.smith@linaro.org>

gold/
        PR gold/22233
        * aarch64.cc (AArch64_relobj::fix_errata_and_relocate_erratum_stubs):
        Fix calculation of stub address.

Thanks for the fix!

-cary

Patch

diff --git a/gold/aarch64.cc b/gold/aarch64.cc
index 4c6e920..c9c1b33 100644
--- a/gold/aarch64.cc
+++ b/gold/aarch64.cc
@@ -2041,7 +2041,8 @@  AArch64_relobj<size, big_endian>::fix_errata_and_relocate_erratum_stubs(
           // executed.
           stub_table->relocate_erratum_stub(
 	    stub,
-	    pview.view + view_offset + (stub_table->address() - pview.address));
+	    pview.view + view_offset + (stub_table->address() -
+                                        (pview.address + view_offset)));
 
           // Next erratum stub.
 	  ++p;