bfd/dwarf2: break equal-range function ties by DIE offset, not pointer

  lookup_address_in_function_table picks the function with the smallest range
containing the address.  When two candidates have the same range -- an inlined
subroutine covering its containing subprogram exactly -- the tie was broken by
comparing the two funcinfo pointers.  Those come from bfd_zalloc, so the winner
tracked heap layout (a new objalloc chunk can land below the old one), and
addr2line named a different function from run to run on the same binary; only
the function and its inlined frames varied, never file and line.

Order by unit_offset instead.  It does not move with the heap, and it restores
the pre-089e3718bd8 result: that algorithm walked the prepend-built
function_table and kept the last-parsed among equals, i.e. the greater DW_AT
offset.  An inlined subroutine's DIE is a child of its subprogram's, so it has
the greater offset and now wins every time -- the innermost routine, as
intended.  The offsets are unique within the one comp_unit these candidates
share.

This only changes output that was already unstable: funcinfos are allocated in
increasing-offset scan order, so within a single chunk pointer order and offset
order agree and both rules pick the same function -- verified byte-identical
over 164k addresses across ten inlined binaries.  Forcing the divergence needs
a chunk placed below its predecessor, which test input cannot, so the added
dw2-inline-tie test is a regression pin (inverting the compare to < fails it),
not a trigger.

bfd/
	* dwarf2.c (lookup_address_in_function_table): Break equal
	best_fit_len ties on funcinfo->unit_offset rather than on the
	funcinfo pointer.

Signed-off-by: Samuel Price <thesamprice@gmail.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 bfd/dwarf2.c                                     |   11 +
 binutils/testsuite/binutils-all/addr2line.exp    |    7 +
 binutils/testsuite/binutils-all/dw2-inline-tie.S |  186 ++++++++++++++++++++++
 binutils/testsuite/binutils-all/dw2-inline-tie.d |   12 +
 4 files changed, 214 insertions(+), 2 deletions(-)
  

On 18.08.2026 04:11, Sam Price wrote:
> lookup_address_in_function_table picks the function with the smallest range
> containing the address.  When two candidates have the same range -- an inlined
> subroutine covering its containing subprogram exactly -- the tie was broken by
> comparing the two funcinfo pointers.  Those come from bfd_zalloc, so the winner
> tracked heap layout (a new objalloc chunk can land below the old one), and
> addr2line named a different function from run to run on the same binary; only
> the function and its inlined frames varied, never file and line.
> 
> Order by unit_offset instead.  It does not move with the heap, and it restores
> the pre-089e3718bd8 result: that algorithm walked the prepend-built
> function_table and kept the last-parsed among equals, i.e. the greater DW_AT
> offset.  An inlined subroutine's DIE is a child of its subprogram's, so it has
> the greater offset and now wins every time -- the innermost routine, as
> intended.  The offsets are unique within the one comp_unit these candidates
> share.
> 
> This only changes output that was already unstable: funcinfos are allocated in
> increasing-offset scan order, so within a single chunk pointer order and offset
> order agree and both rules pick the same function -- verified byte-identical
> over 164k addresses across ten inlined binaries.  Forcing the divergence needs
> a chunk placed below its predecessor, which test input cannot, so the added
> dw2-inline-tie test is a regression pin (inverting the compare to < fails it),
> not a trigger.
> 
> bfd/
> 	* dwarf2.c (lookup_address_in_function_table): Break equal
> 	best_fit_len ties on funcinfo->unit_offset rather than on the
> 	funcinfo pointer.
> 
> Signed-off-by: Samuel Price <thesamprice@gmail.com>
> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
> ---
>  bfd/dwarf2.c                                     |   11 +
>  binutils/testsuite/binutils-all/addr2line.exp    |    7 +
>  binutils/testsuite/binutils-all/dw2-inline-tie.S |  186 ++++++++++++++++++++++
>  binutils/testsuite/binutils-all/dw2-inline-tie.d |   12 +
>  4 files changed, 214 insertions(+), 2 deletions(-)

