[PATCH] Fix gdb.dwarf2/dw2-double-set-die-type.exp with Clang

[PATCH] Fix gdb.dwarf2/dw2-double-set-die-type.exp with Clang

[Gary Benson]

Hi all,

gdb.dwarf2/dw2-double-set-die-type.exp failed to build with Clang,
because of the following issues:

- One .uleb128 directive was specified with an uppercase U,
  causing Clang to fail with the message:
    error: unknown directive
  This commit converts that directive to all-lowercase.

- The label ".Labbrev1_begin" was referenced but not defined,
  causing Clang to fail with the message:
    <unknown>:0: error: Undefined temporary symbol
  This commit adds the label in the appropriate place.

- The label ".Ldebug_line0" was referenced but not defined,
  causing Clang to fail with the message:
    <unknown>:0: error: Undefined temporary symbol
  This commit removes the reference.

Checked on Fedora 32 x86_64, using GCC and Clang.

First, is this ok to commit?

Second, I want to report these issues to the relevang upstream,
but I'm struggling to find documentation.  Does anybody know,
or have an opinion about:

 1) Is it Clang's error that it doesn't accept .Uleb128 as a
    valid directive, or is it gas's error that it does?

 2) Is it Clang's error that it bombs on undefined labels, or
    is it gas's error that it doesn't?

Thanks,
Gary

--
gdb/testsuite/ChangeLog:

	* gdb.dwarf2/dw2-double-set-die-type.S (.Ldie_3e0):
	Convert directive to lowercase.
	(.Labbrev1_begin): Add missing label.
	(abbrev code 0x1): Remove DW_AT_stmt_list.
	(.Ldie_b): Likewise.
---
 gdb/testsuite/ChangeLog                            | 8 ++++++++
 gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.S | 6 ++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.S b/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.S
index 3a31e03..d09a77b 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.S
+++ b/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.S
@@ -50,7 +50,6 @@
 	.ascii	"GNU C++ 4.4.3 20100127 (Red Hat 4.4.3-4)\0"	/* DW_AT_producer */
 	.byte	0x4	/* DW_AT_language */
 	.ascii	"duplicate-type.cc\0"	/* DW_AT_name */
-	.4byte	.Ldebug_line0	/* DW_AT_stmt_list */
 
 .Ldie_38:
 	.uleb128 0x3	/* (DIE (0x38) DW_TAG_typedef) */
@@ -247,7 +246,7 @@
 	.4byte	OFFSET (3d5)	/* DW_AT_type */
 
 .Ldie_3e0:
-	.Uleb128 0x20	/* (DIE (0x3e0) DW_TAG_class_type) */
+	.uleb128 0x20	/* (DIE (0x3e0) DW_TAG_class_type) */
 	.4byte	OFFSET (70)	/* DW_AT_specification */
 	.byte	0x1	/* DW_AT_byte_size */
 	.4byte	OFFSET (44e)	/* DW_AT_sibling */
@@ -351,6 +350,7 @@
 
 /* Abbrev table */
 	.section	.debug_abbrev
+.Labbrev1_begin:
 	.uleb128 0x1	/* (abbrev code) */
 	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
 	.byte	0x1	/* DW_children_yes */
@@ -360,8 +360,6 @@
 	.uleb128 0xb	/* (DW_FORM_data1) */
 	.uleb128 0x3	/* (DW_AT_name) */
 	.uleb128 0x8	/* (DW_FORM_sting) */
-	.uleb128 0x10	/* (DW_AT_stmt_list) */
-	.uleb128 0x6	/* (DW_FORM_data4) */
 	.byte	0x0
 	.byte	0x0
 
-- 
1.8.3.1

[PING][PATCH] Fix gdb.dwarf2/dw2-double-set-die-type.exp with Clang

[Gary Benson]

