Bug 27368 - [dwz, dwarf5] Handle .debug_macro version 5
Summary: [dwz, dwarf5] Handle .debug_macro version 5
Status: RESOLVED FIXED
Alias: None
Product: dwz
Classification: Unclassified
Component: default (show other bugs)
Version: unspecified
: P2 enhancement
Target Milestone: ---
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-08 10:51 UTC by Tom de Vries
Modified: 2021-02-09 07:42 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom de Vries 2021-02-08 10:51:41 UTC
When doing:
...
$ make clean; make check CC="gcc -ggdb3"
...
I get:
...
dwz: 1: Unhandled .debug_macro version 5
dwz: 2: Unhandled .debug_macro version 5
dwz: No suitable DWARF found for multifile optimization
FAIL: testsuite/dwz.tests/multifile.sh
...
Comment 1 Tom de Vries 2021-02-08 10:54:42 UTC
> $ make clean; make check CC="gcc -ggdb3"

Should have been:
...
$ make clean; make check CC="gcc -ggdb3 -gdwarf-5"                                      
...
Comment 2 Tom de Vries 2021-02-08 10:55:20 UTC
Using:
...
$ git diff
diff --git a/dwz.c b/dwz.c
index 10860bf..5b5be1b 100644
--- a/dwz.c
+++ b/dwz.c
@@ -9844,7 +9844,8 @@ read_macro (DSO *dso)
        }
 
       version = read_16 (ptr);
-      if (version != 4)
+      bool supported_version_p = 4 <= version && version <= 5;
+      if (!supported_version_p)
        {
          error (0, 0, "%s: Unhandled .debug_macro version %d", dso->filename,
                 version);
...
I get:
...
                === dwz Summary ===

# of expected passes            63
# of unsupported tests          1
...
Comment 3 Jakub Jelinek 2021-02-08 10:57:25 UTC
I'm not sure I like the 4 <= version operand order, can't you just do
if (version < 4 || version > 5)
?
Comment 4 Tom de Vries 2021-02-08 11:28:34 UTC
(In reply to Jakub Jelinek from comment #3)
> I'm not sure I like the 4 <= version operand order, can't you just do
> if (version < 4 || version > 5)
> ?

I can, of course.

But I don't like that one.  My feeling is that the supported_version_p is easier to read, and requires less mental arithmetic.
Comment 5 Jakub Jelinek 2021-02-08 11:36:27 UTC
I'm fine with the temporary variable, but can you swap the operands on 4 <= version at least?  At least in GCC we want constants on the rhs of comparisons and dwz follows or used to follow that.
Comment 6 Tom de Vries 2021-02-08 11:43:08 UTC
(In reply to Jakub Jelinek from comment #5)
> I'm fine with the temporary variable, but can you swap the operands on 4 <=
> version at least?  At least in GCC we want constants on the rhs of
> comparisons and dwz follows or used to follow that.

Ah, I see.  Sure, will do.

FWIW, I like this notation:
...
  4 <= version && version <= 5
...
because it most closely mimics:
...
  4 <= version <= 5
...
Comment 7 Mark Wielaard 2021-02-08 14:39:45 UTC
The version check should be fine.
My preference would simply be version != 4 && version !=5.

This should fine as is with GCC DWARF5 since:

    DW_MACRO_GNU_define ==              DW_MACRO_define
    DW_MACRO_GNU_undef ==               DW_MACRO_undef
    DW_MACRO_GNU_start_file ==          DW_MACRO_start_file
    DW_MACRO_GNU_end_file ==            DW_MACRO_end_file
    DW_MACRO_GNU_define_indirect ==     DW_MACRO_define_strp
    DW_MACRO_GNU_undef_indirect ==      DW_MACRO_undef_strp
    DW_MACRO_GNU_transparent_include == DW_MACRO_import

That leaves:

    DW_MACRO_define_strx
    DW_MACRO_undef_strx

But those are never generated by GCC (see also DW_FORM_strx[1234] support).

dwz itself could generate (probably behind a --dwarf5 flag):

    DW_MACRO_define_sup
    DW_MACRO_undef_sup
    DW_MACRO_import_sup
Comment 8 Tom de Vries 2021-02-09 07:36:13 UTC
Patch allowing version 5 .debug_macro committed:

https://sourceware.org/git/?p=dwz.git;a=commit;h=b2e260969ad591ac1b46ea05dc3df4a73a4b4796
Comment 9 Tom de Vries 2021-02-09 07:42:10 UTC
(In reply to Mark Wielaard from comment #7)
> The version check should be fine.
> My preference would simply be version != 4 && version !=5.
> 
> This should fine as is with GCC DWARF5 since:
> 
>     DW_MACRO_GNU_define ==              DW_MACRO_define
>     DW_MACRO_GNU_undef ==               DW_MACRO_undef
>     DW_MACRO_GNU_start_file ==          DW_MACRO_start_file
>     DW_MACRO_GNU_end_file ==            DW_MACRO_end_file
>     DW_MACRO_GNU_define_indirect ==     DW_MACRO_define_strp
>     DW_MACRO_GNU_undef_indirect ==      DW_MACRO_undef_strp
>     DW_MACRO_GNU_transparent_include == DW_MACRO_import
> 
> That leaves:
> 
>     DW_MACRO_define_strx
>     DW_MACRO_undef_strx
> 
> But those are never generated by GCC (see also DW_FORM_strx[1234] support).
> 
> dwz itself could generate (probably behind a --dwarf5 flag):
> 
>     DW_MACRO_define_sup
>     DW_MACRO_undef_sup
>     DW_MACRO_import_sup

Thanks for the detailed explanation.

I'm closing this PR, since I understand from this that .debug_macro 5 is now sufficiently supported.