[PATCH v2 0/3] x86: another take at PC-relative reloc overflow checking

[PATCH v2 0/3] x86: another take at PC-relative reloc overflow checking

[Jan Beulich]

As requested this is a split of the original submission, with the
controversial patch put last (to, hopefully, allow the earlier
patches going in).

1: x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs
2: x86-64/ELF: use new reloc override model to deal with x32 special case
3: x86/ELF: permit correct overflow checking for 16-bit PC-relative relocs

Jan

[PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[Jan Beulich]

Right now it is impossible to encode certain valid 32-bit mode
constructs; see the respective new test case. Note that there are
further 32-bit PC-relative relocations, but I don't think they make a
lot of sense to use in mixed-bitness code, so they're not having
overrides put in place.

Putting in place a new testcase, I'd like to note that the two existing
ones (pcrel16 and pcrel16abs) appear to be pretty pointless: They don't
expect any error despite supposedly checking for overflow, and in fact
there can't possibly be any error for the
- former since gas doesn't emit any relocation in the first place there,
- latter because the way the relocation gets expressed by gas doesn't
  allow the linker to notice the overflow; it should be detected by gas
  if at all, but see above (an error would be reported here for x86-64
  afaict, but this test doesn't get re-used there).
---
TBD: I didn't put thoughts yet into also making this work when linking
     ELF to PE.

Note that I'm not sure at all whether this propagation of the struct
elf_linker_x86_params pointer is actually acceptable. But this is the
5th or 6th try I made, with all others having been worse or not even
working out. Hence I'd need pretty detailed guidance on how else the
information could be made available.
---
v2: Re-base and split.

--- a/bfd/elf-linker-x86.h
+++ b/bfd/elf-linker-x86.h
@@ -28,6 +28,13 @@ enum elf_x86_prop_report
   prop_report_shstk	= 1 << 3    /* Report missing SHSTK property.  */
 };
 
+/* Control of PC32 (on 64-bit) overflow check strictness.  */
+enum elf_x86_pcrel_relocs
+{
+  pcrel_relocs_default,
+  pcrel_relocs_lax,
+};
+
 /* Used to pass x86-specific linker options from ld to bfd.  */
 struct elf_linker_x86_params
 {
@@ -64,6 +71,9 @@ struct elf_linker_x86_params
   /* Report relative relocations.  */
   unsigned int report_relative_reloc : 1;
 
+  /* Strictness of PC32 (on 64-bit) overflow checks.  */
+  enum elf_x86_pcrel_relocs pcrel_relocs;
+
   /* X86-64 ISA level needed.  */
   unsigned int isa_level;
 
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -192,6 +192,15 @@ static reloc_howto_type x86_64_elf_howto
 	false)
 };
 
+static reloc_howto_type x86_64_howto_pc32_lax =
+  HOWTO(R_X86_64_PC32, 0, 2, 32, true, 0, complain_overflow_bitfield,
+	bfd_elf_generic_reloc, "R_X86_64_PC32", false, 0, 0xffffffff, true);
+
+static reloc_howto_type x86_64_howto_pc32_bnd_lax =
+  HOWTO(R_X86_64_PC32_BND, 0, 2, 32, true, 0, complain_overflow_bitfield,
+	bfd_elf_generic_reloc, "R_X86_64_PC32_BND", false, 0, 0xffffffff,
+	true);
+
 /* Map BFD relocs to the x86_64 elf relocs.  */
 struct elf_reloc_map
 {
@@ -248,6 +257,30 @@ static const struct elf_reloc_map x86_64
 };
 
 static reloc_howto_type *
