ld: Check bfd_gc_sections return

Message ID CAMe9rOo4GC_Lkw6PYA78CnR5gWsy6T1nYGW+hvdq9DXMeA2YLg@mail.gmail.com
State New
Headers
Series ld: Check bfd_gc_sections return |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed

Commit Message

H.J. Lu Aug. 5, 2026, 8:50 a.m. UTC
  bfd_gc_sections may return false on invalid input:

ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
offset 0x4 in section `.text.get_tls[get_tls]'

and set the BFD error to bfd_error_bad_value.  Check bfd_gc_sections
return and report the fatal error if the BFD error is bfd_error_bad_value.

PR ld/34448
* ldlang.c (lang_gc_sections): Check bfd_gc_sections return and
report the fatal error if the BFD error is bfd_error_bad_value.
  

Comments

Jan Beulich Aug. 5, 2026, 8:59 a.m. UTC | #1
On 05.08.2026 10:50, H.J. Lu wrote:
> bfd_gc_sections may return false on invalid input:
> 
> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> offset 0x4 in section `.text.get_tls[get_tls]'
> 
> and set the BFD error to bfd_error_bad_value.  Check bfd_gc_sections
> return and report the fatal error if the BFD error is bfd_error_bad_value.

And what guarantees that bfd_error_bad_value isn't also set for other kinds
of errors, from about anywhere in the library? Does bfd_gc_sections()
returning false even reliably set the error indicator? That's far from
obvious when looking at e.g. bfd_elf_gc_sections(), so I can't exclude the
possibility of bfd_error_bad_value having been on record before the call,
and it simply not getting changed.

As said on the earlier thread - issuing a fatal error upon encountering
broken input is fine. But a fatal error may only be raised in that case,
not any other one.

Jan
  
H.J. Lu Aug. 5, 2026, 10:32 a.m. UTC | #2
On Wed, Aug 5, 2026 at 4:59 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 05.08.2026 10:50, H.J. Lu wrote:
> > bfd_gc_sections may return false on invalid input:
> >
> > ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> > offset 0x4 in section `.text.get_tls[get_tls]'
> >
> > and set the BFD error to bfd_error_bad_value.  Check bfd_gc_sections
> > return and report the fatal error if the BFD error is bfd_error_bad_value.
>
> And what guarantees that bfd_error_bad_value isn't also set for other kinds
> of errors, from about anywhere in the library? Does bfd_gc_sections()
> returning false even reliably set the error indicator? That's far from

If it isn't the case, it is a bug.

> obvious when looking at e.g. bfd_elf_gc_sections(), so I can't exclude the
> possibility of bfd_error_bad_value having been on record before the call,
> and it simply not getting changed.

Isn't it a bug?

> As said on the earlier thread - issuing a fatal error upon encountering
> broken input is fine. But a fatal error may only be raised in that case,
> not any other one.
>
> Jan
  
Jan Beulich Aug. 5, 2026, 10:40 a.m. UTC | #3
On 05.08.2026 12:32, H.J. Lu wrote:
> On Wed, Aug 5, 2026 at 4:59 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 05.08.2026 10:50, H.J. Lu wrote:
>>> bfd_gc_sections may return false on invalid input:
>>>
>>> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
>>> offset 0x4 in section `.text.get_tls[get_tls]'
>>>
>>> and set the BFD error to bfd_error_bad_value.  Check bfd_gc_sections
>>> return and report the fatal error if the BFD error is bfd_error_bad_value.
>>
>> And what guarantees that bfd_error_bad_value isn't also set for other kinds
>> of errors, from about anywhere in the library? Does bfd_gc_sections()
>> returning false even reliably set the error indicator? That's far from
> 
> If it isn't the case, it is a bug.
> 
>> obvious when looking at e.g. bfd_elf_gc_sections(), so I can't exclude the
>> possibility of bfd_error_bad_value having been on record before the call,
>> and it simply not getting changed.
> 
> Isn't it a bug?

I don't know, as I don't know enough of the history of libbfd. The value could
be meant to be errno-like, i.e. you may need to clear it at certain points
(especially if there's a mix of functions storing into and not storing into
the field).

Jan
  
