public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix gdb.base/step-indirect-call-thunk.exp
@ 2022-06-15 16:21 Carl Love
  2022-06-30 15:12 ` [Ping] " Carl Love
  2022-07-01 12:37 ` Bruno Larsen
  0 siblings, 2 replies; 6+ messages in thread
From: Carl Love @ 2022-06-15 16:21 UTC (permalink / raw)
  To: gdb-patches, will schmidt, cel, Ulrich Weigand

GDB maintainers:

The gdb regression test gdb.base/step-indirect-call-thunk.exp currently
does not run on X86 due to a compile error related to incompatible gcc
command line argument.  Secondly, the gcc command line arguments that
are used are specific to Intel thus generating an unsupported command
line error when compiled on other architectures.

This patch fixes the command line arguments so the test will compile on
X86.  It also adds a check so the test will only run on X86.

Please let me know if this patch is acceptable for mainline.

                           Carl Love


--------------------------------------------------------------
Fix gdb.base/step-indirect-call-thunk.exp

This test fails on Intel X86-64 with the error:

Executing on host: gcc  -fno-stack-protector  -fdiagnostics-color=never
-mindirect-branch=thunk -mfunction-return=thunk -c -g
-o /.../gdb/testsuite/outputs/gdb.base/step-indirect-call-thunk/step-indirect-call-thunk0.o
/.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c
(timeout = 300) builtin_spawn -ignore SIGHUP gcc -fno-stack-protector
-fdiagnostics-color=never -mindirect-branch=thunk -mfunction-return=thunk -c
-g -o /.../gdb/testsuite/outputs/gdb.base/step-indirect-call-thunk/step-indirect-call-thunk0.o
/.../binutils-gdb-current/gdb/testsuite/gdb.base/step-indirect-call-thunk.c
/.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c:
 In function 'inc': /.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c:
22:1: error: '-mindirect-branch' and '-fcf-protection' are not compatible
   22 | {                /* inc.1 */

As stated in the error message the default "-fcf-protection" and
"-mindirect-branch' are in compatible.  The fcf-protection argument needs to be
"-fcf-protection=none" for the test to compile on Intel.

The test also fails on PowerPC as the "-mindirect-branch' is an Intel specific
GCC command line argument.  A check for X86 is added so the test will only run
on X86 platforms.

The patch has been tested and verified on Power 10 and Intel X86-64 systems with
no regressions.
---
 gdb/testsuite/gdb.base/step-indirect-call-thunk.exp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp b/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
index 761e1d9a280..7c1b53c99be 100644
--- a/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
+++ b/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
@@ -15,7 +15,11 @@
 
 standard_testfile
 
-set cflags "-mindirect-branch=thunk -mfunction-return=thunk"
+if { ![istarget "x86*"] } {
+    return
+}
+
+set cflags "-mindirect-branch=thunk -mfunction-return=thunk -fcf-protection=none"
 if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
         [list debug "additional_flags=$cflags"]] } {
     return -1
-- 
2.31.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Ping]  [PATCH] Fix gdb.base/step-indirect-call-thunk.exp
  2022-06-15 16:21 [PATCH] Fix gdb.base/step-indirect-call-thunk.exp Carl Love
@ 2022-06-30 15:12 ` Carl Love
  2022-07-01 12:37 ` Bruno Larsen
  1 sibling, 0 replies; 6+ messages in thread
From: Carl Love @ 2022-06-30 15:12 UTC (permalink / raw)
  To: gdb-patches, will schmidt, Ulrich Weigand

Ping?


