public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] further correct x86'es last-insn tracking
@ 2023-12-11 13:07 Jan Beulich
  2023-12-11 13:08 ` [PATCH v2 1/4] x86: don't needlessly override .bss Jan Beulich
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Jan Beulich @ 2023-12-11 13:07 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

..., again involving corrections to common code as well.

v2: First and last patch are unchanged; the sole original middle patch
was split.

1: x86: don't needlessly override .bss
2: ELF: drop "push" parameter from obj_elf_change_section()
3: ELF: reliably invoke md_elf_section_change_hook()
4: x86: last-insn recording should be per-subsection

Jan

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

* [PATCH v2 1/4] x86: don't needlessly override .bss
  2023-12-11 13:07 [PATCH v2 0/4] further correct x86'es last-insn tracking Jan Beulich
@ 2023-12-11 13:08 ` Jan Beulich
  2023-12-11 13:09 ` [PATCH v2 2/4] ELF: drop "push" parameter from obj_elf_change_section() Jan Beulich
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2023-12-11 13:08 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

ELF, COFF, and Mach-O all have custom handlers for .bss. Don't override
those; install a handler only for a.out.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -181,7 +181,7 @@ static const reg_entry *build_modrm_byte
 static void output_insn (const struct last_insn *);
 static void output_imm (fragS *, offsetT);
 static void output_disp (fragS *, offsetT);
-#ifndef I386COFF
+#ifdef OBJ_AOUT
 static void s_bss (int);
 #endif
 #if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
@@ -1201,9 +1201,10 @@ const pseudo_typeS md_pseudo_table[] =
   {"align", s_align_ptwo, 0},
 #endif
   {"arch", set_cpu_arch, 0},
-#ifndef I386COFF
+#ifdef OBJ_AOUT
   {"bss", s_bss, 0},
-#else
+#endif
+#ifdef I386COFF
   {"lcomm", pe_lcomm, 1},
 #endif
   {"ffloat", float_cons, 'f'},
@@ -15475,17 +15476,13 @@ md_pcrel_from (fixS *fixP)
   return fixP->fx_size + fixP->fx_where + fixP->fx_frag->fr_address;
 }
 
-#ifndef I386COFF
+#ifdef OBJ_AOUT
 
 static void
 s_bss (int ignore ATTRIBUTE_UNUSED)
 {
   int temp;
 
-#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
-  if (IS_ELF)
-    obj_elf_section_change_hook ();
-#endif
   temp = get_absolute_expression ();
   subseg_set (bss_section, (subsegT) temp);
   demand_empty_rest_of_line ();


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

* [PATCH v2 2/4] ELF: drop "push" parameter from obj_elf_change_section()
  2023-12-11 13:07 [PATCH v2 0/4] further correct x86'es last-insn tracking Jan Beulich
  2023-12-11 13:08 ` [PATCH v2 1/4] x86: don't needlessly override .bss Jan Beulich
