[PATCH] Fix gdb.base/step-indirect-call-thunk.exp
Carl Love
cel@us.ibm.com
Wed Jul 6 15:57:27 GMT 2022
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
More information about the Gdb-patches
mailing list