From patchwork Wed Oct 16 15:13:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 35043 Received: (qmail 79794 invoked by alias); 16 Oct 2019 15:13:14 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 79786 invoked by uid 89); 16 Oct 2019 15:13:13 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy= X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 16 Oct 2019 15:13:12 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 55294208EF; Wed, 16 Oct 2019 11:13:10 -0400 (EDT) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id 93A3C2064E; Wed, 16 Oct 2019 11:13:05 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 5EEE420AF6; Wed, 16 Oct 2019 11:13:05 -0400 (EDT) X-Gerrit-PatchSet: 2 Date: Wed, 16 Oct 2019 11:13:04 -0400 From: "Sourceware to Gerrit sync (Code Review)" To: Tom de Vries , Andrew Burgess , Simon Marchi , gdb-patches@sourceware.org Auto-Submitted: auto-generated X-Gerrit-MessageType: newpatchset Subject: [review] [gdb/tdep] Fix 'Unexpected register class' assert in amd64_push_argum... X-Gerrit-Change-Id: I8b66345bbf5c00209ca75b1209fd4d60b36e9ede X-Gerrit-Change-Number: 30 X-Gerrit-ChangeURL: X-Gerrit-Commit: 745ff14e6e1b88f04eb447d4883fab81650f745f In-Reply-To: References: Reply-To: noreply@gnutoolchain-gerrit.osci.io, simon.marchi@polymtl.ca, tdevries@suse.de, andrew.burgess@embecosm.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3 Message-Id: <20191016151305.5EEE420AF6@gnutoolchain-gerrit.osci.io> 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 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 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 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 + + PR tdep/24104 + * amd64-tdep.c (amd64_push_arguments): Handle AMD64_NO_CLASS in loop + that handles 'theclass'. + 2019-10-15 Andrew Burgess * 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 + 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 + 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 {