This all looks good to me, if there wasn't the Co-Authored-By:. That, to me,
suggests that part of the patch was AI-generated, which would be in conflict
with [1].

Nick - if Sam came back saying the patch itself wasn't co-authored by Claude,
i.e. if this was more like an Assisted-by: tag, would that then be okay to
accept without asking further questions? Further, had the tag been absent, I
likely wouldn't have recognized any possible AI origin. Which puts under
question how to actually put the policy into practical effect.

Jan

[1] https://sourceware.org/binutils/wiki/LLM_Generated_Content
  

I was running a bunch of tests in parallel and was getting inconsistent
results.  This turned out to be the issue, and fixed the results.

I was using the ai to help run the tests / debug.  I had to interact with
it through a few iterations to get here, and verify it doesn’t change
results .  Also the original commit message was overlay wordy.  So it’s not
a fully automated patch.

I did read the ai usage for gcc and feel that it being under 15 lines of
code / trivial usage and thought it fell under that.  Would qualify it for
legally trivial.  I could manually write the commit message.  It would
become less wordy, possibly not as clean.

Still learning the ropes of open source commits.  Thank you for reviewing.

Sincerely,

Sam Price


On Fri, Aug 21, 2026 at 7:38 AM Jan Beulich <jbeulich@suse.com> wrote:

> On 18.08.2026 04:11, Sam Price wrote:
> > lookup_address_in_function_table picks the function with the smallest
> range
> > containing the address.  When two candidates have the same range -- an
> inlined
> > subroutine covering its containing subprogram exactly -- the tie was
> broken by
> > comparing the two funcinfo pointers.  Those come from bfd_zalloc, so the
> winner
> > tracked heap layout (a new objalloc chunk can land below the old one),
> and
> > addr2line named a different function from run to run on the same binary;
> only
> > the function and its inlined frames varied, never file and line.
> >
> > Order by unit_offset instead.  It does not move with the heap, and it
> restores
> > the pre-089e3718bd8 result: that algorithm walked the prepend-built
> > function_table and kept the last-parsed among equals, i.e. the greater
> DW_AT
> > offset.  An inlined subroutine's DIE is a child of its subprogram's, so
> it has
> > the greater offset and now wins every time -- the innermost routine, as
> > intended.  The offsets are unique within the one comp_unit these
> candidates
> > share.
> >
> > This only changes output that was already unstable: funcinfos are
> allocated in
> > increasing-offset scan order, so within a single chunk pointer order and
> offset
> > order agree and both rules pick the same function -- verified
> byte-identical
> > over 164k addresses across ten inlined binaries.  Forcing the divergence
> needs
> > a chunk placed below its predecessor, which test input cannot, so the
> added
> > dw2-inline-tie test is a regression pin (inverting the compare to <
> fails it),
> > not a trigger.
> >
> > bfd/
> >       * dwarf2.c (lookup_address_in_function_table): Break equal
> >       best_fit_len ties on funcinfo->unit_offset rather than on the
> >       funcinfo pointer.
> >
> > Signed-off-by: Samuel Price <thesamprice@gmail.com>
> > Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
> > ---
> >  bfd/dwarf2.c                                     |   11 +
> >  binutils/testsuite/binutils-all/addr2line.exp    |    7 +
> >  binutils/testsuite/binutils-all/dw2-inline-tie.S |  186
> ++++++++++++++++++++++
> >  binutils/testsuite/binutils-all/dw2-inline-tie.d |   12 +
> >  4 files changed, 214 insertions(+), 2 deletions(-)
>
> This all looks good to me, if there wasn't the Co-Authored-By:. That, to
> me,
> suggests that part of the patch was AI-generated, which would be in
> conflict
> with [1].
>
> Nick - if Sam came back saying the patch itself wasn't co-authored by
> Claude,
> i.e. if this was more like an Assisted-by: tag, would that then be okay to
> accept without asking further questions? Further, had the tag been absent,
> I
> likely wouldn't have recognized any possible AI origin. Which puts under
> question how to actually put the policy into practical effect.
>
> Jan
>
> [1] https://sourceware.org/binutils/wiki/LLM_Generated_Content
>
  