+elf_x86_64_reloc_override (const bfd *abfd, reloc_howto_type *howto)
+{
+  const struct elf_linker_x86_params *params = elf_x86_tdata (abfd)->params;
+
+  switch (howto->type)
+    {
+    default:
+      break;
+
+    case R_X86_64_PC32:
+      if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax)
+	break;
+      return &x86_64_howto_pc32_lax;
+
+    case R_X86_64_PC32_BND:
+      if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax)
+	break;
+      return &x86_64_howto_pc32_bnd_lax;
+    }
+
+  return howto;
+}
+
+static reloc_howto_type *
 elf_x86_64_rtype_to_howto (bfd *abfd, unsigned r_type)
 {
   unsigned i;
@@ -275,7 +308,7 @@ elf_x86_64_rtype_to_howto (bfd *abfd, un
   else
     i = r_type - (unsigned int) R_X86_64_vt_offset;
   BFD_ASSERT (x86_64_elf_howto_table[i].type == r_type);
-  return &x86_64_elf_howto_table[i];
+  return elf_x86_64_reloc_override (abfd, &x86_64_elf_howto_table[i]);
 }
 
 /* Given a BFD reloc type, return a HOWTO structure.  */
@@ -313,7 +346,7 @@ elf_x86_64_reloc_name_lookup (bfd *abfd,
   for (i = 0; i < ARRAY_SIZE (x86_64_elf_howto_table); i++)
     if (x86_64_elf_howto_table[i].name != NULL
 	&& strcasecmp (x86_64_elf_howto_table[i].name, r_name) == 0)
-      return &x86_64_elf_howto_table[i];
+      return elf_x86_64_reloc_override (abfd, &x86_64_elf_howto_table[i]);
 
   return NULL;
 }
@@ -1846,6 +1879,9 @@ elf_x86_64_scan_relocs (bfd *abfd, struc
 
   BFD_ASSERT (is_x86_elf (abfd, htab));
 
+  /* Make command line controlled settings accessible from the object.  */
+  elf_x86_tdata (abfd)->params = htab->params;
+
   /* Get the section contents.  */
   if (elf_section_data (sec)->this_hdr.contents != NULL)
     contents = elf_section_data (sec)->this_hdr.contents;
--- a/bfd/elfxx-x86.h
+++ b/bfd/elfxx-x86.h
@@ -702,6 +702,9 @@ struct elf_x86_obj_tdata
   /* R_*_RELATIVE relocation in GOT for this local symbol has been
      processed.  */
   char *relative_reloc_done;
+
+  /* Container holding command line controlled linker settings.  */
+  const struct elf_linker_x86_params *params;
 };
 
 enum elf_x86_plt_type
--- /dev/null
+++ b/gas/testsuite/gas/i386/code32.d
@@ -0,0 +1,3 @@
+#name: x86-64 code32
+#as: -mx86-used-note=no --generate-missing-build-notes=no
+#readelf: -n
--- /dev/null
+++ b/gas/testsuite/gas/i386/code32.s
@@ -0,0 +1,11 @@
+	.code32
+	.text
+	.section .text.0, "ax", @progbits
+	.type func0, @function
+func0:
+	call func1
+	ret
+	.section .text.1, "ax", @progbits
+	.type func1, @function
+func1:
+	jmp func0
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -1331,6 +1331,7 @@ if [gas_64_check] then {
 	run_dump_test "x86-64-property-8"
 	run_dump_test "x86-64-property-9"
 	run_dump_test "x86-64-property-14"
+	run_dump_test "code32"
 
 	if {[istarget "*-*-linux*"]} then {
 	    run_dump_test "x86-64-align-branch-3"
--- a/ld/emulparams/elf32_x86_64.sh
+++ b/ld/emulparams/elf32_x86_64.sh
@@ -2,6 +2,7 @@ source_sh ${srcdir}/emulparams/plt_unwin
 source_sh ${srcdir}/emulparams/extern_protected_data.sh
 source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh
 source_sh ${srcdir}/emulparams/reloc_overflow.sh
+source_sh ${srcdir}/emulparams/pcrel-relocs.sh
 source_sh ${srcdir}/emulparams/call_nop.sh
 source_sh ${srcdir}/emulparams/cet.sh
 source_sh ${srcdir}/emulparams/x86-report-relative.sh
--- a/ld/emulparams/elf_x86_64.sh
+++ b/ld/emulparams/elf_x86_64.sh
@@ -2,6 +2,7 @@ source_sh ${srcdir}/emulparams/plt_unwin
 source_sh ${srcdir}/emulparams/extern_protected_data.sh
 source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh
 source_sh ${srcdir}/emulparams/reloc_overflow.sh
+source_sh ${srcdir}/emulparams/pcrel-relocs.sh
 source_sh ${srcdir}/emulparams/call_nop.sh
 source_sh ${srcdir}/emulparams/cet.sh
 source_sh ${srcdir}/emulparams/x86-report-relative.sh
--- /dev/null
+++ b/ld/emulparams/pcrel-relocs.sh
@@ -0,0 +1,11 @@
+PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS='
+  fprintf (file, _("\
+  -z lax-pcrel-relocs         Lax PC-relative relocation overflow checks\n"));
+'
+PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS='
+      else if (strcmp (optarg, "lax-pcrel-relocs") == 0)
+	params.pcrel_relocs = pcrel_relocs_lax;
+'
+
+PARSE_AND_LIST_OPTIONS="$PARSE_AND_LIST_OPTIONS $PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS"
+PARSE_AND_LIST_ARGS_CASE_Z="$PARSE_AND_LIST_ARGS_CASE_Z $PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS"
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
 the linker issue an error for missing properties in input files.
 Supported for Linux/x86_64.
 
+@item lax-pcrel-relocs
+Relax relocation overflow checks for certain 32-bit PC-relative relocations
+which, when used by 32-bit code inside a 64-bit object, may require a
+larger range of values to be considered valid.
+Supported for x86-64 ELF targets.
+
 @item lazy
 When generating an executable or shared library, mark it to tell the
 dynamic linker to defer function call resolution to the point when
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/code32.d
@@ -0,0 +1,19 @@
+#name: x86-64 R_X86_64_PC32 reloc in 32-bit mode
+#source: ${srcdir}/../../../gas/testsuite/gas/i386/code32.s
+#as: --64 --generate-missing-build-notes=no
+#ld: -T code32.t -z lax-pcrel-relocs
+#objdump: -dw -Mi386
+
+.*: +file format .*
+
+Disassembly of section .text.0:
+
+0+10+ <func0>:
+ +[a-f0-9]+:	e8 fb ff ff 8f       	call   a0000000 <func1>
+ +[a-f0-9]+:	c3                   	ret *
+
+Disassembly of section .text.1:
+
+0+a0+ <func1>:
+ +[a-f0-9]+:	e9 fb ff ff 6f       	jmp    10000000 <func0>
+#pass
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/code32.t
@@ -0,0 +1,7 @@
+OUTPUT_FORMAT("elf64-x86-64")
+OUTPUT_ARCH("i386:x86-64")
+SECTIONS
+{
+  .text.0 0x10000000 : { *(.text.0) }
+  .text.1 0xa0000000 : { *(.text.1) }
+}
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -501,6 +501,7 @@ run_dump_test "property-x86-isa3-x32"
 run_dump_test "property-x86-isa4"
 run_dump_test "property-x86-isa4-x32"
 run_dump_test "code16"
+run_dump_test "code32"
 run_dump_test "pr27491-1a"
 run_dump_test "pr27491-1b"
 run_dump_test "pr27491-1c"

[PATCH v2 2/3] x86-64/ELF: use new reloc override model to deal with x32 special case

[Jan Beulich]

Fold the two custom adjustments into a single new case block in
elf_x86_64_reloc_override().

--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -185,12 +185,13 @@ static reloc_howto_type x86_64_elf_howto
   HOWTO (R_X86_64_GNU_VTENTRY, 0, 4, 0, false, 0, complain_overflow_dont,
 	 _bfd_elf_rel_vtable_reloc_fn, "R_X86_64_GNU_VTENTRY", false, 0, 0,
 	 false),
+};
 
 /* Use complain_overflow_bitfield on R_X86_64_32 for x32.  */
+static reloc_howto_type x86_64_howto_32_x32 =
   HOWTO(R_X86_64_32, 0, 2, 32, false, 0, complain_overflow_bitfield,
 	bfd_elf_generic_reloc, "R_X86_64_32", false, 0, 0xffffffff,
-	false)
-};
+	false);
 
 static reloc_howto_type x86_64_howto_pc32_lax =
   HOWTO(R_X86_64_PC32, 0, 2, 32, true, 0, complain_overflow_bitfield,
@@ -266,6 +267,11 @@ elf_x86_64_reloc_override (const bfd *ab
     default:
       break;
 
+    case R_X86_64_32:
+      if (ABI_64_P (abfd))
+	break;
+      return &x86_64_howto_32_x32;
+
     case R_X86_64_PC32:
       if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax)
 	break;
@@ -285,15 +291,8 @@ elf_x86_64_rtype_to_howto (bfd *abfd, un
 {
   unsigned i;
 
-  if (r_type == (unsigned int) R_X86_64_32)
-    {
-      if (ABI_64_P (abfd))
-	i = r_type;
-      else
-	i = ARRAY_SIZE (x86_64_elf_howto_table) - 1;
-    }
-  else if (r_type < (unsigned int) R_X86_64_GNU_VTINHERIT
-	   || r_type >= (unsigned int) R_X86_64_max)
+  if (r_type < (unsigned int) R_X86_64_GNU_VTINHERIT
+      || r_type >= (unsigned int) R_X86_64_max)
     {
       if (r_type >= (unsigned int) R_X86_64_standard)
 	{
@@ -334,15 +333,6 @@ elf_x86_64_reloc_name_lookup (bfd *abfd,
 {
   unsigned int i;
 
-  if (!ABI_64_P (abfd) && strcasecmp (r_name, "R_X86_64_32") == 0)
-    {
-      /* Get x32 R_X86_64_32.  */
-      reloc_howto_type *reloc
-	= &x86_64_elf_howto_table[ARRAY_SIZE (x86_64_elf_howto_table) - 1];
-      BFD_ASSERT (reloc->type == (unsigned int) R_X86_64_32);
-      return reloc;
-    }
-
   for (i = 0; i < ARRAY_SIZE (x86_64_elf_howto_table); i++)
     if (x86_64_elf_howto_table[i].name != NULL
 	&& strcasecmp (x86_64_elf_howto_table[i].name, r_name) == 0)

[PATCH v2 3/3] x86/ELF: permit correct overflow checking for 16-bit PC-relative relocs

[Jan Beulich]

The only insn requiring a truly 16-bit PC-relative relocation outside of
16-bit mode is XBEGIN (with an operand size override). For it, the
relocation generated should behave similar to 8- and (for 64-bit) 32-bit
PC-relatives ones, i.e. be checked for a signed value to fit the field.
The relocation type, however, has been (ab)used by 16-bit code, assuming
wraparound at 16-bit segment boundaries. Proper overflow checks aren't
possible in this case, so introduce a command line option allowing to
have the linker apply strict checking.
---
TBD: Like for the earlier PC32 patch, I didn't put thoughts yet into
     also making this work when linking ELF to PE.
---
v2: Re-base and split.

--- a/bfd/elf-linker-x86.h
+++ b/bfd/elf-linker-x86.h
@@ -28,11 +28,12 @@ enum elf_x86_prop_report
   prop_report_shstk	= 1 << 3    /* Report missing SHSTK property.  */
 };
 
-/* Control of PC32 (on 64-bit) overflow check strictness.  */
+/* Control of PC16 (and PC32 on 64-bit) overflow check strictness.  */
 enum elf_x86_pcrel_relocs
 {
   pcrel_relocs_default,
   pcrel_relocs_lax,
+  pcrel_relocs_strict,
 };
 
 /* Used to pass x86-specific linker options from ld to bfd.  */
@@ -71,7 +72,7 @@ struct elf_linker_x86_params
   /* Report relative relocations.  */
   unsigned int report_relative_reloc : 1;
 
-  /* Strictness of PC32 (on 64-bit) overflow checks.  */
+  /* Strictness of PC16 (and PC32 on 64-bit) overflow checks.  */
   enum elf_x86_pcrel_relocs pcrel_relocs;
 
   /* X86-64 ISA level needed.  */
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -179,6 +179,10 @@ static reloc_howto_type elf_howto_table[
 
 };
 
+static reloc_howto_type elf_howto_pc16_strict =
+  HOWTO(R_386_PC16, 0, 1, 16, true, 0, complain_overflow_signed,
+	bfd_elf_generic_reloc, "R_386_PC16", true, 0xffff, 0xffff, true);
+
 #ifdef DEBUG_GEN_RELOC
 #define TRACE(str) \
   fprintf (stderr, "i386 bfd reloc lookup %d (%s)\n", code, str)
@@ -270,8 +274,14 @@ elf_i386_reloc_type_lookup (bfd *abfd,
       return &elf_howto_table[R_386_16 - R_386_ext_offset];
 
     case BFD_RELOC_16_PCREL:
+    {
+      const struct elf_linker_x86_params *params = elf_x86_tdata (abfd)->params;
+
       TRACE ("BFD_RELOC_16_PCREL");
+      if (params && params->pcrel_relocs == pcrel_relocs_strict)
+	return &elf_howto_pc16_strict;
       return &elf_howto_table[R_386_PC16 - R_386_ext_offset];
+    }
 
     case BFD_RELOC_8:
       TRACE ("BFD_RELOC_8");
@@ -349,21 +359,32 @@ elf_i386_reloc_type_lookup (bfd *abfd,
 }
 
 static reloc_howto_type *
-elf_i386_reloc_name_lookup (bfd *abfd ATTRIBUTE_UNUSED,
-			    const char *r_name)
+elf_i386_reloc_override (const bfd *abfd, reloc_howto_type *howto)
+{
+  const struct elf_linker_x86_params *params = elf_x86_tdata (abfd)->params;
+
+  if (howto->type != R_386_PC16 || params == NULL
+      || params->pcrel_relocs != pcrel_relocs_strict)
+    return howto;
+
+  return &elf_howto_pc16_strict;
+}
+
+static reloc_howto_type *
+elf_i386_reloc_name_lookup (bfd *abfd, const char *r_name)
 {
   unsigned int i;
 
   for (i = 0; i < sizeof (elf_howto_table) / sizeof (elf_howto_table[0]); i++)
     if (elf_howto_table[i].name != NULL
 	&& strcasecmp (elf_howto_table[i].name, r_name) == 0)
-      return &elf_howto_table[i];
+      return elf_i386_reloc_override (abfd, &elf_howto_table[i]);
 
   return NULL;
 }
 
 static reloc_howto_type *
-elf_i386_rtype_to_howto (unsigned r_type)
+elf_i386_rtype_to_howto (bfd *abfd, unsigned r_type)
 {
   unsigned int indx;
 
@@ -378,7 +399,7 @@ elf_i386_rtype_to_howto (unsigned r_type
   /* PR 17512: file: 0f67f69d.  */
   if (elf_howto_table [indx].type != r_type)
     return NULL;
-  return &elf_howto_table[indx];
+  return elf_i386_reloc_override (abfd, &elf_howto_table[indx]);
 }
 
 static bool
@@ -388,7 +409,7 @@ elf_i386_info_to_howto_rel (bfd *abfd,
 {
   unsigned int r_type = ELF32_R_TYPE (dst->r_info);
 
-  if ((cache_ptr->howto = elf_i386_rtype_to_howto (r_type)) == NULL)
+  if ((cache_ptr->howto = elf_i386_rtype_to_howto (abfd, r_type)) == NULL)
     {
       /* xgettext:c-format */
       _bfd_error_handler (_("%pB: unsupported relocation type %#x"),
@@ -1136,8 +1157,8 @@ elf_i386_tls_transition (struct bfd_link
       reloc_howto_type *from, *to;
       const char *name;
 
-      from = elf_i386_rtype_to_howto (from_type);
-      to = elf_i386_rtype_to_howto (to_type);
+      from = elf_i386_rtype_to_howto (abfd, from_type);
+      to = elf_i386_rtype_to_howto (abfd, to_type);
 
       if (h)
 	name = h->root.root.string;
@@ -1476,6 +1497,9 @@ elf_i386_scan_relocs (bfd *abfd,
 
   BFD_ASSERT (is_x86_elf (abfd, htab));
 
+  /* Make command line controlled settings accessible from the object.  */
+  elf_x86_tdata (abfd)->params = htab->params;
+
   /* Get the section contents.  */
   if (elf_section_data (sec)->this_hdr.contents != NULL)
     contents = elf_section_data (sec)->this_hdr.contents;
@@ -2075,7 +2099,7 @@ elf_i386_relocate_section (bfd *output_b
 	  continue;
 	}
 
-      howto = elf_i386_rtype_to_howto (r_type);
+      howto = elf_i386_rtype_to_howto (input_bfd, r_type);
       if (howto == NULL)
 	return _bfd_unrecognized_reloc (input_bfd, input_section, r_type);
 
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -193,6 +193,10 @@ static reloc_howto_type x86_64_howto_32_
 	bfd_elf_generic_reloc, "R_X86_64_32", false, 0, 0xffffffff,
 	false);
 
+static reloc_howto_type x86_64_howto_pc16_strict =
+  HOWTO(R_X86_64_PC16, 0, 1, 16, true, 0, complain_overflow_signed,
+	bfd_elf_generic_reloc, "R_X86_64_PC16", false, 0, 0xffff, true);
+
 static reloc_howto_type x86_64_howto_pc32_lax =
   HOWTO(R_X86_64_PC32, 0, 2, 32, true, 0, complain_overflow_bitfield,
 	bfd_elf_generic_reloc, "R_X86_64_PC32", false, 0, 0xffffffff, true);
@@ -272,6 +276,11 @@ elf_x86_64_reloc_override (const bfd *ab
 	break;
       return &x86_64_howto_32_x32;
 
+    case R_X86_64_PC16:
+      if (params == NULL || params->pcrel_relocs != pcrel_relocs_strict)
+	break;
+      return &x86_64_howto_pc16_strict;
+
     case R_X86_64_PC32:
       if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax)
 	break;
--- a/ld/emulparams/elf_i386.sh
+++ b/ld/emulparams/elf_i386.sh
@@ -1,6 +1,7 @@
 source_sh ${srcdir}/emulparams/plt_unwind.sh
 source_sh ${srcdir}/emulparams/extern_protected_data.sh
 source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh
+source_sh ${srcdir}/emulparams/pcrel-relocs.sh
 source_sh ${srcdir}/emulparams/call_nop.sh
 source_sh ${srcdir}/emulparams/cet.sh
 source_sh ${srcdir}/emulparams/x86-report-relative.sh
--- a/ld/emulparams/pcrel-relocs.sh
+++ b/ld/emulparams/pcrel-relocs.sh
@@ -1,8 +1,12 @@
 PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS='
   fprintf (file, _("\
+  -z strict-pcrel-relocs      Strict PC-relative relocation overflow checks\n"));
+  fprintf (file, _("\
   -z lax-pcrel-relocs         Lax PC-relative relocation overflow checks\n"));
 '
 PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS='
+      else if (strcmp (optarg, "strict-pcrel-relocs") == 0)
+	params.pcrel_relocs = pcrel_relocs_strict;
       else if (strcmp (optarg, "lax-pcrel-relocs") == 0)
 	params.pcrel_relocs = pcrel_relocs_lax;
 '
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -1506,6 +1506,13 @@ recommended to use @samp{-z start-stop-v
 programs and shared libraries so that these symbols are not exported
 between shared objects, which is not usually what's intended.
 
+@item strict-pcrel-relocs
+Tighten relocation overflow checks for 16-bit PC-relative relocations
+which, when used by 16-bit code, may require a larger range of values
+to be considered valid, preventing proper overflow recognition for
+native code .
+Supported for ix86 and x86-64 ELF targets.
+
 @item text
 @itemx notext
 @itemx textoff
--- a/ld/testsuite/ld-i386/pcrel16-2.d
+++ b/ld/testsuite/ld-i386/pcrel16-2.d
@@ -1,6 +1,5 @@
 #name: PCREL16 overflow (2)
 #as: --32
-#ld: -melf_i386
+#ld: -melf_i386 -z strict-pcrel-relocs
 #error: .*relocation truncated to fit: R_386_PC16 .*t16.*
 #error: .*relocation truncated to fit: R_386_PC16 .*_start.*
-#xfail: *-*-*
--- a/ld/testsuite/ld-x86-64/pcrel16-2.d
+++ b/ld/testsuite/ld-x86-64/pcrel16-2.d
@@ -1,6 +1,5 @@
 #name: PCREL16 overflow (2)
 #source: ../ld-i386/pcrel16-2.s
-#ld:
+#ld: -z strict-pcrel-relocs
 #error: .*relocation truncated to fit: R_X86_64_PC16 .*t16.*
 #error: .*relocation truncated to fit: R_X86_64_PC16 .*_start.*
-#xfail: *-*-*

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[H.J. Lu]

On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
> Right now it is impossible to encode certain valid 32-bit mode
> constructs; see the respective new test case. Note that there are
> further 32-bit PC-relative relocations, but I don't think they make a
> lot of sense to use in mixed-bitness code, so they're not having
> overrides put in place.
> 
> Putting in place a new testcase, I'd like to note that the two existing
> ones (pcrel16 and pcrel16abs) appear to be pretty pointless: They don't
> expect any error despite supposedly checking for overflow, and in fact
> there can't possibly be any error for the
> - former since gas doesn't emit any relocation in the first place there,
> - latter because the way the relocation gets expressed by gas doesn't
>   allow the linker to notice the overflow; it should be detected by gas
>   if at all, but see above (an error would be reported here for x86-64
>   afaict, but this test doesn't get re-used there).
> ---
> TBD: I didn't put thoughts yet into also making this work when linking
>      ELF to PE.
> 
> Note that I'm not sure at all whether this propagation of the struct
> elf_linker_x86_params pointer is actually acceptable. But this is the
> 5th or 6th try I made, with all others having been worse or not even
> working out. Hence I'd need pretty detailed guidance on how else the
> information could be made available.
> ---
> v2: Re-base and split.
> 
> --- a/bfd/elf-linker-x86.h
> +++ b/bfd/elf-linker-x86.h
> @@ -28,6 +28,13 @@ enum elf_x86_prop_report
>    prop_report_shstk	= 1 << 3    /* Report missing SHSTK property.  */
>  };
>  
> +/* Control of PC32 (on 64-bit) overflow check strictness.  */
> +enum elf_x86_pcrel_relocs
> +{
> +  pcrel_relocs_default,
> +  pcrel_relocs_lax,
> +};
> +
>  /* Used to pass x86-specific linker options from ld to bfd.  */
>  struct elf_linker_x86_params
>  {
> @@ -64,6 +71,9 @@ struct elf_linker_x86_params
>    /* Report relative relocations.  */
>    unsigned int report_relative_reloc : 1;
>  
> +  /* Strictness of PC32 (on 64-bit) overflow checks.  */
> +  enum elf_x86_pcrel_relocs pcrel_relocs;
> +
>    /* X86-64 ISA level needed.  */
>    unsigned int isa_level;
>  
> --- a/bfd/elf64-x86-64.c
> +++ b/bfd/elf64-x86-64.c
> @@ -192,6 +192,15 @@ static reloc_howto_type x86_64_elf_howto
>  	false)
>  };
>  
> +static reloc_howto_type x86_64_howto_pc32_lax =
> +  HOWTO(R_X86_64_PC32, 0, 2, 32, true, 0, complain_overflow_bitfield,
> +	bfd_elf_generic_reloc, "R_X86_64_PC32", false, 0, 0xffffffff, true);
> +
> +static reloc_howto_type x86_64_howto_pc32_bnd_lax =
> +  HOWTO(R_X86_64_PC32_BND, 0, 2, 32, true, 0, complain_overflow_bitfield,
> +	bfd_elf_generic_reloc, "R_X86_64_PC32_BND", false, 0, 0xffffffff,
> +	true);
> +
>  /* Map BFD relocs to the x86_64 elf relocs.  */
>  struct elf_reloc_map
>  {
> @@ -248,6 +257,30 @@ static const struct elf_reloc_map x86_64
>  };
>  
>  static reloc_howto_type *
> +elf_x86_64_reloc_override (const bfd *abfd, reloc_howto_type *howto)
> +{
> +  const struct elf_linker_x86_params *params = elf_x86_tdata (abfd)->params;
> +
> +  switch (howto->type)
> +    {
> +    default:
> +      break;
> +
> +    case R_X86_64_PC32:
> +      if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax)
> +	break;
> +      return &x86_64_howto_pc32_lax;
> +
> +    case R_X86_64_PC32_BND:
> +      if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax)
> +	break;
> +      return &x86_64_howto_pc32_bnd_lax;
> +    }
> +
> +  return howto;
> +}
> +
> +static reloc_howto_type *
>  elf_x86_64_rtype_to_howto (bfd *abfd, unsigned r_type)
>  {
>    unsigned i;
> @@ -275,7 +308,7 @@ elf_x86_64_rtype_to_howto (bfd *abfd, un
>    else
>      i = r_type - (unsigned int) R_X86_64_vt_offset;
>    BFD_ASSERT (x86_64_elf_howto_table[i].type == r_type);
> -  return &x86_64_elf_howto_table[i];
> +  return elf_x86_64_reloc_override (abfd, &x86_64_elf_howto_table[i]);
>  }
>  
>  /* Given a BFD reloc type, return a HOWTO structure.  */
> @@ -313,7 +346,7 @@ elf_x86_64_reloc_name_lookup (bfd *abfd,
>    for (i = 0; i < ARRAY_SIZE (x86_64_elf_howto_table); i++)
>      if (x86_64_elf_howto_table[i].name != NULL
>  	&& strcasecmp (x86_64_elf_howto_table[i].name, r_name) == 0)
> -      return &x86_64_elf_howto_table[i];
> +      return elf_x86_64_reloc_override (abfd, &x86_64_elf_howto_table[i]);
>  
>    return NULL;
>  }
> @@ -1846,6 +1879,9 @@ elf_x86_64_scan_relocs (bfd *abfd, struc
>  
>    BFD_ASSERT (is_x86_elf (abfd, htab));
>  
> +  /* Make command line controlled settings accessible from the object.  */
> +  elf_x86_tdata (abfd)->params = htab->params;
> +
>    /* Get the section contents.  */
>    if (elf_section_data (sec)->this_hdr.contents != NULL)
>      contents = elf_section_data (sec)->this_hdr.contents;
> --- a/bfd/elfxx-x86.h
> +++ b/bfd/elfxx-x86.h
> @@ -702,6 +702,9 @@ struct elf_x86_obj_tdata
>    /* R_*_RELATIVE relocation in GOT for this local symbol has been
>       processed.  */
>    char *relative_reloc_done;
> +
> +  /* Container holding command line controlled linker settings.  */
> +  const struct elf_linker_x86_params *params;
>  };
>  
>  enum elf_x86_plt_type
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/code32.d
> @@ -0,0 +1,3 @@
> +#name: x86-64 code32
> +#as: -mx86-used-note=no --generate-missing-build-notes=no
> +#readelf: -n
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/code32.s
> @@ -0,0 +1,11 @@
> +	.code32
> +	.text
> +	.section .text.0, "ax", @progbits
> +	.type func0, @function
> +func0:
> +	call func1
> +	ret
> +	.section .text.1, "ax", @progbits
> +	.type func1, @function
> +func1:
> +	jmp func0
> --- a/gas/testsuite/gas/i386/i386.exp
> +++ b/gas/testsuite/gas/i386/i386.exp
> @@ -1331,6 +1331,7 @@ if [gas_64_check] then {
>  	run_dump_test "x86-64-property-8"
>  	run_dump_test "x86-64-property-9"
>  	run_dump_test "x86-64-property-14"
> +	run_dump_test "code32"
>  
>  	if {[istarget "*-*-linux*"]} then {
>  	    run_dump_test "x86-64-align-branch-3"
> --- a/ld/emulparams/elf32_x86_64.sh
> +++ b/ld/emulparams/elf32_x86_64.sh
> @@ -2,6 +2,7 @@ source_sh ${srcdir}/emulparams/plt_unwin
>  source_sh ${srcdir}/emulparams/extern_protected_data.sh
>  source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh
>  source_sh ${srcdir}/emulparams/reloc_overflow.sh
> +source_sh ${srcdir}/emulparams/pcrel-relocs.sh
>  source_sh ${srcdir}/emulparams/call_nop.sh
>  source_sh ${srcdir}/emulparams/cet.sh
>  source_sh ${srcdir}/emulparams/x86-report-relative.sh
> --- a/ld/emulparams/elf_x86_64.sh
> +++ b/ld/emulparams/elf_x86_64.sh
> @@ -2,6 +2,7 @@ source_sh ${srcdir}/emulparams/plt_unwin
>  source_sh ${srcdir}/emulparams/extern_protected_data.sh
>  source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh
>  source_sh ${srcdir}/emulparams/reloc_overflow.sh
> +source_sh ${srcdir}/emulparams/pcrel-relocs.sh
>  source_sh ${srcdir}/emulparams/call_nop.sh
>  source_sh ${srcdir}/emulparams/cet.sh
>  source_sh ${srcdir}/emulparams/x86-report-relative.sh
> --- /dev/null
> +++ b/ld/emulparams/pcrel-relocs.sh
> @@ -0,0 +1,11 @@
> +PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS='
> +  fprintf (file, _("\
> +  -z lax-pcrel-relocs         Lax PC-relative relocation overflow checks\n"));
> +'
> +PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS='
> +      else if (strcmp (optarg, "lax-pcrel-relocs") == 0)
> +	params.pcrel_relocs = pcrel_relocs_lax;
> +'
> +
> +PARSE_AND_LIST_OPTIONS="$PARSE_AND_LIST_OPTIONS $PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS"
> +PARSE_AND_LIST_ARGS_CASE_Z="$PARSE_AND_LIST_ARGS_CASE_Z $PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS"
> --- a/ld/ld.texi
> +++ b/ld/ld.texi
> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
>  the linker issue an error for missing properties in input files.
>  Supported for Linux/x86_64.
>  
> +@item lax-pcrel-relocs
> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
> +which, when used by 32-bit code inside a 64-bit object, may require a
> +larger range of values to be considered valid.
> +Supported for x86-64 ELF targets.
> +

I think the check should be turned on automatically.  Can you use a GNU
property bit to tell linker that a larger range of values should be
checked for R_X86_64_PC32 and issue an error for R_X86_64_PC32_BND?


H.J.

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[Jan Beulich]

On 04.03.2022 15:18, H.J. Lu wrote:
> On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
>> Right now it is impossible to encode certain valid 32-bit mode
>> constructs; see the respective new test case. Note that there are
>> further 32-bit PC-relative relocations, but I don't think they make a
>> lot of sense to use in mixed-bitness code, so they're not having
>> overrides put in place.
>>
>> Putting in place a new testcase, I'd like to note that the two existing
>> ones (pcrel16 and pcrel16abs) appear to be pretty pointless: They don't
>> expect any error despite supposedly checking for overflow, and in fact
>> there can't possibly be any error for the
>> - former since gas doesn't emit any relocation in the first place there,
>> - latter because the way the relocation gets expressed by gas doesn't
>>   allow the linker to notice the overflow; it should be detected by gas
>>   if at all, but see above (an error would be reported here for x86-64
>>   afaict, but this test doesn't get re-used there).
>> ---
>> TBD: I didn't put thoughts yet into also making this work when linking
>>      ELF to PE.
>>
>> Note that I'm not sure at all whether this propagation of the struct
>> elf_linker_x86_params pointer is actually acceptable. But this is the
>> 5th or 6th try I made, with all others having been worse or not even
>> working out. Hence I'd need pretty detailed guidance on how else the
>> information could be made available.
>> ---
>> v2: Re-base and split.
>>
>> --- a/bfd/elf-linker-x86.h
>> +++ b/bfd/elf-linker-x86.h
>> @@ -28,6 +28,13 @@ enum elf_x86_prop_report
>>    prop_report_shstk	= 1 << 3    /* Report missing SHSTK property.  */
>>  };
>>  
>> +/* Control of PC32 (on 64-bit) overflow check strictness.  */
>> +enum elf_x86_pcrel_relocs
>> +{
>> +  pcrel_relocs_default,
>> +  pcrel_relocs_lax,
>> +};
>> +
>>  /* Used to pass x86-specific linker options from ld to bfd.  */
>>  struct elf_linker_x86_params
>>  {
>> @@ -64,6 +71,9 @@ struct elf_linker_x86_params
>>    /* Report relative relocations.  */
>>    unsigned int report_relative_reloc : 1;
>>  
>> +  /* Strictness of PC32 (on 64-bit) overflow checks.  */
>> +  enum elf_x86_pcrel_relocs pcrel_relocs;
>> +
>>    /* X86-64 ISA level needed.  */
>>    unsigned int isa_level;
>>  
>> --- a/bfd/elf64-x86-64.c
>> +++ b/bfd/elf64-x86-64.c
>> @@ -192,6 +192,15 @@ static reloc_howto_type x86_64_elf_howto
>>  	false)
>>  };
>>  
>> +static reloc_howto_type x86_64_howto_pc32_lax =
>> +  HOWTO(R_X86_64_PC32, 0, 2, 32, true, 0, complain_overflow_bitfield,
>> +	bfd_elf_generic_reloc, "R_X86_64_PC32", false, 0, 0xffffffff, true);
>> +
>> +static reloc_howto_type x86_64_howto_pc32_bnd_lax =
>> +  HOWTO(R_X86_64_PC32_BND, 0, 2, 32, true, 0, complain_overflow_bitfield,
>> +	bfd_elf_generic_reloc, "R_X86_64_PC32_BND", false, 0, 0xffffffff,
>> +	true);
>> +
>>  /* Map BFD relocs to the x86_64 elf relocs.  */
>>  struct elf_reloc_map
>>  {
>> @@ -248,6 +257,30 @@ static const struct elf_reloc_map x86_64
>>  };
>>  
>>  static reloc_howto_type *
>> +elf_x86_64_reloc_override (const bfd *abfd, reloc_howto_type *howto)
>> +{
>> +  const struct elf_linker_x86_params *params = elf_x86_tdata (abfd)->params;
>> +
>> +  switch (howto->type)
>> +    {
>> +    default:
>> +      break;
>> +
>> +    case R_X86_64_PC32:
>> +      if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax)
>> +	break;
>> +      return &x86_64_howto_pc32_lax;
>> +
>> +    case R_X86_64_PC32_BND:
>> +      if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax)
>> +	break;
>> +      return &x86_64_howto_pc32_bnd_lax;
>> +    }
>> +
>> +  return howto;
>> +}
>> +
>> +static reloc_howto_type *
>>  elf_x86_64_rtype_to_howto (bfd *abfd, unsigned r_type)
>>  {
>>    unsigned i;
>> @@ -275,7 +308,7 @@ elf_x86_64_rtype_to_howto (bfd *abfd, un
>>    else
>>      i = r_type - (unsigned int) R_X86_64_vt_offset;
>>    BFD_ASSERT (x86_64_elf_howto_table[i].type == r_type);
>> -  return &x86_64_elf_howto_table[i];
>> +  return elf_x86_64_reloc_override (abfd, &x86_64_elf_howto_table[i]);
>>  }
>>  
>>  /* Given a BFD reloc type, return a HOWTO structure.  */
>> @@ -313,7 +346,7 @@ elf_x86_64_reloc_name_lookup (bfd *abfd,
>>    for (i = 0; i < ARRAY_SIZE (x86_64_elf_howto_table); i++)
>>      if (x86_64_elf_howto_table[i].name != NULL
>>  	&& strcasecmp (x86_64_elf_howto_table[i].name, r_name) == 0)
>> -      return &x86_64_elf_howto_table[i];
>> +      return elf_x86_64_reloc_override (abfd, &x86_64_elf_howto_table[i]);
>>  
>>    return NULL;
>>  }
>> @@ -1846,6 +1879,9 @@ elf_x86_64_scan_relocs (bfd *abfd, struc
>>  
>>    BFD_ASSERT (is_x86_elf (abfd, htab));
>>  
>> +  /* Make command line controlled settings accessible from the object.  */
>> +  elf_x86_tdata (abfd)->params = htab->params;
>> +
>>    /* Get the section contents.  */
>>    if (elf_section_data (sec)->this_hdr.contents != NULL)
>>      contents = elf_section_data (sec)->this_hdr.contents;
>> --- a/bfd/elfxx-x86.h
>> +++ b/bfd/elfxx-x86.h
>> @@ -702,6 +702,9 @@ struct elf_x86_obj_tdata
>>    /* R_*_RELATIVE relocation in GOT for this local symbol has been
>>       processed.  */
>>    char *relative_reloc_done;
>> +
>> +  /* Container holding command line controlled linker settings.  */
>> +  const struct elf_linker_x86_params *params;
>>  };
>>  
>>  enum elf_x86_plt_type
>> --- /dev/null
>> +++ b/gas/testsuite/gas/i386/code32.d
>> @@ -0,0 +1,3 @@
>> +#name: x86-64 code32
>> +#as: -mx86-used-note=no --generate-missing-build-notes=no
>> +#readelf: -n
>> --- /dev/null
>> +++ b/gas/testsuite/gas/i386/code32.s
>> @@ -0,0 +1,11 @@
>> +	.code32
>> +	.text
>> +	.section .text.0, "ax", @progbits
>> +	.type func0, @function
>> +func0:
>> +	call func1
>> +	ret
>> +	.section .text.1, "ax", @progbits
>> +	.type func1, @function
>> +func1:
>> +	jmp func0
>> --- a/gas/testsuite/gas/i386/i386.exp
>> +++ b/gas/testsuite/gas/i386/i386.exp
>> @@ -1331,6 +1331,7 @@ if [gas_64_check] then {
>>  	run_dump_test "x86-64-property-8"
>>  	run_dump_test "x86-64-property-9"
>>  	run_dump_test "x86-64-property-14"
>> +	run_dump_test "code32"
>>  
>>  	if {[istarget "*-*-linux*"]} then {
>>  	    run_dump_test "x86-64-align-branch-3"
>> --- a/ld/emulparams/elf32_x86_64.sh
>> +++ b/ld/emulparams/elf32_x86_64.sh
>> @@ -2,6 +2,7 @@ source_sh ${srcdir}/emulparams/plt_unwin
>>  source_sh ${srcdir}/emulparams/extern_protected_data.sh
>>  source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh
>>  source_sh ${srcdir}/emulparams/reloc_overflow.sh
>> +source_sh ${srcdir}/emulparams/pcrel-relocs.sh
>>  source_sh ${srcdir}/emulparams/call_nop.sh
>>  source_sh ${srcdir}/emulparams/cet.sh
>>  source_sh ${srcdir}/emulparams/x86-report-relative.sh
>> --- a/ld/emulparams/elf_x86_64.sh
>> +++ b/ld/emulparams/elf_x86_64.sh
>> @@ -2,6 +2,7 @@ source_sh ${srcdir}/emulparams/plt_unwin
>>  source_sh ${srcdir}/emulparams/extern_protected_data.sh
>>  source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh
>>  source_sh ${srcdir}/emulparams/reloc_overflow.sh
>> +source_sh ${srcdir}/emulparams/pcrel-relocs.sh
>>  source_sh ${srcdir}/emulparams/call_nop.sh
>>  source_sh ${srcdir}/emulparams/cet.sh
>>  source_sh ${srcdir}/emulparams/x86-report-relative.sh
>> --- /dev/null
>> +++ b/ld/emulparams/pcrel-relocs.sh
>> @@ -0,0 +1,11 @@
>> +PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS='
>> +  fprintf (file, _("\
>> +  -z lax-pcrel-relocs         Lax PC-relative relocation overflow checks\n"));
>> +'
>> +PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS='
>> +      else if (strcmp (optarg, "lax-pcrel-relocs") == 0)
>> +	params.pcrel_relocs = pcrel_relocs_lax;
>> +'
>> +
>> +PARSE_AND_LIST_OPTIONS="$PARSE_AND_LIST_OPTIONS $PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS"
>> +PARSE_AND_LIST_ARGS_CASE_Z="$PARSE_AND_LIST_ARGS_CASE_Z $PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS"
>> --- a/ld/ld.texi
>> +++ b/ld/ld.texi
>> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
>>  the linker issue an error for missing properties in input files.
>>  Supported for Linux/x86_64.
>>  
>> +@item lax-pcrel-relocs
>> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
>> +which, when used by 32-bit code inside a 64-bit object, may require a
>> +larger range of values to be considered valid.
>> +Supported for x86-64 ELF targets.
>> +
> 
> I think the check should be turned on automatically.  Can you use a GNU
> property bit to tell linker that a larger range of values should be
> checked for R_X86_64_PC32

I'm not convinced that would be desirable - the relaxed checking, after
all, also affects relocations to 64-bit mode. Hence certain overflows
won't be detected anymore. Therefore I'd expect people to make use of
the new option only if they really have any affected relocations in
32-bit code. Additionally there's no way I can see to set such a
property indicator when encountering the relocations in question only
in data definitions, unless you wanted to tie the setting of the
indicator to the mere use of .code{16,32} anywhere in the source (which
would feel way to aggressive to me). IMO this level of control can only
be achieved via command line option (without (a) becoming much more
intrusive or (b) introducing new relocation types).

If this was to be "automated", the assembler would need to let the
linker know the boundaries between 64-bit and non-64-bit code, and
(yet more difficult) between data used by 64-bit code and such used
by 32-bit code.

> and issue an error for R_X86_64_PC32_BND?

Why should this result in an error?

Jan

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[H.J. Lu]

On Wed, Mar 9, 2022 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 04.03.2022 15:18, H.J. Lu wrote:
> > On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
> >> Right now it is impossible to encode certain valid 32-bit mode
> >> constructs; see the respective new test case. Note that there are
> >> further 32-bit PC-relative relocations, but I don't think they make a
> >> lot of sense to use in mixed-bitness code, so they're not having
> >> overrides put in place.
> >>
> >> Putting in place a new testcase, I'd like to note that the two existing
> >> ones (pcrel16 and pcrel16abs) appear to be pretty pointless: They don't
> >> expect any error despite supposedly checking for overflow, and in fact
> >> there can't possibly be any error for the
> >> - former since gas doesn't emit any relocation in the first place there,
> >> - latter because the way the relocation gets expressed by gas doesn't
> >>   allow the linker to notice the overflow; it should be detected by gas
> >>   if at all, but see above (an error would be reported here for x86-64
> >>   afaict, but this test doesn't get re-used there).
> >> ---
> >> TBD: I didn't put thoughts yet into also making this work when linking
> >>      ELF to PE.
> >>
> >> Note that I'm not sure at all whether this propagation of the struct
> >> elf_linker_x86_params pointer is actually acceptable. But this is the
> >> 5th or 6th try I made, with all others having been worse or not even
> >> working out. Hence I'd need pretty detailed guidance on how else the
> >> information could be made available.
> >> ---
> >> v2: Re-base and split.
> >>
> >> --- a/bfd/elf-linker-x86.h
> >> +++ b/bfd/elf-linker-x86.h
> >> @@ -28,6 +28,13 @@ enum elf_x86_prop_report
> >>    prop_report_shstk = 1 << 3    /* Report missing SHSTK property.  */
> >>  };
> >>
> >> +/* Control of PC32 (on 64-bit) overflow check strictness.  */
> >> +enum elf_x86_pcrel_relocs
> >> +{
> >> +  pcrel_relocs_default,
> >> +  pcrel_relocs_lax,
> >> +};
> >> +
> >>  /* Used to pass x86-specific linker options from ld to bfd.  */
> >>  struct elf_linker_x86_params
> >>  {
> >> @@ -64,6 +71,9 @@ struct elf_linker_x86_params
> >>    /* Report relative relocations.  */
> >>    unsigned int report_relative_reloc : 1;
> >>
> >> +  /* Strictness of PC32 (on 64-bit) overflow checks.  */
> >> +  enum elf_x86_pcrel_relocs pcrel_relocs;
> >> +
> >>    /* X86-64 ISA level needed.  */
> >>    unsigned int isa_level;
> >>
> >> --- a/bfd/elf64-x86-64.c
> >> +++ b/bfd/elf64-x86-64.c
> >> @@ -192,6 +192,15 @@ static reloc_howto_type x86_64_elf_howto
> >>      false)
> >>  };
> >>
> >> +static reloc_howto_type x86_64_howto_pc32_lax =
> >> +  HOWTO(R_X86_64_PC32, 0, 2, 32, true, 0, complain_overflow_bitfield,
> >> +    bfd_elf_generic_reloc, "R_X86_64_PC32", false, 0, 0xffffffff, true);
> >> +
> >> +static reloc_howto_type x86_64_howto_pc32_bnd_lax =
> >> +  HOWTO(R_X86_64_PC32_BND, 0, 2, 32, true, 0, complain_overflow_bitfield,
> >> +    bfd_elf_generic_reloc, "R_X86_64_PC32_BND", false, 0, 0xffffffff,
> >> +    true);
> >> +
> >>  /* Map BFD relocs to the x86_64 elf relocs.  */
> >>  struct elf_reloc_map
> >>  {
> >> @@ -248,6 +257,30 @@ static const struct elf_reloc_map x86_64
> >>  };
> >>
> >>  static reloc_howto_type *
> >> +elf_x86_64_reloc_override (const bfd *abfd, reloc_howto_type *howto)
> >> +{
> >> +  const struct elf_linker_x86_params *params = elf_x86_tdata (abfd)->params;
> >> +
> >> +  switch (howto->type)
> >> +    {
> >> +    default:
> >> +      break;
> >> +
> >> +    case R_X86_64_PC32:
> >> +      if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax)
> >> +    break;
> >> +      return &x86_64_howto_pc32_lax;
> >> +
> >> +    case R_X86_64_PC32_BND:
> >> +      if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax)
> >> +    break;
> >> +      return &x86_64_howto_pc32_bnd_lax;
> >> +    }
> >> +
> >> +  return howto;
> >> +}
> >> +
> >> +static reloc_howto_type *
> >>  elf_x86_64_rtype_to_howto (bfd *abfd, unsigned r_type)
> >>  {
> >>    unsigned i;
> >> @@ -275,7 +308,7 @@ elf_x86_64_rtype_to_howto (bfd *abfd, un
> >>    else
> >>      i = r_type - (unsigned int) R_X86_64_vt_offset;
> >>    BFD_ASSERT (x86_64_elf_howto_table[i].type == r_type);
> >> -  return &x86_64_elf_howto_table[i];
> >> +  return elf_x86_64_reloc_override (abfd, &x86_64_elf_howto_table[i]);
> >>  }
> >>
> >>  /* Given a BFD reloc type, return a HOWTO structure.  */
> >> @@ -313,7 +346,7 @@ elf_x86_64_reloc_name_lookup (bfd *abfd,
> >>    for (i = 0; i < ARRAY_SIZE (x86_64_elf_howto_table); i++)
> >>      if (x86_64_elf_howto_table[i].name != NULL
> >>      && strcasecmp (x86_64_elf_howto_table[i].name, r_name) == 0)
> >> -      return &x86_64_elf_howto_table[i];
> >> +      return elf_x86_64_reloc_override (abfd, &x86_64_elf_howto_table[i]);
> >>
> >>    return NULL;
> >>  }
> >> @@ -1846,6 +1879,9 @@ elf_x86_64_scan_relocs (bfd *abfd, struc
> >>
> >>    BFD_ASSERT (is_x86_elf (abfd, htab));
> >>
> >> +  /* Make command line controlled settings accessible from the object.  */
> >> +  elf_x86_tdata (abfd)->params = htab->params;
> >> +
> >>    /* Get the section contents.  */
> >>    if (elf_section_data (sec)->this_hdr.contents != NULL)
> >>      contents = elf_section_data (sec)->this_hdr.contents;
> >> --- a/bfd/elfxx-x86.h
> >> +++ b/bfd/elfxx-x86.h
> >> @@ -702,6 +702,9 @@ struct elf_x86_obj_tdata
> >>    /* R_*_RELATIVE relocation in GOT for this local symbol has been
> >>       processed.  */
> >>    char *relative_reloc_done;
> >> +
> >> +  /* Container holding command line controlled linker settings.  */
> >> +  const struct elf_linker_x86_params *params;
> >>  };
> >>
> >>  enum elf_x86_plt_type
> >> --- /dev/null
> >> +++ b/gas/testsuite/gas/i386/code32.d
> >> @@ -0,0 +1,3 @@
> >> +#name: x86-64 code32
> >> +#as: -mx86-used-note=no --generate-missing-build-notes=no
> >> +#readelf: -n
> >> --- /dev/null
> >> +++ b/gas/testsuite/gas/i386/code32.s
> >> @@ -0,0 +1,11 @@
> >> +    .code32
> >> +    .text
> >> +    .section .text.0, "ax", @progbits
> >> +    .type func0, @function
> >> +func0:
> >> +    call func1
> >> +    ret
> >> +    .section .text.1, "ax", @progbits
> >> +    .type func1, @function
> >> +func1:
> >> +    jmp func0
> >> --- a/gas/testsuite/gas/i386/i386.exp
> >> +++ b/gas/testsuite/gas/i386/i386.exp
> >> @@ -1331,6 +1331,7 @@ if [gas_64_check] then {
> >>      run_dump_test "x86-64-property-8"
> >>      run_dump_test "x86-64-property-9"
> >>      run_dump_test "x86-64-property-14"
> >> +    run_dump_test "code32"
> >>
> >>      if {[istarget "*-*-linux*"]} then {
> >>          run_dump_test "x86-64-align-branch-3"
> >> --- a/ld/emulparams/elf32_x86_64.sh
> >> +++ b/ld/emulparams/elf32_x86_64.sh
> >> @@ -2,6 +2,7 @@ source_sh ${srcdir}/emulparams/plt_unwin
> >>  source_sh ${srcdir}/emulparams/extern_protected_data.sh
> >>  source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh
> >>  source_sh ${srcdir}/emulparams/reloc_overflow.sh
> >> +source_sh ${srcdir}/emulparams/pcrel-relocs.sh
> >>  source_sh ${srcdir}/emulparams/call_nop.sh
> >>  source_sh ${srcdir}/emulparams/cet.sh
> >>  source_sh ${srcdir}/emulparams/x86-report-relative.sh
> >> --- a/ld/emulparams/elf_x86_64.sh
> >> +++ b/ld/emulparams/elf_x86_64.sh
> >> @@ -2,6 +2,7 @@ source_sh ${srcdir}/emulparams/plt_unwin
> >>  source_sh ${srcdir}/emulparams/extern_protected_data.sh
> >>  source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh
> >>  source_sh ${srcdir}/emulparams/reloc_overflow.sh
> >> +source_sh ${srcdir}/emulparams/pcrel-relocs.sh
> >>  source_sh ${srcdir}/emulparams/call_nop.sh
> >>  source_sh ${srcdir}/emulparams/cet.sh
> >>  source_sh ${srcdir}/emulparams/x86-report-relative.sh
> >> --- /dev/null
> >> +++ b/ld/emulparams/pcrel-relocs.sh
> >> @@ -0,0 +1,11 @@
> >> +PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS='
> >> +  fprintf (file, _("\
> >> +  -z lax-pcrel-relocs         Lax PC-relative relocation overflow checks\n"));
> >> +'
> >> +PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS='
> >> +      else if (strcmp (optarg, "lax-pcrel-relocs") == 0)
> >> +    params.pcrel_relocs = pcrel_relocs_lax;
> >> +'
> >> +
> >> +PARSE_AND_LIST_OPTIONS="$PARSE_AND_LIST_OPTIONS $PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS"
> >> +PARSE_AND_LIST_ARGS_CASE_Z="$PARSE_AND_LIST_ARGS_CASE_Z $PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS"
> >> --- a/ld/ld.texi
> >> +++ b/ld/ld.texi
> >> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
> >>  the linker issue an error for missing properties in input files.
> >>  Supported for Linux/x86_64.
> >>
> >> +@item lax-pcrel-relocs
> >> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
> >> +which, when used by 32-bit code inside a 64-bit object, may require a
> >> +larger range of values to be considered valid.
> >> +Supported for x86-64 ELF targets.
> >> +
> >
> > I think the check should be turned on automatically.  Can you use a GNU
> > property bit to tell linker that a larger range of values should be
> > checked for R_X86_64_PC32
>
> I'm not convinced that would be desirable - the relaxed checking, after
> all, also affects relocations to 64-bit mode. Hence certain overflows
> won't be detected anymore. Therefore I'd expect people to make use of
> the new option only if they really have any affected relocations in
> 32-bit code. Additionally there's no way I can see to set such a
> property indicator when encountering the relocations in question only
> in data definitions, unless you wanted to tie the setting of the
> indicator to the mere use of .code{16,32} anywhere in the source (which
> would feel way to aggressive to me). IMO this level of control can only
> be achieved via command line option (without (a) becoming much more
> intrusive or (b) introducing new relocation types).

A new relocation type sounds better.

> If this was to be "automated", the assembler would need to let the
> linker know the boundaries between 64-bit and non-64-bit code, and
> (yet more difficult) between data used by 64-bit code and such used
> by 32-bit code.
>
> > and issue an error for R_X86_64_PC32_BND?
>
> Why should this result in an error?
>
> Jan
>


-- 
H.J.

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[Jan Beulich]

On 09.03.2022 15:27, H.J. Lu wrote:
> On Wed, Mar 9, 2022 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 04.03.2022 15:18, H.J. Lu wrote:
>>> On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
>>>> --- a/ld/ld.texi
>>>> +++ b/ld/ld.texi
>>>> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
>>>>  the linker issue an error for missing properties in input files.
>>>>  Supported for Linux/x86_64.
>>>>
>>>> +@item lax-pcrel-relocs
>>>> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
>>>> +which, when used by 32-bit code inside a 64-bit object, may require a
>>>> +larger range of values to be considered valid.
>>>> +Supported for x86-64 ELF targets.
>>>> +
>>>
>>> I think the check should be turned on automatically.  Can you use a GNU
>>> property bit to tell linker that a larger range of values should be
>>> checked for R_X86_64_PC32
>>
>> I'm not convinced that would be desirable - the relaxed checking, after
>> all, also affects relocations to 64-bit mode. Hence certain overflows
>> won't be detected anymore. Therefore I'd expect people to make use of
>> the new option only if they really have any affected relocations in
>> 32-bit code. Additionally there's no way I can see to set such a
>> property indicator when encountering the relocations in question only
>> in data definitions, unless you wanted to tie the setting of the
>> indicator to the mere use of .code{16,32} anywhere in the source (which
>> would feel way to aggressive to me). IMO this level of control can only
>> be achieved via command line option (without (a) becoming much more
>> intrusive or (b) introducing new relocation types).
> 
> A new relocation type sounds better.

We've been there before with PC16 - there are enough arguments against
introducing new types. I also never had the intention to propose ABI
extensions.

Jan

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[H.J. Lu]

On Wed, Mar 9, 2022 at 6:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.03.2022 15:27, H.J. Lu wrote:
> > On Wed, Mar 9, 2022 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 04.03.2022 15:18, H.J. Lu wrote:
> >>> On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
> >>>> --- a/ld/ld.texi
> >>>> +++ b/ld/ld.texi
> >>>> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
> >>>>  the linker issue an error for missing properties in input files.
> >>>>  Supported for Linux/x86_64.
> >>>>
> >>>> +@item lax-pcrel-relocs
> >>>> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
> >>>> +which, when used by 32-bit code inside a 64-bit object, may require a
> >>>> +larger range of values to be considered valid.
> >>>> +Supported for x86-64 ELF targets.
> >>>> +
> >>>
> >>> I think the check should be turned on automatically.  Can you use a GNU
> >>> property bit to tell linker that a larger range of values should be
> >>> checked for R_X86_64_PC32
> >>
> >> I'm not convinced that would be desirable - the relaxed checking, after
> >> all, also affects relocations to 64-bit mode. Hence certain overflows
> >> won't be detected anymore. Therefore I'd expect people to make use of
> >> the new option only if they really have any affected relocations in
> >> 32-bit code. Additionally there's no way I can see to set such a
> >> property indicator when encountering the relocations in question only
> >> in data definitions, unless you wanted to tie the setting of the
> >> indicator to the mere use of .code{16,32} anywhere in the source (which
> >> would feel way to aggressive to me). IMO this level of control can only
> >> be achieved via command line option (without (a) becoming much more
> >> intrusive or (b) introducing new relocation types).
> >
> > A new relocation type sounds better.
>
> We've been there before with PC16 - there are enough arguments against
> introducing new types. I also never had the intention to propose ABI
> extensions.
>

A command-line option isn't user friendly.  On the other hand, why
now?  The issue has been there forever.

-- 
H.J.

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[Jan Beulich]

On 09.03.2022 16:08, H.J. Lu wrote:
> On Wed, Mar 9, 2022 at 6:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 09.03.2022 15:27, H.J. Lu wrote:
>>> On Wed, Mar 9, 2022 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 04.03.2022 15:18, H.J. Lu wrote:
>>>>> On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
>>>>>> --- a/ld/ld.texi
>>>>>> +++ b/ld/ld.texi
>>>>>> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
>>>>>>  the linker issue an error for missing properties in input files.
>>>>>>  Supported for Linux/x86_64.
>>>>>>
>>>>>> +@item lax-pcrel-relocs
>>>>>> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
>>>>>> +which, when used by 32-bit code inside a 64-bit object, may require a
>>>>>> +larger range of values to be considered valid.
>>>>>> +Supported for x86-64 ELF targets.
>>>>>> +
>>>>>
>>>>> I think the check should be turned on automatically.  Can you use a GNU
>>>>> property bit to tell linker that a larger range of values should be
>>>>> checked for R_X86_64_PC32
>>>>
>>>> I'm not convinced that would be desirable - the relaxed checking, after
>>>> all, also affects relocations to 64-bit mode. Hence certain overflows
>>>> won't be detected anymore. Therefore I'd expect people to make use of
>>>> the new option only if they really have any affected relocations in
>>>> 32-bit code. Additionally there's no way I can see to set such a
>>>> property indicator when encountering the relocations in question only
>>>> in data definitions, unless you wanted to tie the setting of the
>>>> indicator to the mere use of .code{16,32} anywhere in the source (which
>>>> would feel way to aggressive to me). IMO this level of control can only
>>>> be achieved via command line option (without (a) becoming much more
>>>> intrusive or (b) introducing new relocation types).
>>>
>>> A new relocation type sounds better.
>>
>> We've been there before with PC16 - there are enough arguments against
>> introducing new types. I also never had the intention to propose ABI
>> extensions.
>>
> 
> A command-line option isn't user friendly.  On the other hand, why
> now?  The issue has been there forever.

Because earlier on no-one cared to think about the issue? This really
should have been considered when the ABI was initially written. _That_
would then also have been the time to introduce separate relocation
types. Now we need to apply workarounds ...

Jan

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[H.J. Lu]

On Wed, Mar 9, 2022 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.03.2022 16:08, H.J. Lu wrote:
> > On Wed, Mar 9, 2022 at 6:39 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 09.03.2022 15:27, H.J. Lu wrote:
> >>> On Wed, Mar 9, 2022 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> On 04.03.2022 15:18, H.J. Lu wrote:
> >>>>> On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
> >>>>>> --- a/ld/ld.texi
> >>>>>> +++ b/ld/ld.texi
> >>>>>> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
> >>>>>>  the linker issue an error for missing properties in input files.
> >>>>>>  Supported for Linux/x86_64.
> >>>>>>
> >>>>>> +@item lax-pcrel-relocs
> >>>>>> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
> >>>>>> +which, when used by 32-bit code inside a 64-bit object, may require a
> >>>>>> +larger range of values to be considered valid.
> >>>>>> +Supported for x86-64 ELF targets.
> >>>>>> +
> >>>>>
> >>>>> I think the check should be turned on automatically.  Can you use a GNU
> >>>>> property bit to tell linker that a larger range of values should be
> >>>>> checked for R_X86_64_PC32
> >>>>
> >>>> I'm not convinced that would be desirable - the relaxed checking, after
> >>>> all, also affects relocations to 64-bit mode. Hence certain overflows
> >>>> won't be detected anymore. Therefore I'd expect people to make use of
> >>>> the new option only if they really have any affected relocations in
> >>>> 32-bit code. Additionally there's no way I can see to set such a
> >>>> property indicator when encountering the relocations in question only
> >>>> in data definitions, unless you wanted to tie the setting of the
> >>>> indicator to the mere use of .code{16,32} anywhere in the source (which
> >>>> would feel way to aggressive to me). IMO this level of control can only
> >>>> be achieved via command line option (without (a) becoming much more
> >>>> intrusive or (b) introducing new relocation types).
> >>>
> >>> A new relocation type sounds better.
> >>
> >> We've been there before with PC16 - there are enough arguments against
> >> introducing new types. I also never had the intention to propose ABI
> >> extensions.
> >>
> >
> > A command-line option isn't user friendly.  On the other hand, why
> > now?  The issue has been there forever.
>
> Because earlier on no-one cared to think about the issue? This really
> should have been considered when the ABI was initially written. _That_
> would then also have been the time to introduce separate relocation
> types. Now we need to apply workarounds ...
>

If there is a real issue, we should fix it without a command-line
option.  Can you use the input section name/flags to check it?

-- 
H.J.

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[Jan Beulich]

On 09.03.2022 16:32, H.J. Lu wrote:
> On Wed, Mar 9, 2022 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 09.03.2022 16:08, H.J. Lu wrote:
>>> On Wed, Mar 9, 2022 at 6:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 09.03.2022 15:27, H.J. Lu wrote:
>>>>> On Wed, Mar 9, 2022 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> On 04.03.2022 15:18, H.J. Lu wrote:
>>>>>>> On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
>>>>>>>> --- a/ld/ld.texi
>>>>>>>> +++ b/ld/ld.texi
>>>>>>>> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
>>>>>>>>  the linker issue an error for missing properties in input files.
>>>>>>>>  Supported for Linux/x86_64.
>>>>>>>>
>>>>>>>> +@item lax-pcrel-relocs
>>>>>>>> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
>>>>>>>> +which, when used by 32-bit code inside a 64-bit object, may require a
>>>>>>>> +larger range of values to be considered valid.
>>>>>>>> +Supported for x86-64 ELF targets.
>>>>>>>> +
>>>>>>>
>>>>>>> I think the check should be turned on automatically.  Can you use a GNU
>>>>>>> property bit to tell linker that a larger range of values should be
>>>>>>> checked for R_X86_64_PC32
>>>>>>
>>>>>> I'm not convinced that would be desirable - the relaxed checking, after
>>>>>> all, also affects relocations to 64-bit mode. Hence certain overflows
>>>>>> won't be detected anymore. Therefore I'd expect people to make use of
>>>>>> the new option only if they really have any affected relocations in
>>>>>> 32-bit code. Additionally there's no way I can see to set such a
>>>>>> property indicator when encountering the relocations in question only
>>>>>> in data definitions, unless you wanted to tie the setting of the
>>>>>> indicator to the mere use of .code{16,32} anywhere in the source (which
>>>>>> would feel way to aggressive to me). IMO this level of control can only
>>>>>> be achieved via command line option (without (a) becoming much more
>>>>>> intrusive or (b) introducing new relocation types).
>>>>>
>>>>> A new relocation type sounds better.
>>>>
>>>> We've been there before with PC16 - there are enough arguments against
>>>> introducing new types. I also never had the intention to propose ABI
>>>> extensions.
>>>>
>>>
>>> A command-line option isn't user friendly.  On the other hand, why
>>> now?  The issue has been there forever.
>>
>> Because earlier on no-one cared to think about the issue? This really
>> should have been considered when the ABI was initially written. _That_
>> would then also have been the time to introduce separate relocation
>> types. Now we need to apply workarounds ...
>>
> 
> If there is a real issue, we should fix it without a command-line
> option.  Can you use the input section name/flags to check it?

I don't see how - it's overwhelmingly likely all in .text.

Jan

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[H.J. Lu]

On Wed, Mar 9, 2022 at 7:41 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.03.2022 16:32, H.J. Lu wrote:
> > On Wed, Mar 9, 2022 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 09.03.2022 16:08, H.J. Lu wrote:
> >>> On Wed, Mar 9, 2022 at 6:39 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 09.03.2022 15:27, H.J. Lu wrote:
> >>>>> On Wed, Mar 9, 2022 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>> On 04.03.2022 15:18, H.J. Lu wrote:
> >>>>>>> On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
> >>>>>>>> --- a/ld/ld.texi
> >>>>>>>> +++ b/ld/ld.texi
> >>>>>>>> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
> >>>>>>>>  the linker issue an error for missing properties in input files.
> >>>>>>>>  Supported for Linux/x86_64.
> >>>>>>>>
> >>>>>>>> +@item lax-pcrel-relocs
> >>>>>>>> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
> >>>>>>>> +which, when used by 32-bit code inside a 64-bit object, may require a
> >>>>>>>> +larger range of values to be considered valid.
> >>>>>>>> +Supported for x86-64 ELF targets.
> >>>>>>>> +
> >>>>>>>
> >>>>>>> I think the check should be turned on automatically.  Can you use a GNU
> >>>>>>> property bit to tell linker that a larger range of values should be
> >>>>>>> checked for R_X86_64_PC32
> >>>>>>
> >>>>>> I'm not convinced that would be desirable - the relaxed checking, after
> >>>>>> all, also affects relocations to 64-bit mode. Hence certain overflows
> >>>>>> won't be detected anymore. Therefore I'd expect people to make use of
> >>>>>> the new option only if they really have any affected relocations in
> >>>>>> 32-bit code. Additionally there's no way I can see to set such a
> >>>>>> property indicator when encountering the relocations in question only
> >>>>>> in data definitions, unless you wanted to tie the setting of the
> >>>>>> indicator to the mere use of .code{16,32} anywhere in the source (which
> >>>>>> would feel way to aggressive to me). IMO this level of control can only
> >>>>>> be achieved via command line option (without (a) becoming much more
> >>>>>> intrusive or (b) introducing new relocation types).
> >>>>>
> >>>>> A new relocation type sounds better.
> >>>>
> >>>> We've been there before with PC16 - there are enough arguments against
> >>>> introducing new types. I also never had the intention to propose ABI
> >>>> extensions.
> >>>>
> >>>
> >>> A command-line option isn't user friendly.  On the other hand, why
> >>> now?  The issue has been there forever.
> >>
> >> Because earlier on no-one cared to think about the issue? This really
> >> should have been considered when the ABI was initially written. _That_
> >> would then also have been the time to introduce separate relocation
> >> types. Now we need to apply workarounds ...
> >>
> >
> > If there is a real issue, we should fix it without a command-line
> > option.  Can you use the input section name/flags to check it?
>
> I don't see how - it's overwhelmingly likely all in .text.

These are very very special cases.  You can place these special
instruction sequences in a different text section.

-- 
H.J.

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[Jan Beulich]

On 09.03.2022 16:54, H.J. Lu wrote:
> On Wed, Mar 9, 2022 at 7:41 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 09.03.2022 16:32, H.J. Lu wrote:
>>> On Wed, Mar 9, 2022 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 09.03.2022 16:08, H.J. Lu wrote:
>>>>> On Wed, Mar 9, 2022 at 6:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> On 09.03.2022 15:27, H.J. Lu wrote:
>>>>>>> On Wed, Mar 9, 2022 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>> On 04.03.2022 15:18, H.J. Lu wrote:
>>>>>>>>> On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
>>>>>>>>>> --- a/ld/ld.texi
>>>>>>>>>> +++ b/ld/ld.texi
>>>>>>>>>> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
>>>>>>>>>>  the linker issue an error for missing properties in input files.
>>>>>>>>>>  Supported for Linux/x86_64.
>>>>>>>>>>
>>>>>>>>>> +@item lax-pcrel-relocs
>>>>>>>>>> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
>>>>>>>>>> +which, when used by 32-bit code inside a 64-bit object, may require a
>>>>>>>>>> +larger range of values to be considered valid.
>>>>>>>>>> +Supported for x86-64 ELF targets.
>>>>>>>>>> +
>>>>>>>>>
>>>>>>>>> I think the check should be turned on automatically.  Can you use a GNU
>>>>>>>>> property bit to tell linker that a larger range of values should be
>>>>>>>>> checked for R_X86_64_PC32
>>>>>>>>
>>>>>>>> I'm not convinced that would be desirable - the relaxed checking, after
>>>>>>>> all, also affects relocations to 64-bit mode. Hence certain overflows
>>>>>>>> won't be detected anymore. Therefore I'd expect people to make use of
>>>>>>>> the new option only if they really have any affected relocations in
>>>>>>>> 32-bit code. Additionally there's no way I can see to set such a
>>>>>>>> property indicator when encountering the relocations in question only
>>>>>>>> in data definitions, unless you wanted to tie the setting of the
>>>>>>>> indicator to the mere use of .code{16,32} anywhere in the source (which
>>>>>>>> would feel way to aggressive to me). IMO this level of control can only
>>>>>>>> be achieved via command line option (without (a) becoming much more
>>>>>>>> intrusive or (b) introducing new relocation types).
>>>>>>>
>>>>>>> A new relocation type sounds better.
>>>>>>
>>>>>> We've been there before with PC16 - there are enough arguments against
>>>>>> introducing new types. I also never had the intention to propose ABI
>>>>>> extensions.
>>>>>>
>>>>>
>>>>> A command-line option isn't user friendly.  On the other hand, why
>>>>> now?  The issue has been there forever.
>>>>
>>>> Because earlier on no-one cared to think about the issue? This really
>>>> should have been considered when the ABI was initially written. _That_
>>>> would then also have been the time to introduce separate relocation
>>>> types. Now we need to apply workarounds ...
>>>>
>>>
>>> If there is a real issue, we should fix it without a command-line
>>> option.  Can you use the input section name/flags to check it?
>>
>> I don't see how - it's overwhelmingly likely all in .text.
> 
> These are very very special cases.  You can place these special
> instruction sequences in a different text section.

Not easily, and even less so in existing code. OS boot code typically is
a mix of 16-, 32-, and 64-bit code, just to give an example.

Jan

Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs

[H.J. Lu]

On Wed, Mar 9, 2022 at 8:49 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.03.2022 16:54, H.J. Lu wrote:
> > On Wed, Mar 9, 2022 at 7:41 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 09.03.2022 16:32, H.J. Lu wrote:
> >>> On Wed, Mar 9, 2022 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 09.03.2022 16:08, H.J. Lu wrote:
> >>>>> On Wed, Mar 9, 2022 at 6:39 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> On 09.03.2022 15:27, H.J. Lu wrote:
> >>>>>>> On Wed, Mar 9, 2022 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>> On 04.03.2022 15:18, H.J. Lu wrote:
> >>>>>>>>> On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote:
> >>>>>>>>>> --- a/ld/ld.texi
> >>>>>>>>>> +++ b/ld/ld.texi
> >>>>>>>>>> @@ -1372,6 +1372,12 @@ missing properties in input files.  @opt
> >>>>>>>>>>  the linker issue an error for missing properties in input files.
> >>>>>>>>>>  Supported for Linux/x86_64.
> >>>>>>>>>>
> >>>>>>>>>> +@item lax-pcrel-relocs
> >>>>>>>>>> +Relax relocation overflow checks for certain 32-bit PC-relative relocations
> >>>>>>>>>> +which, when used by 32-bit code inside a 64-bit object, may require a
> >>>>>>>>>> +larger range of values to be considered valid.
> >>>>>>>>>> +Supported for x86-64 ELF targets.
> >>>>>>>>>> +
> >>>>>>>>>
> >>>>>>>>> I think the check should be turned on automatically.  Can you use a GNU
> >>>>>>>>> property bit to tell linker that a larger range of values should be
> >>>>>>>>> checked for R_X86_64_PC32
> >>>>>>>>
> >>>>>>>> I'm not convinced that would be desirable - the relaxed checking, after
> >>>>>>>> all, also affects relocations to 64-bit mode. Hence certain overflows
> >>>>>>>> won't be detected anymore. Therefore I'd expect people to make use of
> >>>>>>>> the new option only if they really have any affected relocations in
> >>>>>>>> 32-bit code. Additionally there's no way I can see to set such a
> >>>>>>>> property indicator when encountering the relocations in question only
> >>>>>>>> in data definitions, unless you wanted to tie the setting of the
> >>>>>>>> indicator to the mere use of .code{16,32} anywhere in the source (which
> >>>>>>>> would feel way to aggressive to me). IMO this level of control can only
> >>>>>>>> be achieved via command line option (without (a) becoming much more
> >>>>>>>> intrusive or (b) introducing new relocation types).
> >>>>>>>
> >>>>>>> A new relocation type sounds better.
> >>>>>>
> >>>>>> We've been there before with PC16 - there are enough arguments against
> >>>>>> introducing new types. I also never had the intention to propose ABI
> >>>>>> extensions.
> >>>>>>
> >>>>>
> >>>>> A command-line option isn't user friendly.  On the other hand, why
> >>>>> now?  The issue has been there forever.
> >>>>
> >>>> Because earlier on no-one cared to think about the issue? This really
> >>>> should have been considered when the ABI was initially written. _That_
> >>>> would then also have been the time to introduce separate relocation
> >>>> types. Now we need to apply workarounds ...
> >>>>
> >>>
> >>> If there is a real issue, we should fix it without a command-line
> >>> option.  Can you use the input section name/flags to check it?
> >>
> >> I don't see how - it's overwhelmingly likely all in .text.
> >
> > These are very very special cases.  You can place these special
> > instruction sequences in a different text section.
>
> Not easily, and even less so in existing code. OS boot code typically is
> a mix of 16-, 32-, and 64-bit code, just to give an example.

Here are my preferences:

1. Add a new relocation type.
2. Add a bit to SHF_MASKPROC to indicate special relocation semantics.
3. Add a suffix to section name to indicate special relocation semantics.

Existing sources aren't impacted.

-- 
H.J.