[PATCH][gdb/testsuite] Don't require gold for gdb.base/morestack.exp

[PATCH][gdb/testsuite] Don't require gold for gdb.base/morestack.exp

[Tom de Vries]

Hi,

While working on PR26935 I noticed that the test-case requires the gold
linker, but doesn't really need it.

The -fuse-ld=gold was added to support the printf in the test-case, which
prints some information but is not otherwise needed for the test-case.

Fix this by commenting out the printf and removing the corresponding
-fuse-ld=gold.

Tested on x86_64.

Also checked that the test still fails when the fix from the commit that added
the test-case is reverted.

Any comments?

Thanks,
- Tom

[gdb/testsuite] Don't require gold for gdb.base/morestack.exp

gdb/testsuite/ChangeLog:

2021-01-04  Tom de Vries  <tdevries@suse.de>

	* gdb.base/morestack.c: Comment out printf.
	* gdb.base/morestack.exp: Don't use -fuse-ld=gold.

---
 gdb/testsuite/gdb.base/morestack.c   | 4 ++++
 gdb/testsuite/gdb.base/morestack.exp | 8 +-------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.base/morestack.c b/gdb/testsuite/gdb.base/morestack.c
index 6f9a27ed2c8..792a38a0cfe 100644
--- a/gdb/testsuite/gdb.base/morestack.c
+++ b/gdb/testsuite/gdb.base/morestack.c
@@ -63,7 +63,11 @@ down (int i)
 
   if (last && last < (void *) buf)
     {
+#if 0
+      /* Debug statement, requires gold linker.  See GCC documentation of
+	 -fsplit-stack.  */
       printf ("%d: %p < %p\n", i, last, buf);
+#endif
       marker_hit ();
     }
   last = buf;
diff --git a/gdb/testsuite/gdb.base/morestack.exp b/gdb/testsuite/gdb.base/morestack.exp
index 627ae8164c3..99807a291ee 100644
--- a/gdb/testsuite/gdb.base/morestack.exp
+++ b/gdb/testsuite/gdb.base/morestack.exp
@@ -21,17 +21,11 @@ if {$gcc_compiled == 0} {
     return -1
 }
 
-if { [have_fuse_ld_gold] == 0} {
-    return -1
-}
-
 standard_testfile
 
-# -fuse-ld=gold is used for calling printf code built without -fsplit-stack
-# which could crash otherwise.  See GCC documentation of -fsplit-stack.
 set opts "additional_flags=-fsplit-stack"
 if { [prepare_for_testing "failed to prepare" ${testfile} $srcfile \
-	  [list $opts additional_flags=-fuse-ld=gold]] } {
+	  [list $opts]] } {
     return -1
 }
 

Re: [PATCH][gdb/testsuite] Don't require gold for gdb.base/morestack.exp

[Andrew Burgess]

* Tom de Vries <tdevries@suse.de> [2021-01-04 13:54:55 +0100]:

> Hi,
> 
> While working on PR26935 I noticed that the test-case requires the gold
> linker, but doesn't really need it.
> 
> The -fuse-ld=gold was added to support the printf in the test-case, which
> prints some information but is not otherwise needed for the test-case.
> 
> Fix this by commenting out the printf and removing the corresponding
> -fuse-ld=gold.
> 
> Tested on x86_64.
> 
> Also checked that the test still fails when the fix from the commit that added
> the test-case is reverted.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/testsuite] Don't require gold for gdb.base/morestack.exp
> 
> gdb/testsuite/ChangeLog:
> 
> 2021-01-04  Tom de Vries  <tdevries@suse.de>
> 
> 	* gdb.base/morestack.c: Comment out printf.
> 	* gdb.base/morestack.exp: Don't use -fuse-ld=gold.
> 
> ---
>  gdb/testsuite/gdb.base/morestack.c   | 4 ++++
>  gdb/testsuite/gdb.base/morestack.exp | 8 +-------
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/morestack.c b/gdb/testsuite/gdb.base/morestack.c
> index 6f9a27ed2c8..792a38a0cfe 100644
> --- a/gdb/testsuite/gdb.base/morestack.c
> +++ b/gdb/testsuite/gdb.base/morestack.c
> @@ -63,7 +63,11 @@ down (int i)
>  
>    if (last && last < (void *) buf)
>      {
> +#if 0
> +      /* Debug statement, requires gold linker.  See GCC documentation of
> +	 -fsplit-stack.  */
>        printf ("%d: %p < %p\n", i, last, buf);
> +#endif

I'm not a fan of '#if 0' guards.  There are a few cases when it can
add value - the code within is effectively an extended comment, but
with "real" code highlighting/indentation.

This doesn't feel like one of these cases, so I'd vote to just remove
the code completely.  It's in the history if anyone ever cares to look
for it.

Thanks,
Andrew



>        marker_hit ();
>      }
>    last = buf;
> diff --git a/gdb/testsuite/gdb.base/morestack.exp b/gdb/testsuite/gdb.base/morestack.exp
> index 627ae8164c3..99807a291ee 100644
> --- a/gdb/testsuite/gdb.base/morestack.exp
> +++ b/gdb/testsuite/gdb.base/morestack.exp
> @@ -21,17 +21,11 @@ if {$gcc_compiled == 0} {
>      return -1
>  }
>  
> -if { [have_fuse_ld_gold] == 0} {
> -    return -1
> -}
> -
>  standard_testfile
>  
> -# -fuse-ld=gold is used for calling printf code built without -fsplit-stack
> -# which could crash otherwise.  See GCC documentation of -fsplit-stack.
>  set opts "additional_flags=-fsplit-stack"
>  if { [prepare_for_testing "failed to prepare" ${testfile} $srcfile \
> -	  [list $opts additional_flags=-fuse-ld=gold]] } {
> +	  [list $opts]] } {
>      return -1
>  }
>  

Re: [PATCH][gdb/testsuite] Don't require gold for gdb.base/morestack.exp

[Tom de Vries]

On 1/4/21 4:03 PM, Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2021-01-04 13:54:55 +0100]:
> 
>> Hi,
>>
>> While working on PR26935 I noticed that the test-case requires the gold
>> linker, but doesn't really need it.
>>
>> The -fuse-ld=gold was added to support the printf in the test-case, which
>> prints some information but is not otherwise needed for the test-case.
>>
>> Fix this by commenting out the printf and removing the corresponding
>> -fuse-ld=gold.
>>
>> Tested on x86_64.
>>
>> Also checked that the test still fails when the fix from the commit that added
>> the test-case is reverted.
>>
>> Any comments?
>>
>> Thanks,
>> - Tom
>>
>> [gdb/testsuite] Don't require gold for gdb.base/morestack.exp
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2021-01-04  Tom de Vries  <tdevries@suse.de>
>>
>> 	* gdb.base/morestack.c: Comment out printf.
>> 	* gdb.base/morestack.exp: Don't use -fuse-ld=gold.
>>
>> ---
>>  gdb/testsuite/gdb.base/morestack.c   | 4 ++++
>>  gdb/testsuite/gdb.base/morestack.exp | 8 +-------
>>  2 files changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/morestack.c b/gdb/testsuite/gdb.base/morestack.c
>> index 6f9a27ed2c8..792a38a0cfe 100644
>> --- a/gdb/testsuite/gdb.base/morestack.c
>> +++ b/gdb/testsuite/gdb.base/morestack.c
>> @@ -63,7 +63,11 @@ down (int i)
>>  
>>    if (last && last < (void *) buf)
>>      {
>> +#if 0
>> +      /* Debug statement, requires gold linker.  See GCC documentation of
>> +	 -fsplit-stack.  */
>>        printf ("%d: %p < %p\n", i, last, buf);
>> +#endif
> 
> I'm not a fan of '#if 0' guards.  There are a few cases when it can
> add value - the code within is effectively an extended comment, but
> with "real" code highlighting/indentation.
> 
> This doesn't feel like one of these cases, so I'd vote to just remove
> the code completely.  It's in the history if anyone ever cares to look
> for it.
> 

Hi Andrew,

thanks for the review.

Committed with the printf removed.

Thanks,
- Tom

> Thanks,
> Andrew
> 
> 
> 
>>        marker_hit ();
>>      }
>>    last = buf;
>> diff --git a/gdb/testsuite/gdb.base/morestack.exp b/gdb/testsuite/gdb.base/morestack.exp
>> index 627ae8164c3..99807a291ee 100644
>> --- a/gdb/testsuite/gdb.base/morestack.exp
>> +++ b/gdb/testsuite/gdb.base/morestack.exp
>> @@ -21,17 +21,11 @@ if {$gcc_compiled == 0} {
>>      return -1
>>  }
>>  
>> -if { [have_fuse_ld_gold] == 0} {
>> -    return -1
>> -}
>> -
>>  standard_testfile
>>  
>> -# -fuse-ld=gold is used for calling printf code built without -fsplit-stack
>> -# which could crash otherwise.  See GCC documentation of -fsplit-stack.
>>  set opts "additional_flags=-fsplit-stack"
>>  if { [prepare_for_testing "failed to prepare" ${testfile} $srcfile \
>> -	  [list $opts additional_flags=-fuse-ld=gold]] } {
>> +	  [list $opts]] } {
>>      return -1
>>  }
>>