On 21.08.2026 13:58, Sam Price wrote:
> I was running a bunch of tests in parallel and was getting inconsistent
> results.  This turned out to be the issue, and fixed the results.
> 
> I was using the ai to help run the tests / debug.  I had to interact with
> it through a few iterations to get here, and verify it doesn’t change
> results .  Also the original commit message was overlay wordy.  So it’s not
> a fully automated patch.
> 
> I did read the ai usage for gcc and feel that it being under 15 lines of
> code / trivial usage and thought it fell under that.  Would qualify it for
> legally trivial.  I could manually write the commit message.  It would
> become less wordy, possibly not as clean.

FWIW, my understanding is that an AI-based commit message is fine; it's AI-
generated code which isn't. But I'll wait for feedback from Nick anyway.

Jan
  

I meant to put in this change after having got feedback from Nick, yet of
course I first ran it through testing, turning up issues:

On 18.08.2026 04:11, Sam Price wrote:
> --- /dev/null
> +++ b/binutils/testsuite/binutils-all/dw2-inline-tie.S
> @@ -0,0 +1,186 @@
> +/* Copyright (C) 2026 Free Software Foundation, Inc.
> +
> +   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, see <http://www.gnu.org/licenses/>.  */
> +
> +/* A function whose whole body came from one inlined call, so the
> +   DW_TAG_inlined_subroutine covers exactly the same range as the
> +   DW_TAG_subprogram containing it.  Two entries in the function table then
> +   have the same address range, and lookup_address_in_function_table has to
> +   break the tie.  It must pick the inlined routine -- the innermost one, and
> +   the one later in the DIE stream -- and it must pick it every time.
> +
> +   Historically the tie was broken by comparing the two funcinfo pointers,
> +   which made the answer depend on the addresses bfd_zalloc happened to
> +   return.  */
> +
> +	.text
> +	.globl outer_fn
> +	.type outer_fn, %function
> +outer_fn:
> +.Lfunc_begin:
> +	.4byte	0
> +	.4byte	0
> +.Lfunc_end:
> +	.size	outer_fn, .-outer_fn
> +
> +/* A pointer, purely so that the CU header's address_size below and the
> +   DW_FORM_addr attributes agree with the target without the test having to
> +   know how wide a pointer is.  */
> +
> +	.section .debug_ptrsize
> +.Lptr_begin:
> +	.dc.a	0
> +.Lptr_end:
> +
> +	.section .debug_info
> +.Lcu_begin:
> +	.4byte	.Lcu_end - .Lcu_start		/* Length of Compilation Unit */
> +.Lcu_start:
> +	.2byte	4				/* DWARF Version */
> +	.4byte	.Labbrev_begin			/* Offset into abbrev section */
> +	.byte	.Lptr_end - .Lptr_begin		/* Pointer size */
> +
> +	/* CU die */
> +	.uleb128	1			/* Abbrev: DW_TAG_compile_unit */
> +	.ascii		"dw2-inline-tie.c\0"	/* DW_AT_name */
> +	.byte		1			/* DW_AT_language (C) */
> +	.4byte		.Lline_begin		/* DW_AT_stmt_list */
> +	.dc.a		.Lfunc_begin		/* DW_AT_low_pc */

On sh-* this (and a few more .dc.a) yields "Error: misaligned data".

> --- /dev/null
> +++ b/binutils/testsuite/binutils-all/dw2-inline-tie.d
> @@ -0,0 +1,12 @@
> +#source: dw2-inline-tie.S
> +#addr2line: -f 0x0 0x4 -e
> +#name: addr2line, inlined subroutine covering its caller exactly
> +
> +# Both the DW_TAG_subprogram and the DW_TAG_inlined_subroutine inside it
> +# cover the same address range, so the function lookup has to break a tie.
> +# The inlined routine must win, at both addresses, on every run.
> +
> +inlined_fn
> +.*dw2-inline-tie\.c:17
> +inlined_fn
> +.*dw2-inline-tie\.c:17

