[PATCH] Fix gdb.dwarf2/clztest.exp with Clang

[PATCH] Fix gdb.dwarf2/clztest.exp with Clang

[Gary Benson via Gdb-patches]

Hi all,

Clang fails to compile gdb.dwarf2/clztest.S, with the following error:

  gdb compile failed, /gdbtest/src/gdb/testsuite/gdb.dwarf2/clztest.S:181:2:
    error: changed section flags for .eh_frame, expected: 0x2

This commit fixes the testcase by defining .eh_frame's flags
as Clang expects, when building with Clang (i.e. as "a" rather
than as "aw".)

Checked on Fedora 32 x86_64, with GCC and Clang.  Ok to commit?

Cheers,
Gary

---
gdb/testsuite/ChangeLog:

	* gdb.dwarf2/clztest.S (EH_FRAME_SH_FLAGS): New macro.
	(.eh_frame): Use the above.
---
 gdb/testsuite/ChangeLog            | 5 +++++
 gdb/testsuite/gdb.dwarf2/clztest.S | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.dwarf2/clztest.S b/gdb/testsuite/gdb.dwarf2/clztest.S
index a904fee..08a8bac 100644
--- a/gdb/testsuite/gdb.dwarf2/clztest.S
+++ b/gdb/testsuite/gdb.dwarf2/clztest.S
@@ -22,6 +22,12 @@
 
 */
 
+#if defined(__clang__)
+# define EH_FRAME_SH_FLAGS "a"
+#else
+# define EH_FRAME_SH_FLAGS "aw"
+#endif
+
 	.file	"clztest.c"
 	.text
 .Ltext0:
@@ -178,7 +184,7 @@ _start:
 .LEFDE4:
 #NO_APP
 #APP
-	.section	.eh_frame,"aw",@progbits
+	.section	.eh_frame,EH_FRAME_SH_FLAGS,@progbits
 .Lframe1:
 	.long	.LECIE1-.LSCIE1	# Length of Common Information Entry
 .LSCIE1:
-- 
1.8.3.1

Re: [PATCH] Fix gdb.dwarf2/clztest.exp with Clang

[Andreas Schwab]

On Nov 02 2020, Gary Benson via Gdb-patches wrote:

> diff --git a/gdb/testsuite/gdb.dwarf2/clztest.S b/gdb/testsuite/gdb.dwarf2/clztest.S
> index a904fee..08a8bac 100644
> --- a/gdb/testsuite/gdb.dwarf2/clztest.S
> +++ b/gdb/testsuite/gdb.dwarf2/clztest.S
> @@ -22,6 +22,12 @@
>  
>  */
>  
> +#if defined(__clang__)
> +# define EH_FRAME_SH_FLAGS "a"
> +#else
> +# define EH_FRAME_SH_FLAGS "aw"
> +#endif
> +
>  	.file	"clztest.c"
>  	.text
>  .Ltext0:
> @@ -178,7 +184,7 @@ _start:
>  .LEFDE4:
>  #NO_APP
>  #APP
> -	.section	.eh_frame,"aw",@progbits
> +	.section	.eh_frame,EH_FRAME_SH_FLAGS,@progbits

Shouldn't .eh_frame always be read-only?  It certainly is when compiled
with gcc.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] Fix gdb.dwarf2/clztest.exp with Clang

[Gary Benson via Gdb-patches]

Andreas Schwab wrote:
> On Nov 02 2020, Gary Benson via Gdb-patches wrote:
> > diff --git a/gdb/testsuite/gdb.dwarf2/clztest.S b/gdb/testsuite/gdb.dwarf2/clztest.S
> > index a904fee..08a8bac 100644
> > --- a/gdb/testsuite/gdb.dwarf2/clztest.S
> > +++ b/gdb/testsuite/gdb.dwarf2/clztest.S
> > @@ -22,6 +22,12 @@
> >  
> >  */
> >  
> > +#if defined(__clang__)
> > +# define EH_FRAME_SH_FLAGS "a"
> > +#else
> > +# define EH_FRAME_SH_FLAGS "aw"
> > +#endif
> > +
> >  	.file	"clztest.c"
> >  	.text
> >  .Ltext0:
> > @@ -178,7 +184,7 @@ _start:
> >  .LEFDE4:
> >  #NO_APP
> >  #APP
> > -	.section	.eh_frame,"aw",@progbits
> > +	.section	.eh_frame,EH_FRAME_SH_FLAGS,@progbits
> 
> Shouldn't .eh_frame always be read-only?

