diff mbox

testsuite: fix is_amd64_regs_target

Message ID 1538034149-12136-2-git-send-email-markus.t.metzger@intel.com
State New, archived
Headers

Commit Message

Metzger, Markus T Sept. 27, 2018, 7:42 a.m. UTC 
  Commit

    c221b2f Testsuite: Add gdb_can_simple_compile

changed the source file name extension of the test program from .s to .c
resulting in compile fails.  This, in turn, causes is_amd64_regs_target
checks to fail.  In gdb.btrace/tailcall.exp and others, this causes the
wrong source file to be picked and the test to fail on 64-bit targets.

Change the test source from an assembly program to a C program using
inline assembly.

There is a similar case for is_aarch32_target that I have not touched as I
would not be able to test my changes.

2018-09-27  Markus Metzger  <markus.t.metzger@intel.com>

testsuite/
	* lib/gdb.exp (is_amd64_regs_target): Change assembly to C inline
	assembly.
---
 gdb/testsuite/lib/gdb.exp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Tom Tromey Sept. 27, 2018, 9:09 p.m. UTC | #1 
>>>>> "Markus" == Markus Metzger <markus.t.metzger@intel.com> writes:

Markus> testsuite/
Markus> 	* lib/gdb.exp (is_amd64_regs_target): Change assembly to C inline
Markus> 	assembly.

Markus> -    set list {}
Markus> -    foreach reg \
Markus> -	{rax rbx rcx rdx rsi rdi rbp rsp r8 r9 r10 r11 r12 r13 r14 r15} {
Markus> -	    lappend list "\tincq %$reg"
Markus> -	}
Markus> +    return [gdb_can_simple_compile is_amd64_regs_target {
Markus> +	int main (void) {
Markus> +	    asm ("incq %rax");
Markus> +	    asm ("incq %r15");

I suppose reducing the number of registers tested doesn't materially
affect the test.

Assuming that's the case, this is ok.  Thank you.

Tom
Metzger, Markus T Sept. 28, 2018, 7:06 a.m. UTC | #2 
Hello Tom,

Thanks for your review.

> Markus> -    set list {}
> Markus> -    foreach reg \
> Markus> -	{rax rbx rcx rdx rsi rdi rbp rsp r8 r9 r10 r11 r12 r13 r14 r15} {
> Markus> -	    lappend list "\tincq %$reg"
> Markus> -	}
> Markus> +    return [gdb_can_simple_compile is_amd64_regs_target {
> Markus> +	int main (void) {
> Markus> +	    asm ("incq %rax");
> Markus> +	    asm ("incq %r15");
> 
> I suppose reducing the number of registers tested doesn't materially affect the
> test.

That's my assumption, as well.  We already checked the architecture so we know it
is some x86 flavor.  A single 64-bit register should suffice.

The gdb.btrace suite passes on 32-bit and 64-bit including m32.  I have not run the
full suite since the problem was already seen on the gdb.btrace suite.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Tom Tromey Sept. 28, 2018, 5:20 p.m. UTC | #3 
>>>>> "Markus" == Metzger, Markus T <markus.t.metzger@intel.com> writes:

Markus> That's my assumption, as well.  We already checked the architecture so we know it
Markus> is some x86 flavor.  A single 64-bit register should suffice.

I didn't see it go in, so I wanted to reiterate that I think it is ok.

Tom
Simon Marchi Sept. 28, 2018, 8:27 p.m. UTC | #4 
On 2018-09-28 01:20 PM, Tom Tromey wrote:
>>>>>> "Markus" == Metzger, Markus T <markus.t.metzger@intel.com> writes:

> 

> Markus> That's my assumption, as well.  We already checked the architecture so we know it

> Markus> is some x86 flavor.  A single 64-bit register should suffice.

> 

> I didn't see it go in, so I wanted to reiterate that I think it is ok.

> 

> Tom

> 


Maybe I'm missing something obvious, but could one of you explain to me
the comment of that proc, which is

# Return 1 if target has x86_64 registers - either amd64 or x32.
# x32 target identifies as x86_64-*-linux*, therefore it cannot be determined
# just from the target string.

If I understand correctly, is_amd64_regs_target should return 1 when using the
x32 abi, as well as when using the standard amd64 abi.  The comment says an x32
target has an x86_64-* triplet... so can't we just check the triplet, and return
1 if it's x86-64-*?

Simon
Pedro Alves Sept. 29, 2018, 6:09 p.m. UTC | #5 
On 09/28/2018 09:27 PM, Simon Marchi wrote:

> Maybe I'm missing something obvious, but could one of you explain to me
> the comment of that proc, which is
> 
> # Return 1 if target has x86_64 registers - either amd64 or x32.
> # x32 target identifies as x86_64-*-linux*, therefore it cannot be determined
> # just from the target string.
> 
> If I understand correctly, is_amd64_regs_target should return 1 when using the
> x32 abi, as well as when using the standard amd64 abi.  The comment says an x32
> target has an x86_64-* triplet... so can't we just check the triplet, and return
> 1 if it's x86-64-*?

No, because toolchains configured with i686-* triplets can also be
used in combination with -mx32/-m64.

Same for is_64_target, is_lp64_target, etc.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index f32abfedd52..1eea92298c4 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2467,13 +2467,14 @@  gdb_caching_proc is_amd64_regs_target {
 	return 0
     }
 
-    set list {}
-    foreach reg \
-	{rax rbx rcx rdx rsi rdi rbp rsp r8 r9 r10 r11 r12 r13 r14 r15} {
-	    lappend list "\tincq %$reg"
-	}
+    return [gdb_can_simple_compile is_amd64_regs_target {
+	int main (void) {
+	    asm ("incq %rax");
+	    asm ("incq %r15");
 
-    return [gdb_can_simple_compile is_amd64_regs_target [join $list \n]]
+	    return 0;
+	}
+    }]
 }
 
 # Return 1 if this target is an x86 or x86-64 with -m32.