[PATCH 1/4] gas: ld: sframe: FDE func start address is from start of section

[Indu Bhagat]

In SFrame V2, the start PC in the SFrame function descriptor entry is a
32-bit signed integer (sfde_func_start_address) which is meant to
indicate the start PC of the function.  According to the specification,
it intends to hold the offset of the start PC of the function from the
_start_of_the_SFrame_section_.  This value can then be used by
stacktracers to simply do:

   sframe_find_fre (pc - sframe_vaddr)

when looking up SFrame stack trace data corresponding to the program
counter (pc).

In the current implementation, however, GAS is actually emitting a
PC-relative RELA such that sfde_func_start_address is the offset of the
start PC of the function from the _start_of_the_SFrame_FDE_.  ld.bfd was
then fixing up the value by adjusting the final values by r_offset (and
hence hiding the issue).  Fix GAS to correcly emit the SFrame section as
per specification; and fix sframe merging routines in the linker to do
the right thing.

Similarly, change the "relocated value" manually put into the SFrame FDE
for plt* sections in _bfd_x86_elf_finish_dynamic_sections ().  While at
it, keep the comments and the commented out code (for debugging) updated
as well.

This is not a specification change, but is necessary for fixing
relocatable links with SFrame.  ET_DYN, ET_EXEC binaries are unaffected.

Co-Authored-By: Jens Remus <jremus@linux.ibm.com>

bfd/
        * elf-sframe.c (_bfd_elf_merge_section_sframe): Do not fixup
	SFrame FDE function start address.
        * elfxx-x86.c (_bfd_x86_elf_finish_dynamic_sections): Likewise
	for PLT entries.
gas/
        * gen-sframe.c (output_sframe_funcdesc): New argument for SFrame
	section start.
        (output_sframe_internal): New symbol for SFrame section start.
include/
	* sframe.h: Fix incorrect comment.

---
[Changes from RFC]
 - Keep comments in bfd/elfxx-x86.c updated too
 - Address Jan's review comments "Some changes look unrelated"
[End of changes from RFC]
---
 bfd/elf-sframe.c |  3 ++-
 bfd/elfxx-x86.c  | 19 +++++++------------
 gas/gen-sframe.c |  9 ++++++---
 include/sframe.h |  2 +-
 4 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
index 97e00734515..4394e5ec57d 100644
--- a/bfd/elf-sframe.c
+++ b/bfd/elf-sframe.c
@@ -464,6 +464,7 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
 		     of PLT_SFRAME_FDE_START_OFFSET is also set to the
 		     same.  */
 		  r_offset = sframe_decoder_get_hdr_size (sfd_ctx);
+
 		  /* For any further SFrame FDEs, the generator has already put
 		     in an offset in place of sfde_func_start_address of the
 		     corresponding FDE.  We will use it by hand to relocate.  */
@@ -480,7 +481,7 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
 	      if (pltn_reloc_by_hand)
 		address += sframe_read_value (abfd, contents,
 					      pltn_r_offset, 4);
-	      address += (sec->output_offset + r_offset);
+	      address += sec->output_offset;
 
 	      /* FIXME For testing only. Cleanup later.  */
 	      // address += (sec->output_section->vma);
diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index 6ea41f29af1..02eaad8d9ba 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -2986,13 +2986,11 @@ _bfd_x86_elf_finish_dynamic_sections (bfd *output_bfd,
 	{
 	  bfd_vma plt_start = htab->elf.splt->output_section->vma;
 	  bfd_vma sframe_start = htab->plt_sframe->output_section->vma
-				   + htab->plt_sframe->output_offset
-				   + PLT_SFRAME_FDE_START_OFFSET;
-#if 0 /* FIXME Testing only. Remove before review.  */
+				   + htab->plt_sframe->output_offset;
+#if 0 /* FIXME Testing only.  */
 	  bfd_vma test_value = (plt_start - sframe_start)
 	    + htab->plt_sframe->output_section->vma
-	    + htab->plt_sframe->output_offset
-	    + PLT_SFRAME_FDE_START_OFFSET;
+	    + htab->plt_sframe->output_offset;
 	  bfd_put_signed_32 (dynobj, test_value,
 #endif
 	  bfd_put_signed_32 (dynobj, plt_start - sframe_start,
@@ -3020,13 +3018,11 @@ _bfd_x86_elf_finish_dynamic_sections (bfd *output_bfd,
 	  bfd_vma plt_start = htab->plt_second->output_section->vma;
 	  bfd_vma sframe_start
 	    = (htab->plt_second_sframe->output_section->vma
-	       + htab->plt_second_sframe->output_offset
-	       + PLT_SFRAME_FDE_START_OFFSET);
-#if 0 /* FIXME Testing only. Remove before review.  */
+	       + htab->plt_second_sframe->output_offset);
+#if 0 /* FIXME Testing only.  */
 	  bfd_vma test_value = (plt_start - sframe_start)
 	    + htab->plt_second_sframe->output_section->vma
-	    + htab->plt_second_sframe->output_offset
-	    + PLT_SFRAME_FDE_START_OFFSET;
+	    + htab->plt_second_sframe->output_offset;
 	  bfd_put_signed_32 (dynobj, test_value,
 #endif
 	  bfd_put_signed_32 (dynobj, plt_start - sframe_start,
@@ -3054,8 +3050,7 @@ _bfd_x86_elf_finish_dynamic_sections (bfd *output_bfd,
 	  bfd_vma plt_start = htab->plt_got->output_section->vma;
 	  bfd_vma sframe_start
 	    = (htab->plt_got_sframe->output_section->vma
-	       + htab->plt_got_sframe->output_offset
-	       + PLT_SFRAME_FDE_START_OFFSET);
+	       + htab->plt_got_sframe->output_offset);
 	  bfd_put_signed_32 (dynobj, plt_start - sframe_start,
 			     htab->plt_got_sframe->contents
 			     + PLT_SFRAME_FDE_START_OFFSET);
diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index 5b5e7b14b3a..3cc8aa35b0c 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -607,7 +607,8 @@ output_sframe_row_entry (symbolS *fde_start_addr,
 }
 
 static void
-output_sframe_funcdesc (symbolS *start_of_fre_section,
+output_sframe_funcdesc (symbolS *sframe_start,
+			symbolS *start_of_fre_section,
 			symbolS *fre_symbol,
 			struct sframe_func_entry *sframe_fde)
 {
@@ -621,7 +622,7 @@ output_sframe_funcdesc (symbolS *start_of_fre_section,
   /* Start address of the function.  */
   exp.X_op = O_subtract;
   exp.X_add_symbol = dw_fde_start_addrS; /* to location.  */
-  exp.X_op_symbol = symbol_temp_new_now (); /* from location.  */
+  exp.X_op_symbol = sframe_start; /* from location.  */
   exp.X_add_number = 0;
   emit_expr (&exp, sizeof_member (sframe_func_desc_entry,
 				  sfde_func_start_address));
@@ -682,6 +683,8 @@ output_sframe_internal (void)
   int fixed_fp_offset = SFRAME_CFA_FIXED_FP_INVALID;
   int fixed_ra_offset = SFRAME_CFA_FIXED_RA_INVALID;
 
+  symbolS *sframe_start = symbol_temp_new_now ();
+
   /* The function descriptor entries as dumped by the assembler are not
      sorted on PCs.  */
   unsigned char sframe_flags = 0;
@@ -763,7 +766,7 @@ output_sframe_internal (void)
   i = 0;
   for (sframe_fde = all_sframe_fdes; sframe_fde; sframe_fde = sframe_fde->next)
     {
-      output_sframe_funcdesc (start_of_fre_section,
+      output_sframe_funcdesc (sframe_start, start_of_fre_section,
 			      fre_symbols[i], sframe_fde);
       i += sframe_fde->num_fres;
     }
diff --git a/include/sframe.h b/include/sframe.h
index a965e23bdd1..6c3152f6121 100644
--- a/include/sframe.h
+++ b/include/sframe.h
@@ -174,7 +174,7 @@ typedef struct sframe_header
 typedef struct sframe_func_desc_entry
 {
   /* Function start address.  Encoded as a signed offset, relative to the
-     beginning of the current FDE.  */
+     beginning of the SFrame section.  */
   int32_t sfde_func_start_address;
   /* Size of the function in bytes.  */
   uint32_t sfde_func_size;
-- 
2.43.0