From patchwork Mon Jan 21 15:52:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 31138 Received: (qmail 8191 invoked by alias); 21 Jan 2019 15:52:21 -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 8049 invoked by uid 89); 21 Jan 2019 15:52:20 -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, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=tll, ta, allowing X-HELO: EUR02-VE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr20064.outbound.protection.outlook.com (HELO EUR02-VE1-obe.outbound.protection.outlook.com) (40.107.2.64) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 21 Jan 2019 15:52:14 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=N6ds7VCmaZ/GdWYg3mfBv+3c9U2hmP+yHtx13uRSdRM=; b=R6hsn5f/hbnnDAQeisggiIg0B03eyVnpiiFqQGlOa8iyc2xKMsy46n6Bfkpv39Xjl7ouerKzB9gW7P+YgfKh0fMGoLc67hhfNlkvnJ3HA+746aHVrDY++RzGiTbQgHQxC+4GA0vr9VXiT769gxwIqAneaTC8aonvl2LzkHBlyU4= Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com (10.172.227.22) by DB6PR0802MB2357.eurprd08.prod.outlook.com (10.172.228.136) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1537.24; Mon, 21 Jan 2019 15:52:11 +0000 Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::acd7:a958:2aaa:562e]) by DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::acd7:a958:2aaa:562e%5]) with mapi id 15.20.1537.031; Mon, 21 Jan 2019 15:52:11 +0000 From: Alan Hayward To: Pedro Alves CC: "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH 1/2] AArch64 AAPCS: Empty structs have non zero size in C++ Date: Mon, 21 Jan 2019 15:52:11 +0000 Message-ID: <018D8C19-1A45-4012-BAC6-90ACDA074740@arm.com> References: <20190116155734.53824-1-alan.hayward@arm.com> <20190116155734.53824-2-alan.hayward@arm.com> <386a4a7f-f7df-e1da-42b8-b0724e1e36b2@redhat.com> <5255A57D-0F2C-4A3B-816A-6F46C0B0C2B1@arm.com> In-Reply-To: <5255A57D-0F2C-4A3B-816A-6F46C0B0C2B1@arm.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) Content-ID: MIME-Version: 1.0 X-IsSubscribed: yes Pushed with changes suggested, including all xpass matches. Alan. > On 18 Jan 2019, at 10:34, Alan Hayward wrote: > > (Thanks, would have pushed, but outstanding question below)… > > >> On 17 Jan 2019, at 17:08, Pedro Alves wrote: >> >> On 01/16/2019 03:57 PM, Alan Hayward wrote: >>> When gdb.base/infcall-nested-structs.c is complied as C++, the structs >>> containing empty structs are no longer passed via float arguments. >> >> This reads a bit ambiguously. Which is it? >> >> #1 - No longer passed by GCC, but GDB still passes. >> #2 - No longer passed by GDB, but GCC still passes. >> >>> This is because structs in C++ have a minimum size of 1. This can then >>> cause padding in the struct, which is disallowed for AAPCS. >> >> Does this "disallowed" mean that structs with padding are >> not allowed to be passed via float arguments? Took me a while to >> grok that. >> >> It'd be good to clarify the commit log. > > I’ll change to: > When gdb.base/infcall-nested-structs.c is complied as C++, the compiler > will not pass structs containing empty structs via float arguments. > This is because structs in C++ have a minimum size of 1, causing padding > in the struct. The AAPCS does not allow structs with padding to be > passed in float arguments. > >>> +foreach l $lang { >>> + set dir "$l" >>> + remote_exec build "rm -rf [standard_output_file ${dir}]" >>> + remote_exec build "mkdir -p [standard_output_file ${dir}]" >> >> I think these should be >> >> remote_exec host >> >> not "build" ? >> >> For remote-host testing, where the compiler and debugger run on the >> host machine. >> > > This was due to copying from another test - I’ll raise a quick > patch to fix those up too. > > >> Could you please file a bug for the x86 internal errors, and >> kfail the test for x86? >> > > Will raise a bug. > > The pattern for which tests pass and fail is not that simple. > Each structure gets tested 49 times on c++ (with different types). > Eg: For struct_01_01, 17 of them fail, but for struct_01_04 only > 13 fail. > > Is it ok to be over cautious (and have some XPASS results) > or do we really need a large messy if statement with all the > exact matches? > > >> Otherwise looks fine to me. >> >> Thanks, >> Pedro Alves >> >>> +} >>> + >>> + >>> set int_types { tc ts ti tl tll } >>> set float_types { tf td tld } >>> set complex_types { tfc tdc tldc } >>> @@ -44,7 +58,7 @@ proc I2A { n } { >>> # types of the struct fields within the source. Run up to main. >>> # Also updates the global "testfile" to reflect the most recent build. >>> >>> -proc start_nested_structs_test { types } { >>> +proc start_nested_structs_test { lang types } { >>> global testfile >>> global srcfile >>> global binfile >>> @@ -53,9 +67,11 @@ proc start_nested_structs_test { types } { >>> global compile_flags >>> >>> standard_testfile .c >>> + set dir "$lang" >>> >>> # Create the additional flags >>> set flags $compile_flags >>> + lappend flags $lang >>> >>> for {set n 0} {$n<[llength ${types}]} {incr n} { >>> set m [I2A ${n}] >>> @@ -64,7 +80,7 @@ proc start_nested_structs_test { types } { >>> append testfile "-" "$t" >>> } >>> >>> - set binfile [standard_output_file ${testfile}] >>> + set binfile [standard_output_file ${dir}/${testfile}] >>> if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable "${flags}"] != "" } { >>> unresolved "failed to compile" >>> return 0 >>> @@ -125,48 +141,50 @@ proc run_tests {} { >>> # Set up a test prefix, compile the test binary, run to main, and then >>> # run some tests. >>> >>> -proc start_gdb_and_run_tests { types } { >>> +proc start_gdb_and_run_tests { lang types } { >> >>> set prefix "types" >>> >>> foreach t $types { >>> append prefix "-" "${t}" >>> } >>> >>> - with_test_prefix $prefix { >>> - if { [start_nested_structs_test $types] } { >>> - run_tests >>> + foreach_with_prefix l $lang { >>> + with_test_prefix $prefix { >>> + if { [start_nested_structs_test $l $types] } { >>> + run_tests >>> + } >>> } >>> } >>> } >>> >>> foreach ta $int_types { >>> - start_gdb_and_run_tests $ta >>> + start_gdb_and_run_tests $lang $ta >>> } >>> >>> if [support_complex_tests] { >>> foreach ta $complex_types { >>> - start_gdb_and_run_tests $ta >>> + start_gdb_and_run_tests $lang $ta >>> } >>> } >>> >>> if ![gdb_skip_float_test] { >>> foreach ta $float_types { >>> - start_gdb_and_run_tests $ta >>> + start_gdb_and_run_tests $lang $ta >>> } >>> >>> foreach ta $int_types { >>> foreach tb $float_types { >>> - start_gdb_and_run_tests [list $ta $tb] >>> + start_gdb_and_run_tests $lang [list $ta $tb] >>> } >>> } >>> >>> foreach ta $float_types { >>> foreach tb $int_types { >>> - start_gdb_and_run_tests [list $ta $tb] >>> + start_gdb_and_run_tests $lang [list $ta $tb] >>> } >>> >>> foreach tb $float_types { >>> - start_gdb_and_run_tests [list $ta $tb] >>> + start_gdb_and_run_tests $lang [list $ta $tb] >>> } >>> } >>> } >>> >> > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index b051563937..7c5d74858d 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -1232,6 +1232,14 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, return -1; count += sub_count; } + + /* Ensure there is no padding between the fields (allowing for empty + zero length structs) */ + int ftype_length = (*fundamental_type == nullptr) + ? 0 : TYPE_LENGTH (*fundamental_type); + if (count * ftype_length != TYPE_LENGTH (type)) + return -1; + return count; } diff --git a/gdb/testsuite/gdb.base/infcall-nested-structs.exp b/gdb/testsuite/gdb.base/infcall-nested-structs.exp index b04d9aaa84..d7d1e3e00d 100644 --- a/gdb/testsuite/gdb.base/infcall-nested-structs.exp +++ b/gdb/testsuite/gdb.base/infcall-nested-structs.exp @@ -24,6 +24,20 @@ if [target_info exists gdb,cannot_call_functions] { continue } +# Only test C++ if we are able. Always use C. +if { [skip_cplus_tests] || [get_compiler_info "c++"] } { + set lang {c} +} else { + set lang {c c++} +} + +foreach l $lang { + set dir "$l" + remote_exec host "rm -rf [standard_output_file ${dir}]" + remote_exec host "mkdir -p [standard_output_file ${dir}]" +} + + set int_types { tc ts ti tl tll } set float_types { tf td tld } set complex_types { tfc tdc tldc } @@ -31,6 +45,7 @@ set complex_types { tfc tdc tldc } set compile_flags {debug} if [support_complex_tests] { lappend compile_flags "additional_flags=-DTEST_COMPLEX" + lappend compile_flags "additional_flags=-Wno-psabi" } # Given N (0..25), return the corresponding alphabetic letter in upper @@ -44,7 +59,7 @@ proc I2A { n } { # types of the struct fields within the source. Run up to main. # Also updates the global "testfile" to reflect the most recent build. -proc start_nested_structs_test { types } { +proc start_nested_structs_test { lang types } { global testfile global srcfile global binfile @@ -53,9 +68,11 @@ proc start_nested_structs_test { types } { global compile_flags standard_testfile .c + set dir "$lang" # Create the additional flags set flags $compile_flags + lappend flags $lang for {set n 0} {$n<[llength ${types}]} {incr n} { set m [I2A ${n}] @@ -64,7 +81,7 @@ proc start_nested_structs_test { types } { append testfile "-" "$t" } - set binfile [standard_output_file ${testfile}] + set binfile [standard_output_file ${dir}/${testfile}] if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable "${flags}"] != "" } { unresolved "failed to compile" return 0 @@ -99,13 +116,21 @@ proc start_nested_structs_test { types } { # Assuming GDB is stopped at main within a test binary, run some tests # passing structures, and reading return value structures. -proc run_tests {} { +proc run_tests { lang types } { global gdb_prompt foreach {name} {struct_01_01 struct_01_02 struct_01_03 struct_01_04 struct_02_01 struct_02_02 struct_02_03 struct_02_04 struct_04_01 struct_04_02 struct_04_03 struct_04_04 struct_05_01 struct_05_02 struct_05_03 struct_05_04} { + + 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] ) ) ) } { + setup_xfail gdb/24104 "x86_64-*-linux*" + } gdb_test "p/d check_arg_${name} (ref_val_${name})" "= 1" set refval [ get_valueof "" "ref_val_${name}" "" ] @@ -113,8 +138,13 @@ proc run_tests {} { set test "check return value ${name}" if { ${refval} != "" } { + 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*" + } gdb_assert [string eq ${answer} ${refval}] ${test} } else { unresolved $test @@ -125,48 +155,50 @@ proc run_tests {} { # Set up a test prefix, compile the test binary, run to main, and then # run some tests. -proc start_gdb_and_run_tests { types } { +proc start_gdb_and_run_tests { lang types } { set prefix "types" foreach t $types { append prefix "-" "${t}" } - with_test_prefix $prefix { - if { [start_nested_structs_test $types] } { - run_tests + foreach_with_prefix l $lang { + with_test_prefix $prefix { + if { [start_nested_structs_test $l $types] } { + run_tests $l $prefix + } } } } foreach ta $int_types { - start_gdb_and_run_tests $ta + start_gdb_and_run_tests $lang $ta } if [support_complex_tests] { foreach ta $complex_types { - start_gdb_and_run_tests $ta + start_gdb_and_run_tests $lang $ta } } if ![gdb_skip_float_test] { foreach ta $float_types { - start_gdb_and_run_tests $ta + start_gdb_and_run_tests $lang $ta } foreach ta $int_types { foreach tb $float_types { - start_gdb_and_run_tests [list $ta $tb] + start_gdb_and_run_tests $lang [list $ta $tb] } } foreach ta $float_types { foreach tb $int_types { - start_gdb_and_run_tests [list $ta $tb] + start_gdb_and_run_tests $lang [list $ta $tb] } foreach tb $float_types { - start_gdb_and_run_tests [list $ta $tb] + start_gdb_and_run_tests $lang [list $ta $tb] } } }