diff mbox series

ld/pdb: Read inlinee-lines signature at correct offset

Message ID 20260930193657.733-2-oleg.tolmatcev@gmail.com
State New
Headers
Series ld/pdb: Read inlinee-lines signature at correct offset |

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-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Oleg Tolmatcev Sept. 30, 2026, 7:36 p.m. UTC 
  parse_inlinee_lines receives a pointer to the subsection payload, so
read its signature from the start rather than from the first entry file
ID.

Give the first test entry a nonzero file ID so the incorrect read is
observable, and report the resulting link failure as a test failure.

ld/
	* pdb.c (parse_inlinee_lines): Read the signature at the start of the
	payload.
	* testsuite/ld-pe/pdb-inlineelines1b.s: Give the first inlinee a
	nonzero file ID.
	* testsuite/ld-pe/pdb-inlineelines1-c13-info2.d: Update expected
	output.
	* testsuite/ld-pe/pdb.exp (test10): Update expected file ID and treat
	a linker failure as a test failure.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32942
---
 ld/pdb.c                                         | 2 +-
 ld/testsuite/ld-pe/pdb-inlineelines1-c13-info2.d | 2 +-
 ld/testsuite/ld-pe/pdb-inlineelines1b.s          | 4 ++--
 ld/testsuite/ld-pe/pdb.exp                       | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

Comments

Jan Beulich Oct. 2, 2026, 9:50 a.m. UTC | #1 
On 30.09.2026 21:36, Oleg Tolmatcev wrote:
> parse_inlinee_lines receives a pointer to the subsection payload, so
> read its signature from the start rather than from the first entry file
> ID.
> 
> Give the first test entry a nonzero file ID so the incorrect read is
> observable, and report the resulting link failure as a test failure.
> 
> ld/
> 	* pdb.c (parse_inlinee_lines): Read the signature at the start of the
> 	payload.
> 	* testsuite/ld-pe/pdb-inlineelines1b.s: Give the first inlinee a
> 	nonzero file ID.
> 	* testsuite/ld-pe/pdb-inlineelines1-c13-info2.d: Update expected
> 	output.
> 	* testsuite/ld-pe/pdb.exp (test10): Update expected file ID and treat
> 	a linker failure as a test failure.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32942
> ---
>  ld/pdb.c                                         | 2 +-
>  ld/testsuite/ld-pe/pdb-inlineelines1-c13-info2.d | 2 +-
>  ld/testsuite/ld-pe/pdb-inlineelines1b.s          | 4 ++--
>  ld/testsuite/ld-pe/pdb.exp                       | 4 ++--
>  4 files changed, 6 insertions(+), 6 deletions(-)

Looks plausible to me, but - Mark: Can you please take a look? Such ...

