[review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments

Tue Jan 14 13:31:00 GMT 2020

On 1/14/20 9:52 AM, Aktemur, Tankut Baris wrote:
> On Monday, January 13, 2020 9:21 PM, Aktemur, Tankut Baris wrote:
>>
>> On Monday, January 13, 2020 8:38 PM, Luis Machado wrote:
>>>
>>> On 1/13/20 4:35 PM, Aktemur, Tankut Baris wrote:
>>>> On Monday, January 13, 2020 7:21 PM, Luis Machado wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I noticed these new tests fail (in large numbers) for GCC 5.4.x. Is it
>>>>> expected? If so, it may be worth making this test conditional on newer
>>>>> GCC versions.
>>>>>
>>>>
>>>> Yes, this is expected. Older GCC versions did not emit certain DWARF attributes
>>>> (DW_AT_deleted, DW_AT_defaulted).  This prevents GDB from making the right
>>>> pass-by-reference decision.  I'll submit a patch for this.
>>>
>>> Thanks for clarifying this.
>>>
>>> I can submit that patch if you like. I have a box running an older GCC,
>>> so i noticed that and thought i'd check.
>>>
>>> Luis
>>
>> Yes, sure.
>>
>> Thank you.
>>
>> -Baris
> 
> I've investigated GCC and Clang for this.  GCC started emitting DW_AT_deleted and
> DW_AT_defaulted with version 7.  Clang does not emit these attributes; however, it
> has been emitting DW_AT_calling_convention starting with version 7.  This
> attribute helps the debugger make the right decision in some cases.
> 
> Based on this, I think the test cases have to be filtered in a somewhat
> fine-granular manner.  Therefore I thought I could save you from the burden of
> having to go through the code-generating test definition.  Below is a patch proposal.
> 
> -Baris

Thanks! I've checked this on my box with an older GCC and i see the 
XFAIL's now. So it looks good to me.

Small nit below...

> 
> 
> 
>  From 86017121c8f1c7d0f0020c7a8da22dba64dad3fd Mon Sep 17 00:00:00 2001
> From: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
> Date: Tue, 14 Jan 2020 13:36:09 +0100
> Subject: [PATCH] testsuite, cp: add pass-by-ref test expected failures for
>   certain compilers
> 
> There exist expected failures in the pass-by-ref.exp and
> pass-by-ref-2.exp tests based on the GCC and Clang version.
> 
> * GCC version <= 6 and Clang do not emit DW_AT_deleted and
>    DW_AT_defaulted.
> 
> * Clang version >= 7 emits DW_AT_calling_convention, which helps the
>    debugger make the right calling convention decision in some cases
>    despite lacking the 'defaulted' and 'deleted' attributes.
> 
> Mark the related tests as XFAIL based on the compiler version.
> 
> Tested on X86_64 using GCC 5.5.0, 6.5.0, 7.4.0, 8.3.0, 9.2.1;
> and Clang 5.0.1, 6.0.0, 7.0.0, 8.0.0, 9.0.1, 10.0.0.
> 
> gdb/testsuite/ChangeLog:
> 2020-01-14  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> 
> 	* gdb.cp/pass-by-ref.exp: Mark some tests as XFAIL based on the
> 	GCC/Clang version.
> 	* gdb.cp/pass-by-ref-2.exp: Ditto.
> 
> Change-Id: I1d8440aa438049f7c4da7f4f76f201c48550f1e4
> 
> ---
>   gdb/testsuite/gdb.cp/pass-by-ref-2.exp |  6 ++++++
>   gdb/testsuite/gdb.cp/pass-by-ref.exp   | 26 ++++++++++++++++++++++++++
>   2 files changed, 32 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.exp b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> index a83ce8d5d7d..e7b04771eb2 100644
> --- a/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> +++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> @@ -42,6 +42,10 @@ if {![runto_main]} {
>       return -1
>   }
>   
> +# GCC version <= 6 and Clang do not emit DW_AT_defaulted and DW_AT_deleted
> +set is_gcc_6_or_older [test_compiler_info {gcc-[0-6]-*}]
> +set is_clang [test_compiler_info {clang-*}]
> +
>   set bp_location [gdb_get_line_number "stop here"]
>   gdb_breakpoint $bp_location
>   gdb_continue_to_breakpoint "end of main" ".*return .*;"

It seems to be a mixed bag, but i see more examples of having a period 
after the sentence than not having it. Multiple cases of this on the patch.

> @@ -65,6 +69,7 @@ set sig "\"Inlined\:\:Inlined\\(.*Inlined const\&\\)\""
>   gdb_test "print cbvInlined (inlined)" \
>       "expression cannot be evaluated .* \\(maybe inlined\\?\\)"
>   
> +if {$is_gcc_6_or_older || $is_clang} {setup_xfail "*-*-*"}
>   gdb_test "print cbvDtorDel (*dtorDel)" \
>       ".* cannot be evaluated .* 'DtorDel' is not destructible" \
>       "type not destructible"
> @@ -94,6 +99,7 @@ gdb_test "print cbvTwoMCtor (twoMctor)" \
>       ".* cannot be evaluated .* 'TwoMCtor' is not copy constructible" \
>       "copy ctor is implicitly deleted"
>   
> +if {$is_gcc_6_or_older || $is_clang} {setup_xfail "*-*-*"}
>   gdb_test "print cbvTwoMCtorAndCCtor (twoMctorAndCctor)" "12" \
>       "call cbvTwoMCtorAndCCtor"
>   gdb_test "print twoMctorAndCctor.x" "2" \
> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.exp b/gdb/testsuite/gdb.cp/pass-by-ref.exp
> index f500710fd43..aff9f9a3c1e 100644
> --- a/gdb/testsuite/gdb.cp/pass-by-ref.exp
> +++ b/gdb/testsuite/gdb.cp/pass-by-ref.exp
> @@ -369,6 +369,12 @@ proc test_for_class { prefix states cbvfun data_field length} {
>       global ADDED
>       global TRACE
>   
> +    # GCC version <= 6 and Clang do not emit DW_AT_defaulted and DW_AT_deleted
> +    set is_gcc_6_or_older [test_compiler_info {gcc-[0-6]-*}]
> +    set is_clang [test_compiler_info {clang-*}]
> +    # But Clang version >= 7 emits DW_AT_calling_convention for types
> +    set is_clang_6_or_older [test_compiler_info {clang-[0-6]-*}]
> +
>       with_test_prefix $name {
>   	if {[is_copy_constructible $states]} {
>   	    set expected [expr {$ORIGINAL + $ADDED}]
> @@ -378,6 +384,19 @@ proc test_for_class { prefix states cbvfun data_field length} {
>   	    if {$dtor == "explicit"} {
>   		gdb_test "print tracer = 0" " = 0" "reset the tracer"
>   	    }
> +
> +	    if {$cctor == "defaultedIn" || $dtor == "defaultedIn"} {
> +		if {$is_gcc_6_or_older || $is_clang_6_or_older} {
> +		    setup_xfail "*-*-*"
> +		} elseif {$is_clang} {
> +		    # If this is a pass-by-value case, Clang >= 7's
> +		    # DW_AT_calling_convention leads to the right decision.
> +		    # Otherwise, it is expected to fail.
> +		    if {"defaultedOut" in $states || "explicit" in $states} {
> +			setup_xfail "*-*-*"
> +		    }
> +		}
> +	    }
>   	    gdb_test "print ${cbvfun}<$name> (${name}_var)" " = $expected" \
>   		"call '$cbvfun'"
>   	    gdb_test "print ${name}_var.${data_field}\[0\]" " = $ORIGINAL" \
> @@ -389,10 +408,17 @@ proc test_for_class { prefix states cbvfun data_field length} {
>   		    "cbv argument should not change (item $last_index)"
>   	    }
>   	    if {$dtor == "explicit"} {
> +		if {$cctor == "defaultedIn"
> +		    && ($is_gcc_6_or_older || $is_clang)} {
> +		    setup_xfail "*-*-*"
> +		}
>   		gdb_test "print tracer" " = $TRACE" \
>   		    "destructor should be called"
>   	    }
>   	} else {
> +	    if {$cctor == "deleted" && ($is_gcc_6_or_older || $is_clang)} {
> +		setup_xfail "*-*-*"
> +	    }
>   	    gdb_test "print ${cbvfun}<$name> (${name}_var)" \
>   		".* cannot be evaluated .* '${name}' is not copy constructible" \
>   		"calling '$cbvfun' should be refused"
> 