On alpha-* and ia64-* this fails with

.../binutils/addr2line: DWARF error: mangled line number section

(There are also failures for targets which don't have gas built, but that's no
different from other binutils-all testcases requiring gas.)

Jan
  

Weird I wonder if the test is catching something existing , I tested this
on a m1 Mac,  for microprocessor code.

Was trying to profile a bunch of different test programs and aggregate the
results together.
Which put memory pressure on that malloc call.  And was causing weird
results when combing results.

I’ll dig into it more next week when I get home from travel.  If possible
could you run that test without the source change?  Would let us know if
it’s an existing test bug or from that 1 line patch?

Sincerely,

Sam Price


On Fri, Sep 4, 2026 at 4:41 AM Jan Beulich <jbeulich@suse.com> wrote:

> I meant to put in this change after having got feedback from Nick, yet of
> course I first ran it through testing, turning up issues:
>
> On 18.08.2026 04:11, Sam Price wrote:
> > --- /dev/null
> > +++ b/binutils/testsuite/binutils-all/dw2-inline-tie.S
> > @@ -0,0 +1,186 @@
> > +/* Copyright (C) 2026 Free Software Foundation, Inc.
> > +
> > +   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, see <http://www.gnu.org/licenses/>.
> */
> > +
> > +/* A function whose whole body came from one inlined call, so the
> > +   DW_TAG_inlined_subroutine covers exactly the same range as the
> > +   DW_TAG_subprogram containing it.  Two entries in the function table
> then
> > +   have the same address range, and lookup_address_in_function_table
> has to
> > +   break the tie.  It must pick the inlined routine -- the innermost
> one, and
> > +   the one later in the DIE stream -- and it must pick it every time.
> > +
> > +   Historically the tie was broken by comparing the two funcinfo
> pointers,
> > +   which made the answer depend on the addresses bfd_zalloc happened to
> > +   return.  */
> > +
> > +     .text
> > +     .globl outer_fn
> > +     .type outer_fn, %function
> > +outer_fn:
> > +.Lfunc_begin:
> > +     .4byte  0
> > +     .4byte  0
> > +.Lfunc_end:
> > +     .size   outer_fn, .-outer_fn
> > +
> > +/* A pointer, purely so that the CU header's address_size below and the
> > +   DW_FORM_addr attributes agree with the target without the test
> having to
> > +   know how wide a pointer is.  */
> > +
> > +     .section .debug_ptrsize
> > +.Lptr_begin:
> > +     .dc.a   0
> > +.Lptr_end:
> > +
> > +     .section .debug_info
> > +.Lcu_begin:
> > +     .4byte  .Lcu_end - .Lcu_start           /* Length of Compilation
> Unit */
> > +.Lcu_start:
> > +     .2byte  4                               /* DWARF Version */
> > +     .4byte  .Labbrev_begin                  /* Offset into abbrev
> section */
> > +     .byte   .Lptr_end - .Lptr_begin         /* Pointer size */
> > +
> > +     /* CU die */
> > +     .uleb128        1                       /* Abbrev:
> DW_TAG_compile_unit */
> > +     .ascii          "dw2-inline-tie.c\0"    /* DW_AT_name */
> > +     .byte           1                       /* DW_AT_language (C) */
> > +     .4byte          .Lline_begin            /* DW_AT_stmt_list */
> > +     .dc.a           .Lfunc_begin            /* DW_AT_low_pc */
>
> On sh-* this (and a few more .dc.a) yields "Error: misaligned data".
>
> > --- /dev/null
> > +++ b/binutils/testsuite/binutils-all/dw2-inline-tie.d
> > @@ -0,0 +1,12 @@
> > +#source: dw2-inline-tie.S
> > +#addr2line: -f 0x0 0x4 -e
> > +#name: addr2line, inlined subroutine covering its caller exactly
> > +
> > +# Both the DW_TAG_subprogram and the DW_TAG_inlined_subroutine inside it
> > +# cover the same address range, so the function lookup has to break a
> tie.
> > +# The inlined routine must win, at both addresses, on every run.
> > +
> > +inlined_fn
> > +.*dw2-inline-tie\.c:17
> > +inlined_fn
> > +.*dw2-inline-tie\.c:17
>
> On alpha-* and ia64-* this fails with
>
> .../binutils/addr2line: DWARF error: mangled line number section
>
> (There are also failures for targets which don't have gas built, but
> that's no
> different from other binutils-all testcases requiring gas.)
>
> Jan
>
  