On Wed, 2022-06-15 at 09:21 -0700, Carl Love wrote:
> GDB maintainers:
> 
> The gdb regression test gdb.base/step-indirect-call-thunk.exp
> currently
> does not run on X86 due to a compile error related to incompatible
> gcc
> command line argument.  Secondly, the gcc command line arguments that
> are used are specific to Intel thus generating an unsupported command
> line error when compiled on other architectures.
> 
> This patch fixes the command line arguments so the test will compile
> on
> X86.  It also adds a check so the test will only run on X86.
> 
> Please let me know if this patch is acceptable for mainline.
> 
>                            Carl Love
> 
> 
> --------------------------------------------------------------
> Fix gdb.base/step-indirect-call-thunk.exp
> 
> This test fails on Intel X86-64 with the error:
> 
> Executing on host: gcc  -fno-stack-protector  -fdiagnostics-
> color=never
> -mindirect-branch=thunk -mfunction-return=thunk -c -g
> -o /.../gdb/testsuite/outputs/gdb.base/step-indirect-call-thunk/step-
> indirect-call-thunk0.o
> /.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c
> (timeout = 300) builtin_spawn -ignore SIGHUP gcc -fno-stack-protector
> -fdiagnostics-color=never -mindirect-branch=thunk -mfunction-
> return=thunk -c
> -g -o /.../gdb/testsuite/outputs/gdb.base/step-indirect-call-
> thunk/step-indirect-call-thunk0.o
> /.../binutils-gdb-current/gdb/testsuite/gdb.base/step-indirect-call-
> thunk.c
> /.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c:
>  In function 'inc': /.../gdb/testsuite/gdb.base/step-indirect-call-
> thunk.c:
> 22:1: error: '-mindirect-branch' and '-fcf-protection' are not
> compatible
>    22 | {                /* inc.1 */
> 
> As stated in the error message the default "-fcf-protection" and
> "-mindirect-branch' are in compatible.  The fcf-protection argument
> needs to be
> "-fcf-protection=none" for the test to compile on Intel.
> 
> The test also fails on PowerPC as the "-mindirect-branch' is an Intel
> specific
> GCC command line argument.  A check for X86 is added so the test will
> only run
> on X86 platforms.
> 
> The patch has been tested and verified on Power 10 and Intel X86-64
> systems with
> no regressions.
> ---
>  gdb/testsuite/gdb.base/step-indirect-call-thunk.exp | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> b/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> index 761e1d9a280..7c1b53c99be 100644
> --- a/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> +++ b/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> @@ -15,7 +15,11 @@
> 
>  standard_testfile
> 
> -set cflags "-mindirect-branch=thunk -mfunction-return=thunk"
> +if { ![istarget "x86*"] } {
> +    return
> +}
> +
> +set cflags "-mindirect-branch=thunk -mfunction-return=thunk -fcf-
> protection=none"
>  if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
>          [list debug "additional_flags=$cflags"]] } {
>      return -1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix gdb.base/step-indirect-call-thunk.exp
  2022-06-15 16:21 [PATCH] Fix gdb.base/step-indirect-call-thunk.exp Carl Love
  2022-06-30 15:12 ` [Ping] " Carl Love
@ 2022-07-01 12:37 ` Bruno Larsen
  2022-07-06 15:57   ` Carl Love
  2022-07-06 15:59   ` [PATCH version 2] " Carl Love
  1 sibling, 2 replies; 6+ messages in thread
From: Bruno Larsen @ 2022-07-01 12:37 UTC (permalink / raw)
  To: Carl Love, gdb-patches, will schmidt, Ulrich Weigand


On 6/15/22 13:21, Carl Love via Gdb-patches wrote:
> GDB maintainers:
> 
> The gdb regression test gdb.base/step-indirect-call-thunk.exp currently
> does not run on X86 due to a compile error related to incompatible gcc
> command line argument.  Secondly, the gcc command line arguments that
> are used are specific to Intel thus generating an unsupported command
> line error when compiled on other architectures.
> 
> This patch fixes the command line arguments so the test will compile on
> X86.  It also adds a check so the test will only run on X86.
> 
> Please let me know if this patch is acceptable for mainline.
> 
>                             Carl Love
> 


Hi Carl!

Thanks for looking at this. The code part of the patch looks good, but I'd suggest a bit
of a change to the commit message.


> 
> --------------------------------------------------------------
> Fix gdb.base/step-indirect-call-thunk.exp
> 
> This test fails on Intel X86-64 with the error:
> 
> Executing on host: gcc  -fno-stack-protector  -fdiagnostics-color=never
> -mindirect-branch=thunk -mfunction-return=thunk -c -g
> -o /.../gdb/testsuite/outputs/gdb.base/step-indirect-call-thunk/step-indirect-call-thunk0.o
> /.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c
> (timeout = 300) builtin_spawn -ignore SIGHUP gcc -fno-stack-protector
> -fdiagnostics-color=never -mindirect-branch=thunk -mfunction-return=thunk -c
> -g -o /.../gdb/testsuite/outputs/gdb.base/step-indirect-call-thunk/step-indirect-call-thunk0.o
> /.../binutils-gdb-current/gdb/testsuite/gdb.base/step-indirect-call-thunk.c
> /.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c:
>   In function 'inc': /.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c:
> 22:1: error: '-mindirect-branch' and '-fcf-protection' are not compatible
>     22 | {                /* inc.1 */
> 
> As stated in the error message the default "-fcf-protection" and
> "-mindirect-branch' are in compatible.  The fcf-protection argument needs to be
> "-fcf-protection=none" for the test to compile on Intel.

This problem doesn't happen on my machine, which is using gcc-8.5.0; I think the default
-fcf-protection value was changed somewhere between gcc-8.5.0 and the one you're using.
I'd mention something along these lines:

	"Due to changes in the default value of -fcf-protection on newer gccs, the test ... can fail."

Also, remember that the commit message should assume the title of the commit was not read,
so please use the full test name instead of "This test".


> 
> The test also fails on PowerPC as the "-mindirect-branch' is an Intel specific
> GCC command line argument.  A check for X86 is added so the test will only run
> on X86 platforms.

I'd remove the specific mention to PowerPC and just say that -mindirect-branch is x86
specific.

With these, I'd give an OK to this patch, but I can't approve it for pushing.

Cheers!
Bruno Larsen
> 
> The patch has been tested and verified on Power 10 and Intel X86-64 systems with
> no regressions.

> ---
>   gdb/testsuite/gdb.base/step-indirect-call-thunk.exp | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp b/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> index 761e1d9a280..7c1b53c99be 100644
> --- a/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> +++ b/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> @@ -15,7 +15,11 @@
>   
>   standard_testfile
>   
> -set cflags "-mindirect-branch=thunk -mfunction-return=thunk"
> +if { ![istarget "x86*"] } {
> +    return
> +}
> +
> +set cflags "-mindirect-branch=thunk -mfunction-return=thunk -fcf-protection=none"
>   if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
>           [list debug "additional_flags=$cflags"]] } {
>       return -1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix gdb.base/step-indirect-call-thunk.exp
  2022-07-01 12:37 ` Bruno Larsen
@ 2022-07-06 15:57   ` Carl Love
  2022-07-06 15:59   ` [PATCH version 2] " Carl Love
  1 sibling, 0 replies; 6+ messages in thread
From: Carl Love @ 2022-07-06 15:57 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches, will schmidt, Ulrich Weigand

Bruno:

Thanks for the feedback.  I updated the commit message and will send
out version 2 of the patch.

                     Carl Love

On Fri, 2022-07-01 at 09:37 -0300, Bruno Larsen wrote:
> On 6/15/22 13:21, Carl Love via Gdb-patches wrote:
> > GDB maintainers:
> > 
> > The gdb regression test gdb.base/step-indirect-call-thunk.exp
> > currently
> > does not run on X86 due to a compile error related to incompatible
> > gcc
> > command line argument.  Secondly, the gcc command line arguments
> > that
> > are used are specific to Intel thus generating an unsupported
> > command
> > line error when compiled on other architectures.
> > 
> > This patch fixes the command line arguments so the test will
> > compile on
> > X86.  It also adds a check so the test will only run on X86.
> > 
> > Please let me know if this patch is acceptable for mainline.
> > 
> >                             Carl Love
> > 
> 
> Hi Carl!
> 
> Thanks for looking at this. The code part of the patch looks good,
> but I'd suggest a bit
> of a change to the commit message.
> 
> 
> > --------------------------------------------------------------
> > Fix gdb.base/step-indirect-call-thunk.exp
> > 
> > This test fails on Intel X86-64 with the error:
> > 
> > Executing on host: gcc  -fno-stack-protector  -fdiagnostics-
> > color=never
> > -mindirect-branch=thunk -mfunction-return=thunk -c -g
> > -o /.../gdb/testsuite/outputs/gdb.base/step-indirect-call-
> > thunk/step-indirect-call-thunk0.o
> > /.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c
> > (timeout = 300) builtin_spawn -ignore SIGHUP gcc -fno-stack-
> > protector
> > -fdiagnostics-color=never -mindirect-branch=thunk -mfunction-
> > return=thunk -c
> > -g -o /.../gdb/testsuite/outputs/gdb.base/step-indirect-call-
> > thunk/step-indirect-call-thunk0.o
> > /.../binutils-gdb-current/gdb/testsuite/gdb.base/step-indirect-
> > call-thunk.c
> > /.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c:
> >   In function 'inc': /.../gdb/testsuite/gdb.base/step-indirect-
> > call-thunk.c:
> > 22:1: error: '-mindirect-branch' and '-fcf-protection' are not
> > compatible
> >     22 | {                /* inc.1 */
> > 
> > As stated in the error message the default "-fcf-protection" and
> > "-mindirect-branch' are in compatible.  The fcf-protection argument
> > needs to be
> > "-fcf-protection=none" for the test to compile on Intel.
> 
> This problem doesn't happen on my machine, which is using gcc-8.5.0;
> I think the default
> -fcf-protection value was changed somewhere between gcc-8.5.0 and the
> one you're using.
> I'd mention something along these lines:
> 
> 	"Due to changes in the default value of -fcf-protection on
> newer gccs, the test ... can fail."
> 
> Also, remember that the commit message should assume the title of the
> commit was not read,
> so please use the full test name instead of "This test".
> 
> 
> > The test also fails on PowerPC as the "-mindirect-branch' is an
> > Intel specific
> > GCC command line argument.  A check for X86 is added so the test
> > will only run
> > on X86 platforms.
> 
> I'd remove the specific mention to PowerPC and just say that
> -mindirect-branch is x86
> specific.
> 
> With these, I'd give an OK to this patch, but I can't approve it for
> pushing.
> 
> Cheers!
> Bruno Larsen
> > The patch has been tested and verified on Power 10 and Intel X86-64 
> > systems with
> > no regressions.
> > ---
> >   gdb/testsuite/gdb.base/step-indirect-call-thunk.exp | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> > b/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> > index 761e1d9a280..7c1b53c99be 100644
> > --- a/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> > +++ b/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
> > @@ -15,7 +15,11 @@
> >   
> >   standard_testfile
> >   
> > -set cflags "-mindirect-branch=thunk -mfunction-return=thunk"
> > +if { ![istarget "x86*"] } {
> > +    return
> > +}
> > +
> > +set cflags "-mindirect-branch=thunk -mfunction-return=thunk -fcf-
> > protection=none"
> >   if { [prepare_for_testing "failed to prepare" $testfile $srcfile
> > \
> >           [list debug "additional_flags=$cflags"]] } {
> >       return -1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH version 2] Fix gdb.base/step-indirect-call-thunk.exp
  2022-07-01 12:37 ` Bruno Larsen
  2022-07-06 15:57   ` Carl Love
@ 2022-07-06 15:59   ` Carl Love
  2022-07-12 10:43     ` Ulrich Weigand
  1 sibling, 1 reply; 6+ messages in thread
From: Carl Love @ 2022-07-06 15:59 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches, will schmidt, Ulrich Weigand


GDB maintainers:

version 2:  Updated the commit message per comments from Bruno Larsen.

The gdb regression test gdb.base/step-indirect-call-thunk.exp currently
does not run on X86 due to a compile error related to incompatible gcc
command line argument.  Secondly, the gcc command line arguments that
are used are specific to Intel thus generating an unsupported command
line error when compiled on other architectures.

This patch fixes the command line arguments so the test will compile on
X86.  It also adds a check so the test will only run on X86.

Please let me know if this patch is acceptable for mainline.

                           Carl Love


-------------------------------------
Fix gdb.base/step-indirect-call-thunk.exp

Due to recent changes in the default value of -fcf-protection for gcc, the
test gdb.base/step-indirect-call-thunk.exp fails on Intel X86-64 with the error:

Executing on host: gcc  -fno-stack-protector  -fdiagnostics-color=never
-mindirect-branch=thunk -mfunction-return=thunk -c -g
-o /.../gdb/testsuite/outputs/gdb.base/step-indirect-call-thunk/step-indirect-call-thunk0.o
/.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c
(timeout = 300) builtin_spawn -ignore SIGHUP gcc -fno-stack-protector
-fdiagnostics-color=never -mindirect-branch=thunk -mfunction-return=thunk -c
-g -o /.../gdb/testsuite/outputs/gdb.base/step-indirect-call-thunk/step-indirect-call-thunk0.o
/.../binutils-gdb-current/gdb/testsuite/gdb.base/step-indirect-call-thunk.c
/.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c:
 In function 'inc': /.../gdb/testsuite/gdb.base/step-indirect-call-thunk.c:
22:1: error: '-mindirect-branch' and '-fcf-protection' are not compatible
   22 | {                /* inc.1 */

As stated in the error message the default "-fcf-protection" and
"-mindirect-branch' are in compatible.  The fcf-protection argument needs to be
"-fcf-protection=none" for the test to compile on Intel.

The gcc command line "-mindirect-branch' is an Intel specific and will give an
error on other platforms.  A check for X86 is added so the test will only run
on X86 platforms.

The patch has been tested and verified on Power 10 and Intel X86-64 systems with
no regressions.
---
 gdb/testsuite/gdb.base/step-indirect-call-thunk.exp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp b/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
index 761e1d9a280..7c1b53c99be 100644
--- a/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
+++ b/gdb/testsuite/gdb.base/step-indirect-call-thunk.exp
@@ -15,7 +15,11 @@
 
 standard_testfile
 
-set cflags "-mindirect-branch=thunk -mfunction-return=thunk"
+if { ![istarget "x86*"] } {
+    return
+}
+
+set cflags "-mindirect-branch=thunk -mfunction-return=thunk -fcf-protection=none"
 if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
         [list debug "additional_flags=$cflags"]] } {
     return -1
-- 
2.31.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH version 2] Fix gdb.base/step-indirect-call-thunk.exp
  2022-07-06 15:59   ` [PATCH version 2] " Carl Love
@ 2022-07-12 10:43     ` Ulrich Weigand
  0 siblings, 0 replies; 6+ messages in thread
From: Ulrich Weigand @ 2022-07-12 10:43 UTC (permalink / raw)
  To: gdb-patches, will_schmidt, blarsen, cel

Carl Love <cel@us.ibm.com> wrote:

>Fix gdb.base/step-indirect-call-thunk.exp

This is OK.

Thanks,
Ulrich


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-07-12 10:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-15 16:21 [PATCH] Fix gdb.base/step-indirect-call-thunk.exp Carl Love
2022-06-30 15:12 ` [Ping] " Carl Love
2022-07-01 12:37 ` Bruno Larsen
2022-07-06 15:57   ` Carl Love
2022-07-06 15:59   ` [PATCH version 2] " Carl Love
2022-07-12 10:43     ` Ulrich Weigand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).