@ 2023-12-11 13:09 ` Jan Beulich
  2023-12-11 13:09 ` [PATCH v2 3/4] ELF: reliably invoke md_elf_section_change_hook() Jan Beulich
  2023-12-11 13:10 ` [PATCH v2 4/4] x86: last-insn recording should be per-subsection Jan Beulich
  3 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2023-12-11 13:09 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu, Nick Clifton, Alan Modra

No caller outside of obj-elf.c cares about the parameter - drop it by
introducing an obj-elf.c-internal wrapper.

While adding the new function parameter, take the opportunity and change
the adjacent boolean one to "bool".
---
v2: New.

--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -540,14 +540,14 @@ get_section_by_match (bfd *abfd ATTRIBUT
    other possibilities, but I don't know what they are.  In any case,
    BFD doesn't really let us set the section type.  */
 
-void
-obj_elf_change_section (const char *name,
-			unsigned int type,
-			bfd_vma attr,
-			int entsize,
-			struct elf_section_match *match_p,
-			int linkonce,
-			int push)
+static void
+change_section (const char *name,
+		unsigned int type,
+		bfd_vma attr,
+		int entsize,
+		struct elf_section_match *match_p,
+		bool linkonce,
+		bool push)
 {
   asection *old_sec;
   segT sec;
@@ -820,6 +820,17 @@ obj_elf_change_section (const char *name
 #endif
 }
 
+void
+obj_elf_change_section (const char *name,
+			unsigned int type,
+			bfd_vma attr,
+			int entsize,
+			struct elf_section_match *match_p,
+			bool linkonce)
+{
+  change_section (name, type, attr, entsize, match_p, linkonce, false);
+}
+
 static bfd_vma
 obj_elf_parse_section_letters (char *str, size_t len,
 			       bool *is_clone, int *inherit, bfd_vma *gnu_attr)
@@ -1488,8 +1499,7 @@ obj_elf_section (int push)
 	}
     }
 
-  obj_elf_change_section (name, type, attr, entsize, &match, linkonce,
-			  push);
+  change_section (name, type, attr, entsize, &match, linkonce, push);
 
   if (linked_to_section_index != -1UL)
     {
--- a/gas/config/obj-elf.h
+++ b/gas/config/obj-elf.h
@@ -198,7 +198,7 @@ extern void obj_elf_data (int);
 extern void obj_elf_text (int);
 extern void obj_elf_change_section
   (const char *, unsigned int, bfd_vma, int, struct elf_section_match *,
-   int, int);
+   bool);
 extern void obj_elf_vtable_inherit (int);
 extern void obj_elf_vtable_entry (int);
 extern struct fix * obj_elf_get_vtable_inherit (void);
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -27775,7 +27775,7 @@ start_unwind_section (const segT text_se
     }
 
   obj_elf_change_section (sec_name, type, flags, 0, &match,
-			  linkonce, 0);
+			  linkonce);
 
   /* Set the section link for index tables.  */
   if (idx)
--- a/gas/config/tc-ia64.c
+++ b/gas/config/tc-ia64.c
@@ -1139,7 +1139,7 @@ obj_elf_vms_common (int ignore ATTRIBUTE
   obj_elf_change_section
     (sec_name, SHT_NOBITS,
      SHF_ALLOC | SHF_WRITE | SHF_IA_64_VMS_OVERLAID | SHF_IA_64_VMS_GLOBAL,
-     0, NULL, 1, 0);
+     0, NULL, true);
 
   S_SET_VALUE (symbolP, 0);
   S_SET_SIZE (symbolP, size);
--- a/gas/config/tc-microblaze.c
+++ b/gas/config/tc-microblaze.c
@@ -150,7 +150,7 @@ microblaze_s_data (int ignore ATTRIBUTE_
 {
 #ifdef OBJ_ELF
   obj_elf_change_section (".data", SHT_PROGBITS, SHF_ALLOC+SHF_WRITE,
-			  0, 0, 0, 0);
+			  0, 0, false);
 #else
   s_data (ignore);
 #endif
@@ -163,7 +163,7 @@ microblaze_s_sdata (int ignore ATTRIBUTE
 {
 #ifdef OBJ_ELF
   obj_elf_change_section (".sdata", SHT_PROGBITS, SHF_ALLOC+SHF_WRITE,
-			  0, 0, 0, 0);
+			  0, 0, false);
 #else
   s_data (ignore);
 #endif
@@ -282,7 +282,7 @@ microblaze_s_rdata (int localvar)
     {
       /* rodata.  */
       obj_elf_change_section (".rodata", SHT_PROGBITS, SHF_ALLOC,
-			      0, 0, 0, 0);
+			      0, 0, false);
       if (rodata_segment == 0)
 	rodata_segment = subseg_new (".rodata", 0);
     }
@@ -290,7 +290,7 @@ microblaze_s_rdata (int localvar)
     {
       /* 1 .sdata2.  */
       obj_elf_change_section (".sdata2", SHT_PROGBITS, SHF_ALLOC,
-			      0, 0, 0, 0);
+			      0, 0, false);
     }
 #else
   s_data (ignore);
@@ -303,12 +303,12 @@ microblaze_s_bss (int localvar)
 #ifdef OBJ_ELF
   if (localvar == 0) /* bss.  */
     obj_elf_change_section (".bss", SHT_NOBITS, SHF_ALLOC+SHF_WRITE,
-			    0, 0, 0, 0);
+			    0, 0, false);
   else if (localvar == 1)
     {
       /* sbss.  */
       obj_elf_change_section (".sbss", SHT_NOBITS, SHF_ALLOC+SHF_WRITE,
-			      0, 0, 0, 0);
+			      0, 0, false);
       if (sbss_segment == 0)
 	sbss_segment = subseg_new (".sbss", 0);
     }
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -16435,7 +16435,7 @@ s_change_section (int ignore ATTRIBUTE_U
     section_type = SHT_PROGBITS;
 
   obj_elf_change_section (section_name, section_type, section_flag,
-			  section_entry_size, 0, 0, 0);
+			  section_entry_size, 0, false);
 }
 
 void
--- a/gas/config/tc-msp430.c
+++ b/gas/config/tc-msp430.c
@@ -622,7 +622,7 @@ msp430_profiler (int dummy ATTRIBUTE_UNU
   subseg = now_subseg;
 
   /* Now go to .profiler section.  */
-  obj_elf_change_section (".profiler", SHT_PROGBITS, 0, 0, 0, 0, 0);
+  obj_elf_change_section (".profiler", SHT_PROGBITS, 0, 0, 0, false);
 
   /* Save flags.  */
   emit_expr (& exp, 2);
--- a/gas/config/tc-rx.c
+++ b/gas/config/tc-rx.c
@@ -488,7 +488,7 @@ parse_rx_section (char * name)
       else
 	type = SHT_NOBITS;
 
-      obj_elf_change_section (name, type, attr, 0, NULL, false, false);
+      obj_elf_change_section (name, type, attr, 0, NULL, false);
     }
   else /* Try not to redefine a section, especially B_1.  */
     {
@@ -503,7 +503,7 @@ parse_rx_section (char * name)
 	| ((flags & SEC_STRINGS) ? SHF_STRINGS : 0)
 	| ((flags & SEC_THREAD_LOCAL) ? SHF_TLS : 0);
 
-      obj_elf_change_section (name, type, attr, 0, NULL, false, false);
+      obj_elf_change_section (name, type, attr, 0, NULL, false);
     }
 
   bfd_set_section_alignment (now_seg, align);
--- a/gas/config/tc-tic6x.c
+++ b/gas/config/tc-tic6x.c
@@ -4662,7 +4662,7 @@ tic6x_start_unwind_section (const segT t
     }
 
   obj_elf_change_section (sec_name, type, flags, 0, &match,
-			  linkonce, 0);
+			  linkonce);
 
   /* Set the section link for index tables.  */
   if (idx)


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

* [PATCH v2 3/4] ELF: reliably invoke md_elf_section_change_hook()
  2023-12-11 13:07 [PATCH v2 0/4] further correct x86'es last-insn tracking Jan Beulich
  2023-12-11 13:08 ` [PATCH v2 1/4] x86: don't needlessly override .bss Jan Beulich
  2023-12-11 13:09 ` [PATCH v2 2/4] ELF: drop "push" parameter from obj_elf_change_section() Jan Beulich
@ 2023-12-11 13:09 ` Jan Beulich
  2023-12-11 13:10 ` [PATCH v2 4/4] x86: last-insn recording should be per-subsection Jan Beulich
  3 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2023-12-11 13:09 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu, Nick Clifton, Alan Modra

... after any (sub)section change. While certain existing target hooks
only look at now_seg, for a few others it looks as if failing to do so
could have caused anomalies if sub-sections were used. In any event a
subsequent x86 change is going to require the sub-section to be properly
in place at the time the hook is invoked.

This primarily means for obj_elf_section() to pass the new subsection
into change_section(), for it to be set right away (ahead of invoking
the hook).

Also adjust obj_elf_ident() to invoke the hook after all section
changes. (Note that obj_elf_version(), which also changes sections and
then changes them back, has no hook invocation at all so far, so none
are added. Presumably there is a reason for this difference in
behavior.)
---
v2: Re-base over new earlier patch.

--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -547,7 +547,8 @@ change_section (const char *name,
 		int entsize,
 		struct elf_section_match *match_p,
 		bool linkonce,
-		bool push)
+		bool push,
+		subsegT new_subsection)
 {
   asection *old_sec;
   segT sec;
@@ -585,10 +586,10 @@ change_section (const char *name,
   if (old_sec)
     {
       sec = old_sec;
-      subseg_set (sec, 0);
+      subseg_set (sec, new_subsection);
     }
   else
-    sec = subseg_force_new (name, 0);
+    sec = subseg_force_new (name, new_subsection);
 
   bed = get_elf_backend_data (stdoutput);
   ssect = (*bed->get_sec_type_attr) (stdoutput, sec);
@@ -828,7 +829,7 @@ obj_elf_change_section (const char *name
 			struct elf_section_match *match_p,
 			bool linkonce)
 {
-  change_section (name, type, attr, entsize, match_p, linkonce, false);
+  change_section (name, type, attr, entsize, match_p, linkonce, false, 0);
 }
 
 static bfd_vma
@@ -1114,8 +1115,8 @@ obj_elf_section (int push)
   bfd_vma attr;
   bfd_vma gnu_attr;
   int entsize;
-  int linkonce;
-  subsegT new_subsection = -1;
+  bool linkonce;
+  subsegT new_subsection = 0;
   struct elf_section_match match;
   unsigned long linked_to_section_index = -1UL;
 
@@ -1499,7 +1500,8 @@ obj_elf_section (int push)
 	}
     }
 
-  change_section (name, type, attr, entsize, &match, linkonce, push);
+  change_section (name, type, attr, entsize, &match, linkonce, push,
+		  new_subsection);
 
   if (linked_to_section_index != -1UL)
     {
@@ -1507,9 +1509,6 @@ obj_elf_section (int push)
       elf_section_data (now_seg)->this_hdr.sh_link = linked_to_section_index;
       /* FIXME: Should we perform some sanity checking on the section index ?  */
     }
-
-  if (push && new_subsection != -1)
-    subseg_set (now_seg, new_subsection);
 }
 
 /* Change to the .bss section.  */
@@ -2529,9 +2528,17 @@ obj_elf_ident (int ignore ATTRIBUTE_UNUS
       *p = 0;
     }
   else
-    subseg_set (comment_section, 0);
+    {
+      subseg_set (comment_section, 0);
+#ifdef md_elf_section_change_hook
+      md_elf_section_change_hook ();
+#endif
+    }
   stringer (8 + 1);
   subseg_set (old_section, old_subsection);
+#ifdef md_elf_section_change_hook
+  md_elf_section_change_hook ();
+#endif
 }
 
 #ifdef INIT_STAB_SECTION


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

* [PATCH v2 4/4] x86: last-insn recording should be per-subsection
  2023-12-11 13:07 [PATCH v2 0/4] further correct x86'es last-insn tracking Jan Beulich
                   ` (2 preceding siblings ...)
  2023-12-11 13:09 ` [PATCH v2 3/4] ELF: reliably invoke md_elf_section_change_hook() Jan Beulich
@ 2023-12-11 13:10 ` Jan Beulich
  3 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2023-12-11 13:10 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

