[v2,gdb/symtab] Fix dwarf version of DWO TU

Message ID 20250120085723.5693-1-tdevries@suse.de
State New
Headers
Series [v2,gdb/symtab] Fix dwarf version of DWO TU |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed

Commit Message

Tom de Vries Jan. 20, 2025, 8:57 a.m. UTC
  When running test-case gdb.ada/access_tagged_param.exp with target board
fission, we run into:
...
(gdb) break pck.adb:19^M
gdb/dwarf2/read.h:289: internal-error: version: \
  Assertion `m_dwarf_version != 0' failed.^M
...

The assertion happens when calling cu->per_cu->version () in this workaround
in read_subroutine_type:
...
  /* PR gas/29517 occurs in 2.39, and is fixed in 2.40, but it's only fixed
     for dwarf version >= 3 which supports DW_TAG_unspecified_type.  */
  if (type->code () == TYPE_CODE_VOID
      && !type->is_stub ()
      && die->child == nullptr
      && (cu->per_cu->version () == 2
	  || producer_is_gas_2_39 (cu)))
    {
...
for a TU in a dwo file.

Fix this by using cu->header.version instead.

Tested on aarch64-linux.

PR symtab/32309
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32309
---
 gdb/dwarf2/read.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: c99345db064e1bd645bc08db65174bae7ac20b0e
  

Comments

Tom Tromey Jan. 21, 2025, 1:32 p.m. UTC | #1
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> When running test-case gdb.ada/access_tagged_param.exp with target board
Tom> fission, we run into:
Tom> ...
Tom> (gdb) break pck.adb:19^M
Tom> gdb/dwarf2/read.h:289: internal-error: version: \
Tom>   Assertion `m_dwarf_version != 0' failed.^M
Tom> ...

Tom> -      && (cu->per_cu->version () == 2
Tom> +      && (cu->header.version == 2

All the stuff in this area seems kind of horrible to me.  Like, first of
all, how can this even fail?  It seems like the CU version should be set
when setting up the reader.  And if not, what's gone wrong there?
Relatedly, can the other callers of version() fail in the same way?
Finally there aren't really that many callers of this method so I wonder
if it can be removed, maybe making this code less fragile.  Looking at
it, I feel pretty sure if I needed a version check I'd probably write
code to call this method but ... maybe that's unsafe?

Tom
  
Tom de Vries Jan. 21, 2025, 3:09 p.m. UTC | #2
On 1/21/25 14:32, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> When running test-case gdb.ada/access_tagged_param.exp with target board
> Tom> fission, we run into:
> Tom> ...
> Tom> (gdb) break pck.adb:19^M
> Tom> gdb/dwarf2/read.h:289: internal-error: version: \
> Tom>   Assertion `m_dwarf_version != 0' failed.^M
> Tom> ...
> 
> Tom> -      && (cu->per_cu->version () == 2
> Tom> +      && (cu->header.version == 2
> 
> All the stuff in this area seems kind of horrible to me.  Like, first of
> all, how can this even fail?  It seems like the CU version should be set
> when setting up the reader. 

Hi Tom,

thanks for the review.

The v1 ( 
https://sourceware.org/pipermail/gdb-patches/2024-October/212676.html ) 
takes the approach of making sure "cu->per_cu->version ()" is set.

Do you prefer that one?

Thanks,
- Tom

> And if not, what's gone wrong there?
> Relatedly, can the other callers of version() fail in the same way?
> Finally there aren't really that many callers of this method so I wonder
> if it can be removed, maybe making this code less fragile.  Looking at
> it, I feel pretty sure if I needed a version check I'd probably write
> code to call this method but ... maybe that's unsafe?
> 
> Tom
  

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4503977d62b..824604261f5 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14019,7 +14019,7 @@  read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
   if (type->code () == TYPE_CODE_VOID
       && !type->is_stub ()
       && die->child == nullptr
-      && (cu->per_cu->version () == 2
+      && (cu->header.version == 2
 	  || producer_is_gas_2_39 (cu)))
     {
       /* Work around PR gas/29517, pretend we have an DW_TAG_unspecified_type