> --- a/ld/pdb.c
> +++ b/ld/pdb.c
> @@ -1879,7 +1879,7 @@ parse_inlinee_lines (uint8_t *data, uint32_t size, uint8_t **bufptr,
>        return false;
>      }
>  
> -  version = bfd_getl32 (data + sizeof (uint32_t) + sizeof (uint32_t));
> +  version = bfd_getl32 (data);
>    if (version != CV_INLINEE_SOURCE_LINE_SIGNATURE)
>      {
>        einfo (_("%P: warning: unexpected DEBUG_S_INLINEELINES version %u\n"),

... uncommented custom adjustment are always a little suspicious. I'm
happy to seem this go away now.

Oleg - in the referenced bug report there is a patch, yet that patch
contains a 2nd change. That looks unrelated, but still potentially
applicable in general (as a standalone change)?

Jan
Oleg Tolmatcev Oct. 2, 2026, 5:35 p.m. UTC | #2 
пт, 2 окт. 2026 г. в 11:50, Jan Beulich <jbeulich@suse.com>:
>
> On 30.09.2026 21:36, Oleg Tolmatcev wrote:
> > parse_inlinee_lines receives a pointer to the subsection payload, so
> > read its signature from the start rather than from the first entry file
> > ID.
> >
> > Give the first test entry a nonzero file ID so the incorrect read is
> > observable, and report the resulting link failure as a test failure.
> >
> > ld/
> >       * pdb.c (parse_inlinee_lines): Read the signature at the start of the
> >       payload.
> >       * testsuite/ld-pe/pdb-inlineelines1b.s: Give the first inlinee a
> >       nonzero file ID.
> >       * testsuite/ld-pe/pdb-inlineelines1-c13-info2.d: Update expected
> >       output.
> >       * testsuite/ld-pe/pdb.exp (test10): Update expected file ID and treat
> >       a linker failure as a test failure.
> >
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32942
> > ---
> >  ld/pdb.c                                         | 2 +-
> >  ld/testsuite/ld-pe/pdb-inlineelines1-c13-info2.d | 2 +-
> >  ld/testsuite/ld-pe/pdb-inlineelines1b.s          | 4 ++--
> >  ld/testsuite/ld-pe/pdb.exp                       | 4 ++--
> >  4 files changed, 6 insertions(+), 6 deletions(-)
>
> Looks plausible to me, but - Mark: Can you please take a look? Such ...
>
> > --- a/ld/pdb.c
> > +++ b/ld/pdb.c
> > @@ -1879,7 +1879,7 @@ parse_inlinee_lines (uint8_t *data, uint32_t size, uint8_t **bufptr,
> >        return false;
> >      }
> >
> > -  version = bfd_getl32 (data + sizeof (uint32_t) + sizeof (uint32_t));
> > +  version = bfd_getl32 (data);
> >    if (version != CV_INLINEE_SOURCE_LINE_SIGNATURE)
> >      {
> >        einfo (_("%P: warning: unexpected DEBUG_S_INLINEELINES version %u\n"),
>
> ... uncommented custom adjustment are always a little suspicious. I'm
> happy to seem this go away now.

I will try to answer. LLVM's
DebugInlineeLinesSubsectionRef::initialize reads the signature as the
first field of the subsection payload.  The writer accounts for it in
DebugInlineeLinesSubsection::calculateSerializedSize, whose comment
says "4 bytes for the signature", and emits it first in commit(). See
https://github.com/llvm/llvm-project/blob/61914fd3ce4100d61859471f4fbc48db82185c0a/llvm/lib/DebugInfo/CodeView/DebugInlineeLinesSubsection.cpp#L46
and https://github.com/llvm/llvm-project/blob/61914fd3ce4100d61859471f4fbc48db82185c0a/llvm/lib/DebugInfo/CodeView/DebugInlineeLinesSubsection.cpp#L67.

> Oleg - in the referenced bug report there is a patch, yet that patch
> contains a 2nd change. That looks unrelated, but still potentially
> applicable in general (as a standalone change)?
>
> Jan

The 2nd change is in a separate patch.

Oleg
diff mbox series

Patch

diff --git a/ld/pdb.c b/ld/pdb.c
index 54237349306..434bdb197ca 100644
--- a/ld/pdb.c
+++ b/ld/pdb.c
@@ -1879,7 +1879,7 @@  parse_inlinee_lines (uint8_t *data, uint32_t size, uint8_t **bufptr,
       return false;
     }
 
-  version = bfd_getl32 (data + sizeof (uint32_t) + sizeof (uint32_t));
+  version = bfd_getl32 (data);
   if (version != CV_INLINEE_SOURCE_LINE_SIGNATURE)
     {
       einfo (_("%P: warning: unexpected DEBUG_S_INLINEELINES version %u\n"),
diff --git a/ld/testsuite/ld-pe/pdb-inlineelines1-c13-info2.d b/ld/testsuite/ld-pe/pdb-inlineelines1-c13-info2.d
index ac4e49bbe46..662ecb26669 100644
--- a/ld/testsuite/ld-pe/pdb-inlineelines1-c13-info2.d
+++ b/ld/testsuite/ld-pe/pdb-inlineelines1-c13-info2.d
@@ -6,5 +6,5 @@  Contents of section .data:
  0010 2301efcd ab8998ba dcfe1023 45670000  #..........#Eg..
  0020 08000000 100198ba dcfe1023 45676745  ...........#EggE
  0030 2301efcd ab890000 f6000000 1c000000  #...............
- 0040 00000000 02100000 00000000 2a000000  ............*...
+ 0040 00000000 02100000 18000000 2a000000  ............*...
  0050 03100000 18000000 1c000000           ............    
diff --git a/ld/testsuite/ld-pe/pdb-inlineelines1b.s b/ld/testsuite/ld-pe/pdb-inlineelines1b.s
index d8d917068f3..23340280d78 100644
--- a/ld/testsuite/ld-pe/pdb-inlineelines1b.s
+++ b/ld/testsuite/ld-pe/pdb-inlineelines1b.s
@@ -138,7 +138,7 @@ 
   *** INLINEE LINES
 
   InlineeId  FileId  StaringLine
-       1003       0           42
+       1003      18           42
        1004      18           28
 */
 
@@ -150,7 +150,7 @@ 
 .long CV_INLINEE_SOURCE_LINE_SIGNATURE
 
 .long 0x1003
-.long .file1 - .chksms_start
+.long .file2 - .chksms_start
 .long 42
 
 .long 0x1004
diff --git a/ld/testsuite/ld-pe/pdb.exp b/ld/testsuite/ld-pe/pdb.exp
index 11e950f0e64..1439baa4bf8 100644
--- a/ld/testsuite/ld-pe/pdb.exp
+++ b/ld/testsuite/ld-pe/pdb.exp
@@ -1763,7 +1763,7 @@  proc test10 { } {
     }
 
     if ![ld_link $ld "tmpdir/pdb-inlineelines1.exe" "--pdb=tmpdir/pdb-inlineelines1.pdb tmpdir/pdb-inlineelines1a.o tmpdir/pdb-inlineelines1b.o"] {
-	unsupported "Create PE image with PDB file"
+	fail "Create PE image with PDB file"
 	return
     }
 
@@ -1805,7 +1805,7 @@  proc test10 { } {
     # *** INLINEE LINES
     #
     # InlineeId  FileId  StaringLine
-    #      1002       0           42
+    #      1002       1           42
     #      1003       1           28
 
     # For some reason it numbers file IDs in bytes for object files but as an