I don't know.

> It certainly is when compiled with gcc.

A comment in that .S file indicated it was originally generated
using GCC (via gcc -dA -S -g -O2 clztest.c -o clztest.S) but that
was 2011, so maybe things changed.

I'm happy to change the test to have .eh_frame read-only for all
compilers, if that seems more correct.

Thanks,
Gary

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

Re: [PATCH] Fix gdb.dwarf2/clztest.exp with Clang

[Andreas Schwab]

On Nov 02 2020, Gary Benson wrote:

> A comment in that .S file indicated it was originally generated
> using GCC (via gcc -dA -S -g -O2 clztest.c -o clztest.S) but that
> was 2011, so maybe things changed.

Modern compilers don't emit .eh_frame contents manually, but use cfi
directives.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] Fix gdb.dwarf2/clztest.exp with Clang

[Rainer Orth]

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Nov 02 2020, Gary Benson wrote:
>
>> A comment in that .S file indicated it was originally generated
>> using GCC (via gcc -dA -S -g -O2 clztest.c -o clztest.S) but that
>> was 2011, so maybe things changed.
>
> Modern compilers don't emit .eh_frame contents manually, but use cfi
> directives.

... if the assembler used supports those, which isn't always the case.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Fix gdb.dwarf2/clztest.exp with Clang

[Andreas Schwab]

On Nov 02 2020, Rainer Orth wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> On Nov 02 2020, Gary Benson wrote:
>>
>>> A comment in that .S file indicated it was originally generated
>>> using GCC (via gcc -dA -S -g -O2 clztest.c -o clztest.S) but that
>>> was 2011, so maybe things changed.
>>
>> Modern compilers don't emit .eh_frame contents manually, but use cfi
>> directives.
>
> ... if the assembler used supports those, which isn't always the case.

So let's say modern toolchains instead. :-)

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] Fix gdb.dwarf2/clztest.exp with Clang

[Rainer Orth]

Hi Gary,

> Andreas Schwab wrote:
>> On Nov 02 2020, Gary Benson via Gdb-patches wrote:
>> > diff --git a/gdb/testsuite/gdb.dwarf2/clztest.S b/gdb/testsuite/gdb.dwarf2/clztest.S
>> > index a904fee..08a8bac 100644
>> > --- a/gdb/testsuite/gdb.dwarf2/clztest.S
>> > +++ b/gdb/testsuite/gdb.dwarf2/clztest.S
>> > @@ -22,6 +22,12 @@
>> >  
>> >  */
>> >  
>> > +#if defined(__clang__)
>> > +# define EH_FRAME_SH_FLAGS "a"
>> > +#else
>> > +# define EH_FRAME_SH_FLAGS "aw"
>> > +#endif
>> > +
>> >  	.file	"clztest.c"
>> >  	.text
>> >  .Ltext0:
>> > @@ -178,7 +184,7 @@ _start:
>> >  .LEFDE4:
>> >  #NO_APP
>> >  #APP
>> > -	.section	.eh_frame,"aw",@progbits
>> > +	.section	.eh_frame,EH_FRAME_SH_FLAGS,@progbits
>> 
>> Shouldn't .eh_frame always be read-only?
>
> I don't know.
>
>> It certainly is when compiled with gcc.
>
> A comment in that .S file indicated it was originally generated
> using GCC (via gcc -dA -S -g -O2 clztest.c -o clztest.S) but that
> was 2011, so maybe things changed.
>
> I'm happy to change the test to have .eh_frame read-only for all
> compilers, if that seems more correct.

it's not: cf. EH_TABLES_CAN_BE_READ_ONLY in gcc/config/i386/sol2.h and
DWARF2_EH_FRAME_READ_ONLY in gas/config/te-solaris.h and
gas/config/tc-hppa.h.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Fix gdb.dwarf2/clztest.exp with Clang

[Tom Tromey]

>> Shouldn't .eh_frame always be read-only?

Gary> I don't know.

For a test in particular I think the question is whether the change can
somehow negatively affect the test itself; and maybe secondarily whether
some plausible and/or planned future change would break the test.

If not then it seems fine to move forward.

Generally I think we'd be better off eliminating these assembly tests in
favor of something like the test suite's DWARF assembler, though I
didn't look to see whether this one would qualify.

Tom

[PATCH v2] Fix gdb.dwarf2/clztest.exp with Clang

[Gary Benson via Gdb-patches]

Hi Tom, Andreas,

