diff mbox

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

Message ID BYAPR11MB30300020F6B609A0C4B9E9BBC40A0@BYAPR11MB3030.namprd11.prod.outlook.com
State New
Headers show

Commit Message

Tankut Baris Aktemur Jan. 28, 2020, 10:40 a.m. UTC
On Wednesday, January 15, 2020 3:30 PM, Aktemur, Tankut Baris wrote:
> On Tuesday, January 14, 2020 2:13 PM, Luis Machado wrote:

> >

> > On 1/14/20 9:52 AM, Aktemur, Tankut Baris wrote:

> > >

> > > 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...

> >

> 

> Thank you.  I'll fix that and wait for an official approval.

> 

> -Baris


Kindly pinging.

> 

> > > +# 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.

> >


The version with Luis' comment addressed is below.

Thanks,
-Baris

From 3d1f0e23fb4aa647d41743be2c4eb8e5bc4b5328 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 v2] 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-28  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.cp/pass-by-ref-2.exp: Mark some tests as XFAIL based on the
	GCC/Clang version.
	* gdb.cp/pass-by-ref.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(+)

-- 
2.17.1



Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Comments

Simon Marchi Jan. 29, 2020, 2:52 a.m. UTC | #1
On 2020-01-28 5:40 a.m., Aktemur, Tankut Baris wrote:
> The version with Luis' comment addressed is below.

Thanks, that looks good to me.

Simon
Tankut Baris Aktemur Jan. 29, 2020, 10:47 a.m. UTC | #2
On 

> -----Original Message-----

> From: gdb-patches-owner@sourceware.org <gdb-patches-owner@sourceware.org> On Behalf Of Simon Marchi

> Sent: Wednesday, January 29, 2020 3:53 AM

> To: Aktemur, Tankut Baris <tankut.baris.aktemur@intel.com>; Luis Machado <luis.machado@linaro.org>;

> tromey@sourceware.org; gdb-patches@sourceware.org

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

> 

> On 2020-01-28 5:40 a.m., Aktemur, Tankut Baris wrote:

> > The version with Luis' comment addressed is below.

> 

> Thanks, that looks good to me.

> 

> Simon


5f440116e87
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Tankut Baris Aktemur Jan. 29, 2020, 10:54 a.m. UTC | #3
> > The version with Luis' comment addressed is below.

> 

> Thanks, that looks good to me.

> 

> Simon


Thank you.  Pushed with commit hash 5f440116e87.

(Sorry about the previous email.  It went out by mistake before it was complete.)

-Baris

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.exp b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
index a83ce8d5d7d..913f9af3ee7 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 .*;"
@@ -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..06a36129e83 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"