Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Tom de Vries <tdevries@suse.de>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH][gdb/testsuite] Don't require gold for gdb.base/morestack.exp
Date: Mon, 4 Jan 2021 15:03:53 +0000	[thread overview]
Message-ID: <20210104150353.GZ2945@embecosm.com> (raw)
In-Reply-To: <20210104125453.GA4963@delia>

* 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
>  }
>  

  reply	other threads:[~2021-01-04 15:03 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-04 12:54 Tom de Vries
2021-01-04 15:03 ` Andrew Burgess [this message]
2021-01-04 18:38   ` Tom de Vries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210104150353.GZ2945@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tdevries@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox