[PATCH] ld/microblaze: don't index the local symbol cache with a global symbol index

Neal Frager <[email protected]>
Newsgroups gmane.comp.gnu.binutils
Message-ID <[email protected]>
From: Sam Price <[email protected]>

microblaze_elf_relax_section() was copied from sh_elf_relax_delete_bytes()
in 2009 (7ba29e2a41) but the copy dropped a bounds check that was already
present in the sh original.  This adds the missing two-line guard from
bfd/elf32-sh.c:1227-1228, preventing an out-of-bounds read that silently
corrupts relocation addends.

The bug: when scanning relocations in other sections, the code indexes
isymbuf with a global symbol index without checking it against sh_info.
isymbuf (from symtab_hdr->contents) holds only sh_info local symbols, so
global symbol indices read past the end of the allocation.

If the out-of-bounds bytes happen to look like a local section symbol for
the section being relaxed, the relocation's addend is incorrectly adjusted,
losing 4 bytes per deleted IMM instruction.  This corrupts structure member
accesses built at -O2:

	lwi rD, r0, sym + offsetof(struct s, member)

This was found as a silent miscompilation in RTEMS where
Per_CPU_Control::executing was fetched as Per_CPU_Control::dispatch_necessary.

The fix copies the guard from bfd/elf32-sh.c:1227-1228, present since the
first binutils commit (252b5132c7, 1999-05-03).

Three tests are added (the first for this target), checking that addends
survive relaxation.  relax-addend.d and relax-addend-data.d use
--gc-sections; relax-addend-eh.d uses .eh_frame editing (the route live on
current master).

Test behavior depends on how ld was built:

  ld built normally   all three pass with and without the fix, because
                      whether the out-of-bounds read corrupts the addend
                      depends on what is in the adjacent heap

  ld built with ASan  relax-addend-eh.d FAILS without the fix, because the
                      read itself is unconditional and ASan traps it, and
                      passes with the fix

ld/testsuite results on microblaze-elf are otherwise unchanged.

bfd/
	* elf32-microblaze.c (microblaze_elf_relax_section): Skip
	relocations against global symbols when scanning the relocations
	of other sections.

ld/
	* testsuite/ld-microblaze/microblaze.exp: New file.
	* testsuite/ld-microblaze/relax-addend.s: New file.
	* testsuite/ld-microblaze/relax-addend-support.s: New file.
	* testsuite/ld-microblaze/relax-addend.ld: New file.
	* testsuite/ld-microblaze/relax-addend.d: New test.
	* testsuite/ld-microblaze/relax-addend-data.d: New test.
	* testsuite/ld-microblaze/relax-addend-eh.s: New file.
	* testsuite/ld-microblaze/relax-addend-eh-support.s: New file.
	* testsuite/ld-microblaze/relax-addend-eh.ld: New file.
	* testsuite/ld-microblaze/relax-addend-eh.d: New test.

Signed-off-by: Neal Frager <[email protected]>
Signed-off-by: Sam Price <[email protected]>
Assisted-by: Claude (Anthropic)
---
 bfd/elf32-microblaze.c                        |  3 +
 ld/testsuite/ld-microblaze/microblaze.exp     | 28 +++++++
 .../ld-microblaze/relax-addend-data.d         | 17 +++++
 .../ld-microblaze/relax-addend-eh-support.s   | 16 ++++
 ld/testsuite/ld-microblaze/relax-addend-eh.d  | 27 +++++++
 ld/testsuite/ld-microblaze/relax-addend-eh.ld |  9 +++
 ld/testsuite/ld-microblaze/relax-addend-eh.s  | 76 +++++++++++++++++++
 .../ld-microblaze/relax-addend-support.s      | 24 ++++++
 ld/testsuite/ld-microblaze/relax-addend.d     | 26 +++++++
 ld/testsuite/ld-microblaze/relax-addend.ld    | 23 ++++++
 ld/testsuite/ld-microblaze/relax-addend.s     | 55 ++++++++++++++
 11 files changed, 304 insertions(+)
 create mode 100644 ld/testsuite/ld-microblaze/microblaze.exp
 create mode 100644 ld/testsuite/ld-microblaze/relax-addend-data.d
 create mode 100644 ld/testsuite/ld-microblaze/relax-addend-eh-support.s
 create mode 100644 ld/testsuite/ld-microblaze/relax-addend-eh.d
 create mode 100644 ld/testsuite/ld-microblaze/relax-addend-eh.ld
 create mode 100644 ld/testsuite/ld-microblaze/relax-addend-eh.s
 create mode 100644 ld/testsuite/ld-microblaze/relax-addend-support.s
 create mode 100644 ld/testsuite/ld-microblaze/relax-addend.d
 create mode 100644 ld/testsuite/ld-microblaze/relax-addend.ld
 create mode 100644 ld/testsuite/ld-microblaze/relax-addend.s

diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
index a52358a9a12..06cfb42cd6f 100644
--- a/bfd/elf32-microblaze.c
+++ b/bfd/elf32-microblaze.c
@@ -2084,6 +2084,9 @@ microblaze_elf_relax_section (bfd *abfd,
 	  irelscanend = irelocs + o->reloc_count;
 	  for (irelscan = irelocs; irelscan < irelscanend; irelscan++)
 	    {
+	      if (ELF32_R_SYM (irelscan->r_info) >= symtab_hdr->sh_info)
+		continue;
+
 	      if ((ELF32_R_TYPE (irelscan->r_info) == (int) R_MICROBLAZE_32)
 		  || (ELF32_R_TYPE (irelscan->r_info) == (int) R_MICROBLAZE_32_NONE))
 		{
diff --git a/ld/testsuite/ld-microblaze/microblaze.exp b/ld/testsuite/ld-microblaze/microblaze.exp
new file mode 100644
index 00000000000..919fc2c2a49
--- /dev/null
+++ b/ld/testsuite/ld-microblaze/microblaze.exp
@@ -0,0 +1,28 @@
+# Expect script for MicroBlaze ELF linker tests.
+#   Copyright (C) 2026 Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+if { ![istarget "microblaze*-*-*"] } {
+    return
+}
+
+foreach test [lsort [glob -nocomplain $srcdir/$subdir/*.d]] {
+    verbose [file rootname $test]
+    run_dump_test [file rootname $test]
+}
diff --git a/ld/testsuite/ld-microblaze/relax-addend-data.d b/ld/testsuite/ld-microblaze/relax-addend-data.d
new file mode 100644
index 00000000000..c4b5232d870
--- /dev/null
+++ b/ld/testsuite/ld-microblaze/relax-addend-data.d
@@ -0,0 +1,17 @@
+#source: relax-addend.s
+#source: relax-addend-support.s
+#as: -EL
+#ld: -EL -relax --gc-sections -T $srcdir/$subdir/relax-addend.ld
+#readelf: -x .checkdata
+#name: MicroBlaze relaxation preserves R_MICROBLAZE_32 addends
+
+# The same check for the R_MICROBLAZE_32 arm of the same loop, using a data
+# word rather than an instruction so that readelf alone can verify it.
+# .checkdata holds a single word initialised to gvar + 0x18; with .data pinned
+# by the linker script that is 0x90001018, little endian 18 10 00 90.
+# A linker which corrupts the addend stores 14 10 00 90.
+
+#...
+Hex dump of section '.checkdata':
+[ 	]*0x90001100 18100090 .*
+#pass
diff --git a/ld/testsuite/ld-microblaze/relax-addend-eh-support.s b/ld/testsuite/ld-microblaze/relax-addend-eh-support.s
new file mode 100644
index 00000000000..d06df4b83e8
--- /dev/null
+++ b/ld/testsuite/ld-microblaze/relax-addend-eh-support.s
@@ -0,0 +1,16 @@
+	.section .text.zz_support,"ax",@progbits
+	.globl	near_callee
+near_callee:
+	rtsd	r15, 8
+	nop
+	.globl	_start
+_start:
+	brlid	r15, aaa_relaxed
+	nop
+	brlid	r15, zzz_victim
+	nop
+	bri	0
+	.data
+	.globl	gvar
+	.align	2
+gvar:	.space	64
diff --git a/ld/testsuite/ld-microblaze/relax-addend-eh.d b/ld/testsuite/ld-microblaze/relax-addend-eh.d
new file mode 100644
index 00000000000..d061fd04fb1
--- /dev/null
+++ b/ld/testsuite/ld-microblaze/relax-addend-eh.d
@@ -0,0 +1,27 @@
+#source: relax-addend-eh.s
+#source: relax-addend-eh-support.s
+#as: -EL
+#ld: -EL -relax --gc-sections -T $srcdir/$subdir/relax-addend-eh.ld
+#objdump: -d
+#name: MicroBlaze relaxation preserves addends across .eh_frame editing
+
+# Same defect as relax-addend.d, reached the other way.
+#
+# _bfd_elf_discard_section_eh_frame installs a locals-only symbol cache in
+# symtab_hdr->contents when editing .eh_frame moves a local symbol defined
+# inside it; the generic ELF emulation then calls lang_relax_sections from the
+# same after_allocation.  dead_fn is unreferenced, so --gc-sections drops it,
+# its FDE is removed, the section shrinks and ehlocal moves -- which is what
+# makes the cache appear.  The linker script must KEEP .eh_frame or it is swept
+# and none of this happens.
+#
+# gvar lands at 0x90000074, so the reference to gvar + 0x18 must be 0x9000008c,
+# encoded as IMM 0x9000 followed by LWI with 0x008c.  A linker which corrupts
+# the addend emits e8600088.
+
+.*: +file format .*
+#...
+9000001c <zzz_victim>:
+[ 	]*9000001c:[ 	]+b0009000[ 	]+imm[ 	]+-28672
+[ 	]*90000020:[ 	]+e860008c[ 	]+lwi[ 	]+r3, r0, 140
+#pass
diff --git a/ld/testsuite/ld-microblaze/relax-addend-eh.ld b/ld/testsuite/ld-microblaze/relax-addend-eh.ld
new file mode 100644
index 00000000000..c0d11cc9cb4
--- /dev/null
+++ b/ld/testsuite/ld-microblaze/relax-addend-eh.ld
@@ -0,0 +1,9 @@
+ENTRY(_start)
+SECTIONS
+{
+  . = 0x90000000;
+  .text : { *(.text) *(.text.*) }
+  .eh_frame : { KEEP (*(.eh_frame)) }
+  .data : { *(.data) *(.data.*) }
+  /DISCARD/ : { *(.comment) *(.note*) }
+}
diff --git a/ld/testsuite/ld-microblaze/relax-addend-eh.s b/ld/testsuite/ld-microblaze/relax-addend-eh.s
new file mode 100644
index 00000000000..5900f8cc956
--- /dev/null
+++ b/ld/testsuite/ld-microblaze/relax-addend-eh.s
@@ -0,0 +1,76 @@
+/* dead_fn is dropped by --gc-sections; its FDE is then removed from .eh_frame,
+   which shifts ehlocal and makes adjust_eh_frame_local_symbols() cache a
+   locals-only symbol buffer in symtab_hdr->contents. */
+	.section .text.dead,"ax",@progbits
+	.globl	dead_fn
+	.type	dead_fn,@function
+dead_fn:
+	rtsd	r15, 8
+	nop
+	.size	dead_fn, .-dead_fn
+
+	.section .text.aaa_relaxed,"ax",@progbits
+	.globl	aaa_relaxed
+	.type	aaa_relaxed,@function
+aaa_relaxed:
+	addik	r1, r1, -28
+	swi	r15, r1, 0
+	brlid	r15, near_callee
+	nop
+	lwi	r15, r1, 0
+	rtsd	r15, 8
+	addik	r1, r1, 28
+	.size	aaa_relaxed, .-aaa_relaxed
+
+	.section .text.zzz_victim,"ax",@progbits
+	.globl	zzz_victim
+	.type	zzz_victim,@function
+zzz_victim:
+	lwi	r3, r0, gvar+24
+	rtsd	r15, 8
+	nop
+	.size	zzz_victim, .-zzz_victim
+
+	.section .eh_frame,"a",@progbits
+/* Hand-assembled so that no label-difference expressions are used.  MicroBlaze
+   GAS emits an R_MICROBLAZE_NONE marker for every resolved label difference,
+   which lands in .rela.eh_frame and trips the
+   BFD_ASSERT (cookie->rel->r_offset == ent->offset + 8) in
+   _bfd_elf_discard_section_eh_frame.  Literal lengths and CIE pointers keep
+   .rela.eh_frame to just the two FDE initial-location relocations. */
+
+	/* CIE at 0x00, total 20 bytes */
+	.4byte	16			/* length */
+	.4byte	0			/* CIE id */
+	.byte	1			/* version */
+	.asciz	"zR"			/* augmentation */
+	.uleb128 1			/* code alignment factor */
+	.sleb128 -4			/* data alignment factor */
+	.byte	15			/* return address register */
+	.uleb128 1			/* augmentation data length */
+	.byte	0x00			/* FDE encoding: DW_EH_PE_absptr */
+	.byte	0x0c, 0x01, 0x00	/* DW_CFA_def_cfa r1, 0 */
+
+	/* FDE for dead_fn at 0x14, total 20 bytes.  dead_fn is dropped by
+	   --gc-sections, so this FDE is removed and everything after it moves. */
+	.4byte	16			/* length */
+	.4byte	0x18			/* CIE pointer: this field's offset - 0 */
+	.4byte	dead_fn			/* initial location  <- the only reloc */
+	.4byte	8			/* address range */
+	.uleb128 0			/* augmentation data length */
+	.byte	0, 0, 0			/* padding to 20 bytes */
+
+ehlocal:				/* local symbol inside .eh_frame; it is
+					   this symbol moving that makes
+					   adjust_eh_frame_local_symbols() cache
+					   a locals-only symbol buffer */
+
+	/* FDE for aaa_relaxed at 0x28, total 20 bytes */
+	.4byte	16
+	.4byte	0x2c
+	.4byte	aaa_relaxed		/* <- the only other reloc */
+	.4byte	32
+	.uleb128 0
+	.byte	0, 0, 0
+
+	.4byte	0			/* terminator */
diff --git a/ld/testsuite/ld-microblaze/relax-addend-support.s b/ld/testsuite/ld-microblaze/relax-addend-support.s
new file mode 100644
index 00000000000..03d5eb0ffba
--- /dev/null
+++ b/ld/testsuite/ld-microblaze/relax-addend-support.s
@@ -0,0 +1,24 @@
+# Placed in .text.zz_support so that it is laid out after the sections of
+# relax-addend.s and the call from aaa_relaxed is a forward reference.  A
+# backward reference is not relaxed and the test would not exercise anything.
+
+	.section .text.zz_support,"ax",@progbits
+	.globl	near_callee
+	.type	near_callee,@function
+near_callee:
+	rtsd	r15, 8
+	nop
+	.size	near_callee, .-near_callee
+
+	.globl	_start
+_start:
+	brlid	r15, aaa_relaxed
+	nop
+	brlid	r15, zzz_victim
+	nop
+	bri	0
+
+	.data
+	.globl	gvar
+	.align	2
+gvar:	.space	64
diff --git a/ld/testsuite/ld-microblaze/relax-addend.d b/ld/testsuite/ld-microblaze/relax-addend.d
new file mode 100644
index 00000000000..951c8737696
--- /dev/null
+++ b/ld/testsuite/ld-microblaze/relax-addend.d
@@ -0,0 +1,26 @@
+#source: relax-addend.s
+#source: relax-addend-support.s
+#as: -EL
+#ld: -EL -relax --gc-sections -T $srcdir/$subdir/relax-addend.ld
+#objdump: -d
+#name: MicroBlaze relaxation preserves R_MICROBLAZE_64 addends
+
+# .text.aaa_relaxed contains an IMM that relaxation deletes.  .text.zzz_victim,
+# a sibling section of the same object, refers to gvar + 0x18.  The linker must
+# not let the deletion disturb that addend.
+#
+# The linker script pins .data, so gvar is at 0x90001000 and the reference must
+# resolve to 0x90001018, encoded as IMM 0x9000 followed by LWI with 0x1018.
+# A linker which corrupts the addend emits e8601014 instead.
+
+.*: +file format .*
+#...
+90000000 <aaa_relaxed>:
+[ 	]*90000000:[ 	]+3021ffe4[ 	]+addik[ 	]+r1, r1, -28
+[ 	]*90000004:[ 	]+f9e10000[ 	]+swi[ 	]+r15, r1, 0
+[ 	]*90000008:[ 	]+b9f40024[ 	]+brlid[ 	]+r15, 36
+#...
+9000001c <zzz_victim>:
+[ 	]*9000001c:[ 	]+b0009000[ 	]+imm[ 	]+-28672
+[ 	]*90000020:[ 	]+e8601018[ 	]+lwi[ 	]+r3, r0, 4120
+#pass
diff --git a/ld/testsuite/ld-microblaze/relax-addend.ld b/ld/testsuite/ld-microblaze/relax-addend.ld
new file mode 100644
index 00000000000..ca4d3603989
--- /dev/null
+++ b/ld/testsuite/ld-microblaze/relax-addend.ld
@@ -0,0 +1,23 @@
+/* Fixed layout so that the expected dump is deterministic.  gvar is the first
+   thing in .data, which is pinned, so the reference to gvar + 0x18 has a known
+   value regardless of how the sections happen to be ordered.  */
+ENTRY(_start)
+SECTIONS
+{
+  . = 0x90000000;
+  .text : {
+    *(.text.aaa_relaxed)
+    *(.text.zzz_victim)
+    *(.text.zz_support)
+    *(.text)
+    *(.text.*)
+  }
+  . = 0x90001000;
+  .data : {
+    *(.data)
+    *(.data.*)
+  }
+  . = 0x90001100;
+  .checkdata : { KEEP (*(.checkdata)) }
+  /DISCARD/ : { *(.comment) *(.note*) }
+}
diff --git a/ld/testsuite/ld-microblaze/relax-addend.s b/ld/testsuite/ld-microblaze/relax-addend.s
new file mode 100644
index 00000000000..16e500170fb
--- /dev/null
+++ b/ld/testsuite/ld-microblaze/relax-addend.s
@@ -0,0 +1,55 @@
+# Linker relaxation must not disturb the addend of an R_MICROBLAZE_64
+# relocation that refers to a symbol outside the section being relaxed.
+#
+# .text.aaa_relaxed contains an IMM that relaxation deletes.  .text.zzz_victim,
+# a sibling section in the same object file, contains
+#
+#	R_MICROBLAZE_64  gvar + 0x18
+#
+# While relaxing .text.aaa_relaxed, microblaze_elf_relax_section() walks the
+# relocations of every other section of the same BFD and, for R_MICROBLAZE_64,
+# indexes isymbuf with ELF32_R_SYM (irelscan->r_info) without checking it
+# against symtab_hdr->sh_info.  gvar is global, so its symbol index is >=
+# sh_info and the read is out of bounds -- isymbuf holds only sh_info entries
+# once --gc-sections has installed the locals-only cache.  If the bytes read
+# happen to satisfy
+#
+#	isym->st_shndx == shndx && ELF32_ST_TYPE (isym->st_info) == STT_SECTION
+#
+# the addend is decremented by calc_fixup(), and gvar + 0x18 links as
+# gvar + 0x14.
+#
+# The out-of-bounds read itself is unconditional; run ld under ASan or
+# valgrind to observe it.  Whether it corrupts the addend depends on what is
+# in the adjacent heap, so the check below is a correctness assertion rather
+# than a reliable trigger.
+
+	.section .text.aaa_relaxed,"ax",@progbits
+	.globl	aaa_relaxed
+	.type	aaa_relaxed,@function
+aaa_relaxed:
+	addik	r1, r1, -28
+	swi	r15, r1, 0
+	brlid	r15, near_callee	# IMM + BRLID; the IMM is deleted
+	nop
+	lwi	r15, r1, 0
+	rtsd	r15, 8
+	addik	r1, r1, 28
+	.size	aaa_relaxed, .-aaa_relaxed
+
+	.section .text.zzz_victim,"ax",@progbits
+	.globl	zzz_victim
+	.type	zzz_victim,@function
+zzz_victim:
+	lwi	r3, r0, gvar+24		# IMM + LWI; R_MICROBLAZE_64 gvar+0x18
+	rtsd	r15, 8
+	nop
+	.size	zzz_victim, .-zzz_victim
+
+/* A data reference to the same symbol with the same non-zero addend.  This
+   goes through the R_MICROBLAZE_32 arm of the same loop, and unlike the
+   instruction above it can be checked with readelf alone.  */
+	.section .checkdata,"aw",@progbits
+	.globl	check_word
+check_word:
+	.4byte	gvar + 0x18
-- 
2.25.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.