H.J. Lu Aug. 5, 2026, 9:24 p.m. UTC | #4
On Wed, Aug 5, 2026 at 6:40 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 05.08.2026 12:32, H.J. Lu wrote:
> > On Wed, Aug 5, 2026 at 4:59 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 05.08.2026 10:50, H.J. Lu wrote:
> >>> bfd_gc_sections may return false on invalid input:
> >>>
> >>> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> >>> offset 0x4 in section `.text.get_tls[get_tls]'
> >>>
> >>> and set the BFD error to bfd_error_bad_value.  Check bfd_gc_sections
> >>> return and report the fatal error if the BFD error is bfd_error_bad_value.
> >>
> >> And what guarantees that bfd_error_bad_value isn't also set for other kinds
> >> of errors, from about anywhere in the library? Does bfd_gc_sections()
> >> returning false even reliably set the error indicator? That's far from
> >
> > If it isn't the case, it is a bug.
> >
> >> obvious when looking at e.g. bfd_elf_gc_sections(), so I can't exclude the
> >> possibility of bfd_error_bad_value having been on record before the call,
> >> and it simply not getting changed.
> >
> > Isn't it a bug?
>
> I don't know, as I don't know enough of the history of libbfd. The value could
> be meant to be errno-like, i.e. you may need to clear it at certain points
> (especially if there's a mix of functions storing into and not storing into
> the field).

When a bfd function returns an error, the BFD error should be set
appropriately.  If it isn't the case, it is a BFD bug.
  
Alan Modra Aug. 5, 2026, 11:04 p.m. UTC | #5
On Thu, Aug 06, 2026 at 05:24:14AM +0800, H.J. Lu wrote:
> On Wed, Aug 5, 2026 at 6:40 PM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 05.08.2026 12:32, H.J. Lu wrote:
> > > On Wed, Aug 5, 2026 at 4:59 PM Jan Beulich <jbeulich@suse.com> wrote:
> > >>
> > >> On 05.08.2026 10:50, H.J. Lu wrote:
> > >>> bfd_gc_sections may return false on invalid input:
> > >>>
> > >>> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> > >>> offset 0x4 in section `.text.get_tls[get_tls]'
> > >>>
> > >>> and set the BFD error to bfd_error_bad_value.  Check bfd_gc_sections
> > >>> return and report the fatal error if the BFD error is bfd_error_bad_value.
> > >>
> > >> And what guarantees that bfd_error_bad_value isn't also set for other kinds
> > >> of errors, from about anywhere in the library? Does bfd_gc_sections()
> > >> returning false even reliably set the error indicator? That's far from
> > >
> > > If it isn't the case, it is a bug.
> > >
> > >> obvious when looking at e.g. bfd_elf_gc_sections(), so I can't exclude the
> > >> possibility of bfd_error_bad_value having been on record before the call,
> > >> and it simply not getting changed.
> > >
> > > Isn't it a bug?
> >
> > I don't know, as I don't know enough of the history of libbfd. The value could
> > be meant to be errno-like, i.e. you may need to clear it at certain points
> > (especially if there's a mix of functions storing into and not storing into
> > the field).
> 
> When a bfd function returns an error, the BFD error should be set
> appropriately.  If it isn't the case, it is a BFD bug.

The original commits 303b4cc64fa8 and 71b012a810fd omitted
checking the return from bfd_gc_sections, but I'm inclined to think
that any false return from bfd_gc_sections should result in an error
exit from ld, displaying the bfd error with %E.  We also should not be
ignoring a false return from gc_mark_extra_sections in
bfd_elf_gc_sections.
  

Patch

From 02ba2ff2dd6c58cb657cdbab4c33339b40ce1689 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 5 Aug 2026 16:43:05 +0800
Subject: [PATCH] ld: Check bfd_gc_sections return

bfd_gc_sections may return false on invalid input:

ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for offset 0x4 in section `.text.get_tls[get_tls]'

and set the BFD error to bfd_error_bad_value.  Check bfd_gc_sections
return and report the fatal error if the BFD error is bfd_error_bad_value.

	PR ld/34448
	* ldlang.c (lang_gc_sections): Check bfd_gc_sections return and
	report the fatal error if the BFD error is bfd_error_bad_value.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 ld/ldlang.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ld/ldlang.c b/ld/ldlang.c
index 886c49a1861..f8e9d2496a4 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -8165,8 +8165,10 @@  lang_gc_sections (void)
 	}
     }
 
-  if (link_info.gc_sections)
-    bfd_gc_sections (link_info.output_bfd, &link_info);
+  if (link_info.gc_sections
+      && !bfd_gc_sections (link_info.output_bfd, &link_info)
+      && bfd_get_error () == bfd_error_bad_value)
+    fatal (_("%P: --gc-sections failed: %E\n"));
 }
 
 /* Worker for lang_find_relro_sections_1.  */
-- 
2.55.0