Otherwise intermediate subsection switches result in inconsistent
behavior. Leverage ELF's section change hook to switch state as
necessary, limiting overhead to the bare minimum when subsections aren't
used.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -15920,6 +15920,39 @@ i386_elf_section_type (const char *str,
   return -1;
 }
 
+void
+i386_elf_section_change_hook (void)
+{
+  struct i386_segment_info *info = &seg_info(now_seg)->tc_segment_info_data;
+  struct i386_segment_info *curr, *prev;
+
+  if (info->subseg == now_subseg)
+    return;
+
+  /* Find the (or make a) list entry to save state into.  */
+  for (prev = info; (curr = prev->next) != NULL; prev = curr)
+    if (curr->subseg == info->subseg)
+      break;
+  if (!curr)
+    {
+      curr = XNEW (struct i386_segment_info);
+      curr->subseg = info->subseg;
+      curr->next = NULL;
+      prev->next = curr;
+    }
+  curr->last_insn = info->last_insn;
+
+  /* Find the list entry to load state from.  */
+  for (curr = info->next; curr; curr = curr->next)
+    if (curr->subseg == now_subseg)
+      break;
+  if (curr)
+    info->last_insn = curr->last_insn;
+  else
+    memset (&info->last_insn, 0, sizeof (info->last_insn));
+  info->subseg = now_subseg;
+}
+
 #ifdef TE_SOLARIS
 void
 i386_solaris_fix_up_eh_frame (segT sec)
--- a/gas/config/tc-i386.h
+++ b/gas/config/tc-i386.h
@@ -294,6 +294,8 @@ struct i386_segment_info {
 	last_insn_prefix
       } kind;
   } last_insn;
+  subsegT subseg;
+  struct i386_segment_info *next;
 };
 
 #define TC_SEGMENT_INFO_TYPE struct i386_segment_info
@@ -395,6 +397,9 @@ extern void tc_x86_frame_initial_instruc
 #define md_elf_section_type(str,len) i386_elf_section_type (str, len)
 extern int i386_elf_section_type (const char *, size_t);
 
+#define md_elf_section_change_hook i386_elf_section_change_hook
+extern void i386_elf_section_change_hook (void);
+
 #ifdef TE_SOLARIS
 #define md_fix_up_eh_frame(sec) i386_solaris_fix_up_eh_frame (sec)
 extern void i386_solaris_fix_up_eh_frame (segT);
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -745,6 +745,8 @@ if [gas_32_check] then {
 	run_dump_test "nop-6"
 	run_dump_test "unique"
 
+	run_dump_test "lfence-subsect"
+
 	run_dump_test "property-1"
 
 	if {[istarget "*-*-linux*"]} then {
--- /dev/null
+++ b/gas/testsuite/gas/i386/lfence-subsect.d
@@ -0,0 +1,18 @@
+#as: -mlfence-before-indirect-branch=all
+#warning_output: lfence-section.e
+#objdump: -dw
+#name: -mlfence-before-indirect-branch=all w/ subsection switches
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <_start>:
+ +[a-f0-9]+:	f3 ff d0             	repz call \*%eax
+ +[a-f0-9]+:	f3 c3                	repz ret
+ +[a-f0-9]+:	cc                   	int3
+ +[a-f0-9]+:	cc                   	int3
+ +[a-f0-9]+:	cc                   	int3
+
+0+8 <aux1>:
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/i386/lfence-subsect.s
@@ -0,0 +1,19 @@
+	.text
+_start:
+	rep
+
+	.subsection 2
+aux1:
+	nop
+
+	.previous
+	call *%eax
+	rep
+
+	.pushsection .text, 2
+aux2:
+	nop
+
+	.popsection
+	ret
+	.p2align 2, 0xcc


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

end of thread, other threads:[~2023-12-11 13:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-11 13:07 [PATCH v2 0/4] further correct x86'es last-insn tracking Jan Beulich
2023-12-11 13:08 ` [PATCH v2 1/4] x86: don't needlessly override .bss Jan Beulich
2023-12-11 13:09 ` [PATCH v2 2/4] ELF: drop "push" parameter from obj_elf_change_section() Jan Beulich
2023-12-11 13:09 ` [PATCH v2 3/4] ELF: reliably invoke md_elf_section_change_hook() Jan Beulich
2023-12-11 13:10 ` [PATCH v2 4/4] x86: last-insn recording should be per-subsection Jan Beulich

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).