Tom Tromey wrote:
> >> Shouldn't .eh_frame always be read-only?
> 
> Gary> I don't know.
> 
> For a test in particular I think the question is whether the change
> can somehow negatively affect the test itself; and maybe secondarily
> whether some plausible and/or planned future change would break the
> test.
> 
> If not then it seems fine to move forward.

GCC doesn't complain about making that section read-only, so I've
updated the test to make the section read-only always.

> Generally I think we'd be better off eliminating these assembly
> tests in favor of something like the test suite's DWARF assembler,
> though I didn't look to see whether this one would qualify.

Sure, but I'm not volunteering to do this one today! ;)

I've inlined an updated patch below.  As before I checked it on
Fedora 32 x86_64, with GCC and Clang.  Is it ok for me to commit?

Thanks,
Gary

---
Clang fails to compile gdb.dwarf2/clztest.S, with the following error:

  gdb compile failed, /gdbtest/src/gdb/testsuite/gdb.dwarf2/clztest.S:181:2:
    error: changed section flags for .eh_frame, expected: 0x2

This commit fixes the testcase by defining .eh_frame's flags
as Clang expects, as "a" rather than as "aw", thus making the
section read-only.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/clztest.S (.eh_frame): Make read-only.
---
 gdb/testsuite/ChangeLog            | 4 ++++
 gdb/testsuite/gdb.dwarf2/clztest.S | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.dwarf2/clztest.S b/gdb/testsuite/gdb.dwarf2/clztest.S
index a904fee..5e6cdae 100644
--- a/gdb/testsuite/gdb.dwarf2/clztest.S
+++ b/gdb/testsuite/gdb.dwarf2/clztest.S
@@ -178,7 +178,7 @@ _start:
 .LEFDE4:
 #NO_APP
 #APP
-	.section	.eh_frame,"aw",@progbits
+	.section	.eh_frame,"a",@progbits
 .Lframe1:
 	.long	.LECIE1-.LSCIE1	# Length of Common Information Entry
 .LSCIE1:
-- 
1.8.3.1

[PING][PATCH v2] Fix gdb.dwarf2/clztest.exp with Clang

[Gary Benson via Gdb-patches]

Ping.

gdb-patches wrote:
> Hi Tom, Andreas,
> 
> Tom Tromey wrote:
> > >> Shouldn't .eh_frame always be read-only?
> > 
> > Gary> I don't know.
> > 
> > For a test in particular I think the question is whether the change
> > can somehow negatively affect the test itself; and maybe secondarily
> > whether some plausible and/or planned future change would break the
> > test.
> > 
> > If not then it seems fine to move forward.
> 
> GCC doesn't complain about making that section read-only, so I've
> updated the test to make the section read-only always.
> 
> > Generally I think we'd be better off eliminating these assembly
> > tests in favor of something like the test suite's DWARF assembler,
> > though I didn't look to see whether this one would qualify.
> 
> Sure, but I'm not volunteering to do this one today! ;)
> 
> I've inlined an updated patch below.  As before I checked it on
> Fedora 32 x86_64, with GCC and Clang.  Is it ok for me to commit?
> 
> Thanks,
> Gary
> 
> ---
> Clang fails to compile gdb.dwarf2/clztest.S, with the following error:
> 
>   gdb compile failed, /gdbtest/src/gdb/testsuite/gdb.dwarf2/clztest.S:181:2:
>     error: changed section flags for .eh_frame, expected: 0x2
> 
> This commit fixes the testcase by defining .eh_frame's flags
> as Clang expects, as "a" rather than as "aw", thus making the
> section read-only.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.dwarf2/clztest.S (.eh_frame): Make read-only.
> ---
>  gdb/testsuite/ChangeLog            | 4 ++++
>  gdb/testsuite/gdb.dwarf2/clztest.S | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.dwarf2/clztest.S b/gdb/testsuite/gdb.dwarf2/clztest.S
> index a904fee..5e6cdae 100644
> --- a/gdb/testsuite/gdb.dwarf2/clztest.S
> +++ b/gdb/testsuite/gdb.dwarf2/clztest.S
> @@ -178,7 +178,7 @@ _start:
>  .LEFDE4:
>  #NO_APP
>  #APP
> -	.section	.eh_frame,"aw",@progbits
> +	.section	.eh_frame,"a",@progbits
>  .Lframe1:
>  	.long	.LECIE1-.LSCIE1	# Length of Common Information Entry
>  .LSCIE1:
> -- 
> 1.8.3.1
> 

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