* [review] [gdb/tdep] Fix 'Unexpected register class' assert in amd64_push_argum...
[not found] <gerrit.1571043259000.I8b66345bbf5c00209ca75b1209fd4d60b36e9ede@gnutoolchain-gerrit.osci.io>
@ 2019-10-16 15:13 ` Sourceware to Gerrit sync (Code Review)
2019-10-16 15:13 ` Sourceware to Gerrit sync (Code Review)
1 sibling, 0 replies; 2+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-10-16 15:13 UTC (permalink / raw)
To: Tom de Vries, Andrew Burgess, Simon Marchi, gdb-patches
Sourceware to Gerrit sync has uploaded a new patch set version (#2) to the change originally created by Tom de Vries.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/30
......................................................................
[gdb/tdep] Fix 'Unexpected register class' assert in amd64_push_arguments
Atm, when executing gdb.base/infcall-nested-structs.exp on x86_64-linux, we get:
...
FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-tc-tf: \
p/d check_arg_struct_02_01 (ref_val_struct_02_01)
FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ts-tf: \
p/d check_arg_struct_02_01 (ref_val_struct_02_01)
FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ti-tf: \
p/d check_arg_struct_02_01 (ref_val_struct_02_01)
=== gdb Summary ===
nr of expected passes 9255
nr of unexpected failures 3
nr of expected failures 142
...
The 3 FAILs are reported as PR tdep/25096.
The 142 XFAILs are for a gdb assertion failure, reported in PR tdep/24104,
which should have been KFAILs since there's a problem in gdb rather than in
the environment.
A minimal version of the assertion failure looks like this. Consider test.c:
...
struct s { struct { } es1; long f; };
struct s ref = { {}, 'f' };
int __attribute__((noinline,noclone)) check (struct s arg)
{ return arg.f == 'f'; }
int main (void)
{ return check (ref); }
...
When calling 'check (ref)' from main, we have '1' as expected:
...
$ g++ test3.c -g && ( ./a.out; echo $? )
1
...
But when calling 'check (ref)' from the gdb prompt, we get:
...
$ gdb a.out -batch -ex start -ex "p check (ref)"
Temporary breakpoint 1 at 0x4004f7: file test.c, line 8.
Temporary breakpoint 1, main () at test.c:8
8 { return check (ref); }
src/gdb/amd64-tdep.c:982: internal-error: \
CORE_ADDR amd64_push_arguments(regcache*, int, value**, CORE_ADDR, \
function_call_return_method): \
Assertion `!"Unexpected register class."' failed.
...
The assert happens in this loop in amd64_push_arguments:
...
for (j = 0; len > 0; j++, len -= 8)
{
int regnum = -1;
int offset = 0;
switch (theclass[j])
{
case AMD64_INTEGER:
regnum = integer_regnum[integer_reg++];
break;
case AMD64_SSE:
regnum = sse_regnum[sse_reg++];
break;
case AMD64_SSEUP:
gdb_assert (sse_reg > 0);
regnum = sse_regnum[sse_reg - 1];
offset = 8;
break;
default:
gdb_assert (!"Unexpected register class.");
}
...
}
...
when processing theclass[0], which is AMD64_NO_CLASS:
...
(gdb) p theclass
$1 = {AMD64_NO_CLASS, AMD64_INTEGER}
...
The layout of struct s is that the empty field es1 occupies one byte (due to
c++) at offset 0, and the long field f occupies 8 bytes at offset 8.
When compiling at -O2, we can see from the disassembly of main:
...
4003f0: 48 8b 3d 41 0c 20 00 mov 0x200c41(%rip),%rdi \
# 601038 <ref+0x8>
4003f7: e9 e4 00 00 00 jmpq 4004e0 <_Z5check1s>
4003fc: 0f 1f 40 00 nopl 0x0(%rax)
...
that check is called with field f passed in %rdi, meaning that the
classification in theclass is correct, it's just not supported in the loop in
amd64_push_arguments mentioned above.
Fix the assert by implementing support for 'AMD64_NO_CLASS' in that loop.
This exposes 9 more FAILs of the PR tdep/25096 type, so mark all 12 of them as
KFAIL.
Tested on x86_64-linux.
Tested with g++ 4.8.5, 7.4.1, 8.3.1, 9.2.1. With 4.8.5, 3 of the 12 KFAILs
are KPASSing.
Tested with clang++ 5.0.2 (which requires removing additional_flags=-Wno-psabi
and adding additional_flags=-Wno-deprecated).
gdb/ChangeLog:
2019-10-16 Tom de Vries <tdevries@suse.de>
PR tdep/24104
* amd64-tdep.c (amd64_push_arguments): Handle AMD64_NO_CLASS in loop
that handles 'theclass'.
gdb/testsuite/ChangeLog:
2019-10-16 Tom de Vries <tdevries@suse.de>
PR tdep/24104
* gdb.base/infcall-nested-structs.exp: Remove XFAIL for PR tdep/24104.
Add KFAIL for PR tdep/25096.
Change-Id: I8b66345bbf5c00209ca75b1209fd4d60b36e9ede
---
M gdb/ChangeLog
M gdb/amd64-tdep.c
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.base/infcall-nested-structs.exp
4 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3827c4d..eeba0ee 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-10-16 Tom de Vries <tdevries@suse.de>
+
+ PR tdep/24104
+ * amd64-tdep.c (amd64_push_arguments): Handle AMD64_NO_CLASS in loop
+ that handles 'theclass'.
+
2019-10-15 Andrew Burgess <andrew.burgess@embecosm.com>
* linespec.c (decode_digits_ordinary): Update comment.
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 232d16d..9006ec0 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -978,6 +978,9 @@
offset = 8;
break;
+ case AMD64_NO_CLASS:
+ continue;
+
default:
gdb_assert (!"Unexpected register class.");
}
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 877d0de..3277ee3 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,11 @@
2019-10-16 Tom de Vries <tdevries@suse.de>
+ PR tdep/24104
+ * gdb.base/infcall-nested-structs.exp: Remove XFAIL for PR tdep/24104.
+ Add KFAIL for PR tdep/25096.
+
+2019-10-16 Tom de Vries <tdevries@suse.de>
+
PR testsuite/25059
* gdb.cp/local-static.exp (do_test): Add xfails for gcc PR debug/55541.
diff --git a/gdb/testsuite/gdb.base/infcall-nested-structs.exp b/gdb/testsuite/gdb.base/infcall-nested-structs.exp
index f5fbf44..957eb31 100644
--- a/gdb/testsuite/gdb.base/infcall-nested-structs.exp
+++ b/gdb/testsuite/gdb.base/infcall-nested-structs.exp
@@ -132,16 +132,9 @@
continue
}
- if { $lang == "c++"
- && ( ( [regexp "struct_01_0(1|2|3)" $name match] && [regexp "^types-(td($|-)|tl(|l)(|-tf|-td|-tld)$)" $types match] )
- || ( $name == "struct_01_02" && $types == "types-tfc" )
- || ( $name == "struct_01_04" && [regexp "^types-(tf($|-)|ti(|-tf|-td|-tld)$)" $types match] )
- || ( $name == "struct_02_01" && [regexp "^types-tf-t(c|s|i)" $types match] )
- || ( $name == "struct_static_02_02" && [regexp "^types-(t(f|d|ld)-t(d|l|ll)$|t(d|l|ll)$|t(c|s|i|l|ll)-td)" $types match] )
- || ( $name == "struct_static_02_03" && [regexp "^types-(ti-t(f|l|d|)|tf(-|$)|ti$)" $types match] )
- || ( $name == "struct_static_04_02" && [regexp "^types-(t(c|s)-tf|tf-ts)" $types match] )
- || ( $name == "struct_static_06_04" && ![regexp "^types-(t(c|dc|ldc|ld)$|t.-tld|tl(l|d)-tld|t(f|d|ld)-tc)" $types match] ) ) } {
- setup_xfail gdb/24104 "x86_64-*-linux*"
+ if { $lang == "c++" && $name == "struct_02_01"
+ && [regexp "^types-(tf-t(c|s|i)|t(c|s|i)-tf)" $types match] } {
+ setup_kfail gdb/25096 "x86_64-*-linux*"
}
gdb_test "p/d check_arg_${name} (ref_val_${name})" "= 1"
@@ -154,8 +147,9 @@
set answer [ get_valueof "" "rtn_str_${name} ()" "XXXX"]
verbose -log "Answer: ${answer}"
- if { ($lang == "c++" && $name == "struct_02_01" && [regexp "^types-(tf-t(c|s|i)|t(c|s|i)-tf)" $types match] ) } {
- setup_xfail gdb/24104 "x86_64-*-linux*"
+ if { $lang == "c++" && $name == "struct_02_01"
+ && [regexp "^types-(tf-t(c|s|i)|t(c|s|i)-tf)" $types match] } {
+ setup_kfail gdb/25096 "x86_64-*-linux*"
}
gdb_assert [string eq ${answer} ${refval}] ${test}
} else {
^ permalink raw reply [flat|nested] 2+ messages in thread* [review] [gdb/tdep] Fix 'Unexpected register class' assert in amd64_push_argum...
[not found] <gerrit.1571043259000.I8b66345bbf5c00209ca75b1209fd4d60b36e9ede@gnutoolchain-gerrit.osci.io>
2019-10-16 15:13 ` [review] [gdb/tdep] Fix 'Unexpected register class' assert in amd64_push_argum Sourceware to Gerrit sync (Code Review)
@ 2019-10-16 15:13 ` Sourceware to Gerrit sync (Code Review)
1 sibling, 0 replies; 2+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-10-16 15:13 UTC (permalink / raw)
To: Tom de Vries, gdb-patches; +Cc: Andrew Burgess, Simon Marchi
Sourceware to Gerrit sync has submitted this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/30
......................................................................
[gdb/tdep] Fix 'Unexpected register class' assert in amd64_push_arguments
Atm, when executing gdb.base/infcall-nested-structs.exp on x86_64-linux, we get:
...
FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-tc-tf: \
p/d check_arg_struct_02_01 (ref_val_struct_02_01)
FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ts-tf: \
p/d check_arg_struct_02_01 (ref_val_struct_02_01)
FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ti-tf: \
p/d check_arg_struct_02_01 (ref_val_struct_02_01)
=== gdb Summary ===
nr of expected passes 9255
nr of unexpected failures 3
nr of expected failures 142
...
The 3 FAILs are reported as PR tdep/25096.
The 142 XFAILs are for a gdb assertion failure, reported in PR tdep/24104,
which should have been KFAILs since there's a problem in gdb rather than in
the environment.
A minimal version of the assertion failure looks like this. Consider test.c:
...
struct s { struct { } es1; long f; };
struct s ref = { {}, 'f' };
int __attribute__((noinline,noclone)) check (struct s arg)
{ return arg.f == 'f'; }
int main (void)
{ return check (ref); }
...
When calling 'check (ref)' from main, we have '1' as expected:
...
$ g++ test3.c -g && ( ./a.out; echo $? )
1
...
But when calling 'check (ref)' from the gdb prompt, we get:
...
$ gdb a.out -batch -ex start -ex "p check (ref)"
Temporary breakpoint 1 at 0x4004f7: file test.c, line 8.
Temporary breakpoint 1, main () at test.c:8
8 { return check (ref); }
src/gdb/amd64-tdep.c:982: internal-error: \
CORE_ADDR amd64_push_arguments(regcache*, int, value**, CORE_ADDR, \
function_call_return_method): \
Assertion `!"Unexpected register class."' failed.
...
The assert happens in this loop in amd64_push_arguments:
...
for (j = 0; len > 0; j++, len -= 8)
{
int regnum = -1;
int offset = 0;
switch (theclass[j])
{
case AMD64_INTEGER:
regnum = integer_regnum[integer_reg++];
break;
case AMD64_SSE:
regnum = sse_regnum[sse_reg++];
break;
case AMD64_SSEUP:
gdb_assert (sse_reg > 0);
regnum = sse_regnum[sse_reg - 1];
offset = 8;
break;
default:
gdb_assert (!"Unexpected register class.");
}
...
}
...
when processing theclass[0], which is AMD64_NO_CLASS:
...
(gdb) p theclass
$1 = {AMD64_NO_CLASS, AMD64_INTEGER}
...
The layout of struct s is that the empty field es1 occupies one byte (due to
c++) at offset 0, and the long field f occupies 8 bytes at offset 8.
When compiling at -O2, we can see from the disassembly of main:
...
4003f0: 48 8b 3d 41 0c 20 00 mov 0x200c41(%rip),%rdi \
# 601038 <ref+0x8>
4003f7: e9 e4 00 00 00 jmpq 4004e0 <_Z5check1s>
4003fc: 0f 1f 40 00 nopl 0x0(%rax)
...
that check is called with field f passed in %rdi, meaning that the
classification in theclass is correct, it's just not supported in the loop in
amd64_push_arguments mentioned above.
Fix the assert by implementing support for 'AMD64_NO_CLASS' in that loop.
This exposes 9 more FAILs of the PR tdep/25096 type, so mark all 12 of them as
KFAIL.
Tested on x86_64-linux.
Tested with g++ 4.8.5, 7.4.1, 8.3.1, 9.2.1. With 4.8.5, 3 of the 12 KFAILs
are KPASSing.
Tested with clang++ 5.0.2 (which requires removing additional_flags=-Wno-psabi
and adding additional_flags=-Wno-deprecated).
gdb/ChangeLog:
2019-10-16 Tom de Vries <tdevries@suse.de>
PR tdep/24104
* amd64-tdep.c (amd64_push_arguments): Handle AMD64_NO_CLASS in loop
that handles 'theclass'.
gdb/testsuite/ChangeLog:
2019-10-16 Tom de Vries <tdevries@suse.de>
PR tdep/24104
* gdb.base/infcall-nested-structs.exp: Remove XFAIL for PR tdep/24104.
Add KFAIL for PR tdep/25096.
Change-Id: I8b66345bbf5c00209ca75b1209fd4d60b36e9ede
---
M gdb/ChangeLog
M gdb/amd64-tdep.c
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.base/infcall-nested-structs.exp
4 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3827c4d..eeba0ee 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-10-16 Tom de Vries <tdevries@suse.de>
+
+ PR tdep/24104
+ * amd64-tdep.c (amd64_push_arguments): Handle AMD64_NO_CLASS in loop
+ that handles 'theclass'.
+
2019-10-15 Andrew Burgess <andrew.burgess@embecosm.com>
* linespec.c (decode_digits_ordinary): Update comment.
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 232d16d..9006ec0 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -978,6 +978,9 @@
offset = 8;
break;
+ case AMD64_NO_CLASS:
+ continue;
+
default:
gdb_assert (!"Unexpected register class.");
}
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 877d0de..3277ee3 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,11 @@
2019-10-16 Tom de Vries <tdevries@suse.de>
+ PR tdep/24104
+ * gdb.base/infcall-nested-structs.exp: Remove XFAIL for PR tdep/24104.
+ Add KFAIL for PR tdep/25096.
+
+2019-10-16 Tom de Vries <tdevries@suse.de>
+
PR testsuite/25059
* gdb.cp/local-static.exp (do_test): Add xfails for gcc PR debug/55541.
diff --git a/gdb/testsuite/gdb.base/infcall-nested-structs.exp b/gdb/testsuite/gdb.base/infcall-nested-structs.exp
index f5fbf44..957eb31 100644
--- a/gdb/testsuite/gdb.base/infcall-nested-structs.exp
+++ b/gdb/testsuite/gdb.base/infcall-nested-structs.exp
@@ -132,16 +132,9 @@
continue
}
- if { $lang == "c++"
- && ( ( [regexp "struct_01_0(1|2|3)" $name match] && [regexp "^types-(td($|-)|tl(|l)(|-tf|-td|-tld)$)" $types match] )
- || ( $name == "struct_01_02" && $types == "types-tfc" )
- || ( $name == "struct_01_04" && [regexp "^types-(tf($|-)|ti(|-tf|-td|-tld)$)" $types match] )
- || ( $name == "struct_02_01" && [regexp "^types-tf-t(c|s|i)" $types match] )
- || ( $name == "struct_static_02_02" && [regexp "^types-(t(f|d|ld)-t(d|l|ll)$|t(d|l|ll)$|t(c|s|i|l|ll)-td)" $types match] )
- || ( $name == "struct_static_02_03" && [regexp "^types-(ti-t(f|l|d|)|tf(-|$)|ti$)" $types match] )
- || ( $name == "struct_static_04_02" && [regexp "^types-(t(c|s)-tf|tf-ts)" $types match] )
- || ( $name == "struct_static_06_04" && ![regexp "^types-(t(c|dc|ldc|ld)$|t.-tld|tl(l|d)-tld|t(f|d|ld)-tc)" $types match] ) ) } {
- setup_xfail gdb/24104 "x86_64-*-linux*"
+ if { $lang == "c++" && $name == "struct_02_01"
+ && [regexp "^types-(tf-t(c|s|i)|t(c|s|i)-tf)" $types match] } {
+ setup_kfail gdb/25096 "x86_64-*-linux*"
}
gdb_test "p/d check_arg_${name} (ref_val_${name})" "= 1"
@@ -154,8 +147,9 @@
set answer [ get_valueof "" "rtn_str_${name} ()" "XXXX"]
verbose -log "Answer: ${answer}"
- if { ($lang == "c++" && $name == "struct_02_01" && [regexp "^types-(tf-t(c|s|i)|t(c|s|i)-tf)" $types match] ) } {
- setup_xfail gdb/24104 "x86_64-*-linux*"
+ if { $lang == "c++" && $name == "struct_02_01"
+ && [regexp "^types-(tf-t(c|s|i)|t(c|s|i)-tf)" $types match] } {
+ setup_kfail gdb/25096 "x86_64-*-linux*"
}
gdb_assert [string eq ${answer} ${refval}] ${test}
} else {
^ permalink raw reply [flat|nested] 2+ messages in thread