[1/2] AArch64 AAPCS: Empty structs have non zero size in C++
Commit Message
When gdb.base/infcall-nested-structs.c is complied as C++, the structs
containing empty structs are no longer passed via float arguments.
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.
Add padding checks to AArch64 and add C++ compile variant to the test.
gdb/ChangeLog:
2019-01-16 Alan Hayward <alan.hayward@arm.com>
* aarch64-tdep.c (aapcs_is_vfp_call_or_return_candidate_1): Check
for padding.
gdb/testsuite/ChangeLog:
2019-01-16 Alan Hayward <alan.hayward@arm.com>
* gdb.base/infcall-nested-structs.exp: Test C++ in addition to C.
---
gdb/aarch64-tdep.c | 8 ++++
.../gdb.base/infcall-nested-structs.exp | 42 +++++++++++++------
2 files changed, 38 insertions(+), 12 deletions(-)
Comments
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.
> +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.
Could you please file a bug for the x86 internal errors, and
kfail the test for x86?
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]
> }
> }
> }
>
(Thanks, would have pushed, but outstanding question below)…
> On 17 Jan 2019, at 17:08, Pedro Alves <palves@redhat.com> 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]
>> }
>> }
>> }
>>
>
@@ -1392,6 +1392,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;
}
@@ -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 build "rm -rf [standard_output_file ${dir}]"
+ remote_exec build "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 }
@@ -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]
}
}
}