It’s definitely that test I’ll debug and get back to you.  Sorry about the
emails. And my mobile kept the history of the last conversation.  Will look
into how to turn that feature off also.


Sincerely,
  

On 04.09.2026 14:00, Sam Price wrote:
> Weird I wonder if the test is catching something existing , I tested this
> on a m1 Mac,  for microprocessor code.
> 
> Was trying to profile a bunch of different test programs and aggregate the
> results together.
> Which put memory pressure on that malloc call.  And was causing weird
> results when combing results.
> 
> I’ll dig into it more next week when I get home from travel.  If possible
> could you run that test without the source change?  Would let us know if
> it’s an existing test bug or from that 1 line patch?

It's an issue with the testcase, I think. Apparently .dc.a can have alignment
enforced upon it, which you don't want here. For ia64-linux I'm seeing an
extra 0x00 byte inserted there (i.e. before the DW_LNS_advance_line).

Jan
  

Hi Jan,
I could do one of the following.
- Delete that test.  It is adding code coverage for the single fall
through line. I don't think coverage is a metric for addr2line.
- Flag the test to not run for those platforms. ia64-linux lost linux
support in 2023 I think.
- I don't know how to test those platforms at the moment, I could dig
into this more but it might be a while.
- Move the assembly to C in the existing addr2line test.  Use inline
functions inside of a function.  I tested this and it hits those lines
(with temporary debug prints).
I probably would lean towards just deleting the test, and reducing the
wordy comments, and resubmitting the "trivial" patch as a V2.

Thanks,
Sam

On Fri, Sep 4, 2026 at 8:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 04.09.2026 14:00, Sam Price wrote:
> > Weird I wonder if the test is catching something existing , I tested this
> > on a m1 Mac,  for microprocessor code.
> >
> > Was trying to profile a bunch of different test programs and aggregate the
> > results together.
> > Which put memory pressure on that malloc call.  And was causing weird
> > results when combing results.
> >
> > I’ll dig into it more next week when I get home from travel.  If possible
> > could you run that test without the source change?  Would let us know if
> > it’s an existing test bug or from that 1 line patch?
>
> It's an issue with the testcase, I think. Apparently .dc.a can have alignment
> enforced upon it, which you don't want here. For ia64-linux I'm seeing an
> extra 0x00 byte inserted there (i.e. before the DW_LNS_advance_line).
>
> Jan
  

On 08.09.2026 16:39, Sam Price wrote:
> I could do one of the following.
> - Delete that test.  It is adding code coverage for the single fall
> through line. I don't think coverage is a metric for addr2line.
> - Flag the test to not run for those platforms. ia64-linux lost linux
> support in 2023 I think.
> - I don't know how to test those platforms at the moment, I could dig
> into this more but it might be a while.
> - Move the assembly to C in the existing addr2line test.  Use inline
> functions inside of a function.  I tested this and it hits those lines
> (with temporary debug prints).
> I probably would lean towards just deleting the test, and reducing the
> wordy comments, and resubmitting the "trivial" patch as a V2.

If making the test work on specific targets is difficult, just exclude
those targets (ideally leaving a comment as to why). We want a test in
the common case at least.

As to testing for such targets (taking ia64/Linux as example), you'd
simply pass --target=ia64-linux to ./configure, then build and check
normally. If it wasn't that simple, I likely wouldn't have noticed the
issue.

Jan
  

I had 2am night brain working that.  For some reason I was thinking I
needed to spin up qemu and build/run the addr2line on an emulator.
I will do the cross compile test.