Re: [RFC][GOLD] ARM: Add cantunwind when unwind info does not match start of section

[Yury Usishchev <y dot usishchev at samsung dot com>]

Hi all!

Sorry, I forgot to add attachement:)

BR,
Yury Usishchev

diff --git a/gold/arm.cc b/gold/arm.cc
index ed13c87..c30ca2f 100644
--- a/gold/arm.cc
+++ b/gold/arm.cc
@@ -6022,6 +6022,19 @@ Arm_output_section<big_endian>::fix_exidx_coverage(
       const unsigned char* exidx_contents =
 	exidx_relobj->section_contents(exidx_shndx, &exidx_size, false);
 
+      // first_word is start address of first unwind entry in current input
+      // section.  If first_word is not zero then unwind info in this section
+      // does not cover start of section.  In this case we may need to add
+      // cantunwind to have consistent unwind info in output.
+      typedef typename elfcpp::Swap<32, big_endian>::Valtype Valtype;
+      const Valtype* wv = reinterpret_cast<const Valtype*>(exidx_contents);
+      uint32_t first_word = elfcpp::Swap<32, big_endian>::readval(wv);
+      if (first_word)
+	{
+	  exidx_fixup.add_exidx_cantunwind_as_needed();
+	}
+
+
       // Fix up coverage and append input section to output data list.
       Arm_exidx_section_offset_map* section_offset_map = NULL;
       uint32_t deleted_bytes =
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index ca24205..98c5e96 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -3238,7 +3238,7 @@ check_SCRIPTS += arm_exidx_test.sh
 check_DATA += arm_exidx_test.stdout
 
 arm_exidx_test.stdout: arm_exidx_test.so
-	$(TEST_READELF) -Sr $< > $@
+	$(TEST_READELF) -Sru $< > $@
 
 arm_exidx_test.so: arm_exidx_test.o ../ld-new
 	../ld-new -shared -o $@ $<
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index 253493f..4b61208 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -6781,7 +6781,7 @@ uninstall-am:
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_AS) -o $@ $<
 
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@arm_exidx_test.stdout: arm_exidx_test.so
-@DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_READELF) -Sr $< > $@
+@DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_READELF) -Sru $< > $@
 
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@arm_exidx_test.so: arm_exidx_test.o ../ld-new
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	../ld-new -shared -o $@ $<
diff --git a/gold/testsuite/arm_exidx_test.s b/gold/testsuite/arm_exidx_test.s
index 8e550e4..13f7d8b 100644
--- a/gold/testsuite/arm_exidx_test.s
+++ b/gold/testsuite/arm_exidx_test.s
@@ -23,6 +23,38 @@ empty:
         .fnend
         .size   empty, .-empty
 
+	.section        .text.start,"ax",%progbits
+	.align	2
+	.global start
+	.type start, %function
+start:
+	bx lr
+
+	.global	start1
+	.type	start1, %function
+start1:
+	.fnstart
+	.save {r4, lr}
+	.vsave {d0}
+	.vsave {d4}
+	bx lr
+	.fnend
+
+	.section        .text.end,"ax",%progbits
+	.align	2
+	.global end
+	.type end, %function
+end:
+	bx lr
+
+	.global another
+	.type another, %function
+another:
+	.fnstart
+	.save {r4, lr}
+	bx lr
+	.fnend
+
 # Check that no dynamic relocations for __exidx_start and __exidx_stop
 # generated.
 	.data
diff --git a/gold/testsuite/arm_exidx_test.sh b/gold/testsuite/arm_exidx_test.sh
index 699bdd1..6e8ae1a 100755
--- a/gold/testsuite/arm_exidx_test.sh
+++ b/gold/testsuite/arm_exidx_test.sh
@@ -56,5 +56,7 @@ check arm_exidx_test.stdout ".* .ARM.exidx .* ARM_EXIDX .* AL .*"
 check arm_exidx_test.stdout ".* .ARM.extab .* PROGBITS .* A .*"
 check_not arm_exidx_test.stdout ".* .* R_ARM_GLOB_DAT .* __exidx_start"
 check_not arm_exidx_test.stdout ".* .* R_ARM_GLOB_DAT .* __exidx_end"
+check arm_exidx_test.stdout ".*end.*cantunwind.*"
+check_not arm_exidx_test.stdout ".*start.*cantunwind.*"
 
 exit 0

Yury Usishchev <y.usishchev@samsung.com> writes:

> Hi all!
>
> I want to add one more condition to fix_exidx_coverage. This will wix PR gold/19611.
> I have a question about condition itself, I need to compare start address of
> first unwind entry (first_word) to actual start of the section. In bfd I
> compared it to vma. But as I understand vma of input sections is always zero.
> So the question is: is it correct to compare first_word to zero (as done
> in attached patch)?
>
>
> This bug was initially found in rtld
> (https://sourceware.org/ml/libc-alpha/2015-12/msg00501.html) and already fixed
> in bfd (https://sourceware.org/ml/binutils/2015-12/msg00367.html)
>
> It happens when two sections are merged into one. First section should have
> non-cantunwind last unwind entry and in second section first unwind entry
> should not match start of section.
>
> In this situation initial sections are correct (if unwinder hits start of
> section without unwind info it stops). But merged section has a gap in unwind
> info, which can cause problems.
>
> BR,
> Yury Usishchev
>
>
> gold/
> 2016-02-11 Yury Usishchev <y.usishchev@samsung.com>
>
> 	PR gold/19611
> 	* arm.cc (fix_exidx_coverage): Insert cantunwind when address in
> 	first unwind entry does not match start of section.
> 	* testsuite/Makefile.am (arm_exidx_test.stdout): Add dump of unwind info
> 	* testsuite/Makefile.in: Regenerate
> 	* testsuite/arm_exidx_test.c: Add sections to test unwind info merging
> 	* testsuite/arm_exidx_test.sh: Add check for correct unwind info merging