gdb-patches wrote:
> Hi all,
> 
> gdb.dwarf2/dw2-double-set-die-type.exp failed to build with Clang,
> because of the following issues:
> 
> - One .uleb128 directive was specified with an uppercase U,
>   causing Clang to fail with the message:
>     error: unknown directive
>   This commit converts that directive to all-lowercase.
> 
> - The label ".Labbrev1_begin" was referenced but not defined,
>   causing Clang to fail with the message:
>     <unknown>:0: error: Undefined temporary symbol
>   This commit adds the label in the appropriate place.
> 
> - The label ".Ldebug_line0" was referenced but not defined,
>   causing Clang to fail with the message:
>     <unknown>:0: error: Undefined temporary symbol
>   This commit removes the reference.
> 
> Checked on Fedora 32 x86_64, using GCC and Clang.
> 
> First, is this ok to commit?
> 
> Second, I want to report these issues to the relevang upstream,
> but I'm struggling to find documentation.  Does anybody know,
> or have an opinion about:
> 
>  1) Is it Clang's error that it doesn't accept .Uleb128 as a
>     valid directive, or is it gas's error that it does?
> 
>  2) Is it Clang's error that it bombs on undefined labels, or
>     is it gas's error that it doesn't?
> 
> Thanks,
> Gary
> 
> --
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.dwarf2/dw2-double-set-die-type.S (.Ldie_3e0):
> 	Convert directive to lowercase.
> 	(.Labbrev1_begin): Add missing label.
> 	(abbrev code 0x1): Remove DW_AT_stmt_list.
> 	(.Ldie_b): Likewise.
> ---
>  gdb/testsuite/ChangeLog                            | 8 ++++++++
>  gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.S | 6 ++----
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.S b/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.S
> index 3a31e03..d09a77b 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.S
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-double-set-die-type.S
> @@ -50,7 +50,6 @@
>  	.ascii	"GNU C++ 4.4.3 20100127 (Red Hat 4.4.3-4)\0"	/* DW_AT_producer */
>  	.byte	0x4	/* DW_AT_language */
>  	.ascii	"duplicate-type.cc\0"	/* DW_AT_name */
> -	.4byte	.Ldebug_line0	/* DW_AT_stmt_list */
>  
>  .Ldie_38:
>  	.uleb128 0x3	/* (DIE (0x38) DW_TAG_typedef) */
> @@ -247,7 +246,7 @@
>  	.4byte	OFFSET (3d5)	/* DW_AT_type */
>  
>  .Ldie_3e0:
> -	.Uleb128 0x20	/* (DIE (0x3e0) DW_TAG_class_type) */
> +	.uleb128 0x20	/* (DIE (0x3e0) DW_TAG_class_type) */
>  	.4byte	OFFSET (70)	/* DW_AT_specification */
>  	.byte	0x1	/* DW_AT_byte_size */
>  	.4byte	OFFSET (44e)	/* DW_AT_sibling */
> @@ -351,6 +350,7 @@
>  
>  /* Abbrev table */
>  	.section	.debug_abbrev
> +.Labbrev1_begin:
>  	.uleb128 0x1	/* (abbrev code) */
>  	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
>  	.byte	0x1	/* DW_children_yes */
> @@ -360,8 +360,6 @@
>  	.uleb128 0xb	/* (DW_FORM_data1) */
>  	.uleb128 0x3	/* (DW_AT_name) */
>  	.uleb128 0x8	/* (DW_FORM_sting) */
> -	.uleb128 0x10	/* (DW_AT_stmt_list) */
> -	.uleb128 0x6	/* (DW_FORM_data4) */
>  	.byte	0x0
>  	.byte	0x0
>  
> -- 
> 1.8.3.1
> 

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

Re: [PING][PATCH] Fix gdb.dwarf2/dw2-double-set-die-type.exp with Clang

[Joel Brobecker]

Hi Gary,

> > gdb/testsuite/ChangeLog:
> > 
> > 	* gdb.dwarf2/dw2-double-set-die-type.S (.Ldie_3e0):
> > 	Convert directive to lowercase.
> > 	(.Labbrev1_begin): Add missing label.
> > 	(abbrev code 0x1): Remove DW_AT_stmt_list.
> > 	(.Ldie_b): Likewise.

Apologies for the delay in reviewing. This looks good to me.

> > Second, I want to report these issues to the relevang upstream,
> > but I'm struggling to find documentation.  Does anybody know,
> > or have an opinion about:
> > 
> >  1) Is it Clang's error that it doesn't accept .Uleb128 as a
> >     valid directive, or is it gas's error that it does?

The GAS documentation gives a clue
(https://sourceware.org/binutils/docs-2.35/as/Pseudo-Ops.html#Pseudo-Ops):

    | All assembler directives have names that begin with a period
    | (‘.’). The names are case insensitive for most targets, and
    | usually written in lower case.

I don't know if this means that Clang is in error however.
I don't think the targets usually define the assembly language,
particularly for the pseudo ops, do they?

> >  2) Is it Clang's error that it bombs on undefined labels, or
> >     is it gas's error that it doesn't?

That, I do not know. Sounds odd indeed that there wouldn't be an error
for those, but maybe there is something behind it. It's a question for
the binutils group, who will be better place to answer those.

-- 
Joel

Re: [PING][PATCH] Fix gdb.dwarf2/dw2-double-set-die-type.exp with Clang

[Gary Benson]

Hi Joel,

Joel Brobecker wrote:
> Hi Gary,
> 
> > > gdb/testsuite/ChangeLog:
> > > 
> > > 	* gdb.dwarf2/dw2-double-set-die-type.S (.Ldie_3e0):
> > > 	Convert directive to lowercase.
> > > 	(.Labbrev1_begin): Add missing label.
> > > 	(abbrev code 0x1): Remove DW_AT_stmt_list.
> > > 	(.Ldie_b): Likewise.
> 
> Apologies for the delay in reviewing. This looks good to me.

Thank you, I've pushed this now.

> > > Second, I want to report these issues to the relevang upstream,
> > > but I'm struggling to find documentation.  Does anybody know,
> > > or have an opinion about:
> > > 
> > >  1) Is it Clang's error that it doesn't accept .Uleb128 as a
> > >     valid directive, or is it gas's error that it does?
> 
> The GAS documentation gives a clue
> (https://sourceware.org/binutils/docs-2.35/as/Pseudo-Ops.html#Pseudo-Ops):
> 
>     | All assembler directives have names that begin with a period
>     | (‘.’). The names are case insensitive for most targets, and
>     | usually written in lower case.
> 
> I don't know if this means that Clang is in error however.
> I don't think the targets usually define the assembly language,
> particularly for the pseudo ops, do they?

No, I get the impression it's always been somewhat tool-specific,
though it's not a field I have any deep knowledge in.  I'll take
this to Clang's developers, see what they think.

> > >  2) Is it Clang's error that it bombs on undefined labels, or
> > >     is it gas's error that it doesn't?
> 
> That, I do not know. Sounds odd indeed that there wouldn't be an
> error for those, but maybe there is something behind it. It's a
> question for the binutils group, who will be better place to answer
> those.

I asked Nick, he said it makes sense for gas to accept undefined
labels as the label could be defined in another file.  So again,
I'll take this to Clang's developers.

Thanks for your time Joel!

Cheers,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat