public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments
@ 2019-10-18 13:53 Tankut Baris Aktemur (Code Review)
  2019-10-28 22:06 ` Tom Tromey (Code Review)
                   ` (10 more replies)
  0 siblings, 11 replies; 25+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-10-18 13:53 UTC (permalink / raw)
  To: gdb-patches

Tankut Baris Aktemur has uploaded a new change for review.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................

testsuite, cp: increase the coverage of testing pass-by-ref arguments

Extend testcases for GDB's infcall of call-by-value functions that
take aggregate values as parameters.  In particular, existing test has
been substantially extended with class definitions whose definitions
of copy constructor, destructor, and move constructor functions are a
combination of

(1) explicitly defined by the user,
(2) defaulted inside the class declaration,
(3) defaulted outside the class declaration,
(4) deleted
(5) not defined in the source.

For each combination, a small and a large class is generated as well
as a derived class and a container class.  Additionally, the following
manually-written cases are provided:

- a dynamic class (i.e. class with a virtual method)
- classes that contain an array field
- a class whose copy ctor is inlined
- a class whose destructor is deleted

Test cases check whether GDB makes the right decision to pass an
object by value or implicitly by reference, whether really a copy of
the argument is passed, and whether the copy constructor and
destructor of the clone of the argument are invoked properly.

The input program pass-by-ref.cc is generated.  By means of a flag
inside the test file, the program can be re-generated.  The input
program pass-by-ref-2.cc is manually-written.

Tests have been verified on the X86_64 architecture with
GCC 7.4.0 and 8.2.0.

gdb/testsuite/ChangeLog:
2019-MM-DD  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.cp/pass-by-ref.cc: Extend with more cases (generated).
	* gdb.cp/pass-by-ref.exp: Extend with more cases.
	* gdb.cp/pass-by-ref-2.cc: New file.
	* gdb.cp/pass-by-ref-2.exp: New file.

Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
---
A gdb/testsuite/gdb.cp/pass-by-ref-2.cc
A gdb/testsuite/gdb.cp/pass-by-ref-2.exp
M gdb/testsuite/gdb.cp/pass-by-ref.cc
M gdb/testsuite/gdb.cp/pass-by-ref.exp
4 files changed, 8,015 insertions(+), 45 deletions(-)




^ permalink raw reply	[flat|nested] 25+ messages in thread

* [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
@ 2019-10-28 22:06 ` Tom Tromey (Code Review)
  2019-10-31 13:19 ` Tankut Baris Aktemur (Code Review)
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey (Code Review) @ 2019-10-28 22:06 UTC (permalink / raw)
  To: Tankut Baris Aktemur, gdb-patches

Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................


Patch Set 1:

Thank you for the patch.

This looks pretty good to me.

I wanted to verify that the tests failed without your series.

Also, I think the generated file could just be generated each test run.
Then this wouldn't need a special mode to regenerate the file.
I think there are other tests that do this already.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Mon, 28 Oct 2019 22:06:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
  2019-10-28 22:06 ` Tom Tromey (Code Review)
@ 2019-10-31 13:19 ` Tankut Baris Aktemur (Code Review)
  2019-11-01 15:18   ` Tom Tromey
  2019-12-09 17:45 ` [review v2] " Tankut Baris Aktemur (Code Review)
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-10-31 13:19 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Tankut Baris Aktemur has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> Thank you for the patch.
> 
> This looks pretty good to me.
> 
> I wanted to verify that the tests failed without your series.
> 

Yes, tests fail without the patches in the series.  To give more details on this:

Without the series, using g++ 8.3.0 or 7.4.0, the pass/fail result is 378/546.

With the series;
* using g++ 8.3.0 or 7.4.0, it is 924/0.
* using g++ 5.5.0 or 6.5.0, it is 699/225.
* using clang++ 7.0.0 or 8.0.0, it is 727/197.

For the latter two, the main reason for failures is that the compiler does not emit the DW_AT_defaulted and DW_AT_deleted attributes.  There are more passes with clang because it emits DW_AT_calling_convention, which gcc does not, and the patch I sent favors compiler's hint if it detects a conflict between the DW_AT_calling_convention and the inferred result.

> Also, I think the generated file could just be generated each test run.
> Then this wouldn't need a special mode to regenerate the file.
> I think there are other tests that do this already.

Done.  I'll send this with the next revision; I want to revise the other patches in the series, too, and send the v2 in one go.

Thanks.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Thu, 31 Oct 2019 13:19:28 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-31 13:19 ` Tankut Baris Aktemur (Code Review)
@ 2019-11-01 15:18   ` Tom Tromey
  2019-11-08  8:08     ` Aktemur, Tankut Baris
  0 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2019-11-01 15:18 UTC (permalink / raw)
  To: Tankut Baris Aktemur (Code Review)
  Cc: gdb-patches, gnutoolchain-gerrit, Tom Tromey

>>>>> "Tankut" == Tankut Baris Aktemur (Code Review) <gerrit@gnutoolchain-gerrit.osci.io> writes:

Tankut> For the latter two, the main reason for failures is that the compiler
Tankut> does not emit the DW_AT_defaulted and DW_AT_deleted attributes.  There
Tankut> are more passes with clang because it emits DW_AT_calling_convention,
Tankut> which gcc does not, and the patch I sent favors compiler's hint if it
Tankut> detects a conflict between the DW_AT_calling_convention and the
Tankut> inferred result.

Could you file a GCC bug report for this?

Tom

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-11-01 15:18   ` Tom Tromey
@ 2019-11-08  8:08     ` Aktemur, Tankut Baris
  0 siblings, 0 replies; 25+ messages in thread
From: Aktemur, Tankut Baris @ 2019-11-08  8:08 UTC (permalink / raw)
  To: Tom Tromey, Tankut Baris Aktemur (Code Review)
  Cc: gdb-patches, gnutoolchain-gerrit, Tom Tromey

* On Friday, November 1, 2019 4:19 PM, Tom Tromey wrote:
> >>>>> "Tankut" == Tankut Baris Aktemur (Code Review) <gerrit@gnutoolchain-gerrit.osci.io> writes:
> 
> Tankut> For the latter two, the main reason for failures is that the compiler
> Tankut> does not emit the DW_AT_defaulted and DW_AT_deleted attributes.  There
> Tankut> are more passes with clang because it emits DW_AT_calling_convention,
> Tankut> which gcc does not, and the patch I sent favors compiler's hint if it
> Tankut> detects a conflict between the DW_AT_calling_convention and the
> Tankut> inferred result.
> 
> Could you file a GCC bug report for this?
> 
> Tom

Yes.  Filed here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92418

I'll put this link in the patch, too.

-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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [review v2] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
  2019-10-28 22:06 ` Tom Tromey (Code Review)
  2019-10-31 13:19 ` Tankut Baris Aktemur (Code Review)
@ 2019-12-09 17:45 ` Tankut Baris Aktemur (Code Review)
  2019-12-09 17:56 ` Tankut Baris Aktemur (Code Review)
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-12-09 17:45 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................

testsuite, cp: increase the coverage of testing pass-by-ref arguments

Extend testcases for GDB's infcall of call-by-value functions that
take aggregate values as parameters.  In particular, existing test has
been substantially extended with class definitions whose definitions
of copy constructor, destructor, and move constructor functions are a
combination of

(1) explicitly defined by the user,
(2) defaulted inside the class declaration,
(3) defaulted outside the class declaration,
(4) deleted
(5) not defined in the source.

For each combination, a small and a large class is generated as well
as a derived class and a container class.  Additionally, the following
manually-written cases are provided:

- a dynamic class (i.e. class with a virtual method)
- classes that contain an array field
- a class whose copy ctor is inlined
- a class whose destructor is deleted
- classes with multiple copy and/or move ctors

Test cases check whether GDB makes the right decision to pass an
object by value or implicitly by reference, whether really a copy of
the argument is passed, and whether the copy constructor and
destructor of the clone of the argument are invoked properly.

The input program pass-by-ref.cc is generated each time the test is
executed.  The input program pass-by-ref-2.cc is manually-written.

Tests have been verified on the X86_64 architecture with
GCC 7.4.0, 8.2.0, and 9.2.1.

gdb/testsuite/ChangeLog:
2019-11-07  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.cp/pass-by-ref.cc: Extend with more cases (generated).
	* gdb.cp/pass-by-ref.exp: Extend with more cases.
	* gdb.cp/pass-by-ref-2.cc: New file.
	* gdb.cp/pass-by-ref-2.exp: New file.

Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
---
A gdb/testsuite/gdb.cp/pass-by-ref-2.cc
A gdb/testsuite/gdb.cp/pass-by-ref-2.exp
M gdb/testsuite/gdb.cp/pass-by-ref.cc
M gdb/testsuite/gdb.cp/pass-by-ref.exp
4 files changed, 8,173 insertions(+), 69 deletions(-)




-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [review v2] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
                   ` (2 preceding siblings ...)
  2019-12-09 17:45 ` [review v2] " Tankut Baris Aktemur (Code Review)
@ 2019-12-09 17:56 ` Tankut Baris Aktemur (Code Review)
  2019-12-13 21:38 ` Tom Tromey (Code Review)
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-12-09 17:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Tankut Baris Aktemur has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................


Patch Set 2:

I've added more test cases to also test overload resolution in the presence of multiple copy constructors.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Mon, 09 Dec 2019 17:56:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [review v2] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
                   ` (3 preceding siblings ...)
  2019-12-09 17:56 ` Tankut Baris Aktemur (Code Review)
@ 2019-12-13 21:38 ` Tom Tromey (Code Review)
  2019-12-14  9:53 ` [review v3] " Tankut Baris Aktemur (Code Review)
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey (Code Review) @ 2019-12-13 21:38 UTC (permalink / raw)
  To: Tankut Baris Aktemur, gdb-patches

Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................


Patch Set 2:

(1 comment)

Thanks for the patch.
I think it needs another modification, see below.

| --- gdb/testsuite/gdb.cp/pass-by-ref.exp
| +++ gdb/testsuite/gdb.cp/pass-by-ref.exp
| @@ -24,4 +343,18 @@ foreach state $all_combinations {
| +    append stmts [generate_stmts "Derived" $state]
| +
| +    append classes [generate_container_class $state]
| +    append stmts [generate_stmts "Container" $state "cbv_container"]
| +}
| +
| +# Generate the program code and compile
| +set program [generate_program $classes $stmts]
| +set testfolder [file dirname $testfile]
| +gdb_produce_source ${srcdir}/${subdir}/$srcfile $program

PS2, Line 352:

I think the generated source should not be put into the source
directory; nor should it be part of the patch (or checked in).
Instead it can be built in test case's output directory.

| +
| +set options {debug c++ additional_flags=-std=c++11}
| +if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
|      return -1
|  }
|  
| -if ![runto_main] then {
| +if {![runto_main]} {
| +    untested "failed to run to main"

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Fri, 13 Dec 2019 21:37:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
                   ` (4 preceding siblings ...)
  2019-12-13 21:38 ` Tom Tromey (Code Review)
@ 2019-12-14  9:53 ` Tankut Baris Aktemur (Code Review)
  2020-01-13 18:58   ` Luis Machado
  2019-12-14  9:56 ` Tankut Baris Aktemur (Code Review)
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-12-14  9:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................

testsuite, cp: increase the coverage of testing pass-by-ref arguments

Extend testcases for GDB's infcall of call-by-value functions that
take aggregate values as parameters.  In particular, existing test has
been substantially extended with class definitions whose definitions
of copy constructor, destructor, and move constructor functions are a
combination of

(1) explicitly defined by the user,
(2) defaulted inside the class declaration,
(3) defaulted outside the class declaration,
(4) deleted
(5) not defined in the source.

For each combination, a small and a large class is generated as well
as a derived class and a container class.  Additionally, the following
manually-written cases are provided:

- a dynamic class (i.e. class with a virtual method)
- classes that contain an array field
- a class whose copy ctor is inlined
- a class whose destructor is deleted
- classes with multiple copy and/or move ctors

Test cases check whether GDB makes the right decision to pass an
object by value or implicitly by reference, whether really a copy of
the argument is passed, and whether the copy constructor and
destructor of the clone of the argument are invoked properly.

The input program pass-by-ref.cc is generated in the test's output
directory.  The input program pass-by-ref-2.cc is manually-written.

Tests have been verified on the X86_64 architecture with
GCC 7.4.0, 8.2.0, and 9.2.1.

gdb/testsuite/ChangeLog:
2019-11-07  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.cp/pass-by-ref.cc: Delete.  Generated in the output
	directory instead.
	* gdb.cp/pass-by-ref.exp: Extend with more cases.
	* gdb.cp/pass-by-ref-2.cc: New file.
	* gdb.cp/pass-by-ref-2.exp: New file.

Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
---
A gdb/testsuite/gdb.cp/pass-by-ref-2.cc
A gdb/testsuite/gdb.cp/pass-by-ref-2.exp
D gdb/testsuite/gdb.cp/pass-by-ref.cc
M gdb/testsuite/gdb.cp/pass-by-ref.exp
4 files changed, 791 insertions(+), 86 deletions(-)



diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.cc b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
new file mode 100644
index 0000000..1cd5a16
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
@@ -0,0 +1,295 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+class ByVal {
+public:
+  ByVal (void);
+
+  int x;
+};
+
+ByVal::ByVal (void)
+{
+  x = 2;
+}
+
+class ByRef {
+public:
+  ByRef (void);
+
+  ByRef (const ByRef &rhs);
+
+  int x;
+};
+
+ByRef::ByRef (void)
+{
+  x = 2;
+}
+
+ByRef::ByRef (const ByRef &rhs)
+{
+  x = 3; /* ByRef-cctor */
+}
+
+class ArrayContainerByVal {
+public:
+  ByVal items[2];
+};
+
+int
+cbvArrayContainerByVal (ArrayContainerByVal arg)
+{
+  arg.items[0].x += 4;  // intentionally modify
+  return arg.items[0].x;
+}
+
+class ArrayContainerByRef {
+public:
+  ByRef items[2];
+};
+
+int
+cbvArrayContainerByRef (ArrayContainerByRef arg)
+{
+  arg.items[0].x += 4;  // intentionally modify
+  return arg.items[0].x;
+}
+
+class DynamicBase {
+public:
+  DynamicBase (void);
+
+  virtual int get (void);
+
+  int x;
+};
+
+DynamicBase::DynamicBase (void)
+{
+  x = 2;
+}
+
+int
+DynamicBase::get (void)
+{
+  return 42;
+}
+
+class Dynamic : public DynamicBase {
+public:
+  virtual int get (void);
+};
+
+int
+Dynamic::get (void)
+{
+  return 9999;
+}
+
+int
+cbvDynamic (DynamicBase arg)
+{
+  arg.x += 4;  // intentionally modify
+  return arg.x + arg.get ();
+}
+
+class Inlined {
+public:
+  Inlined (void);
+
+  __attribute__((always_inline))
+  Inlined (const Inlined &rhs)
+  {
+    x = 3;
+  }
+
+  int x;
+};
+
+Inlined::Inlined (void)
+{
+  x = 2;
+}
+
+int
+cbvInlined (Inlined arg)
+{
+  arg.x += 4;  // intentionally modify
+  return arg.x;
+}
+
+class DtorDel {
+public:
+  DtorDel (void);
+
+  ~DtorDel (void) = delete;
+
+  int x;
+};
+
+DtorDel::DtorDel (void)
+{
+  x = 2;
+}
+
+int
+cbvDtorDel (DtorDel arg)
+{
+  // Calling this method should be rejected
+  return arg.x;
+}
+
+class FourCCtor {
+public:
+  FourCCtor (void);
+
+  FourCCtor (FourCCtor &rhs);
+  FourCCtor (const FourCCtor &rhs);
+  FourCCtor (volatile FourCCtor &rhs);
+  FourCCtor (const volatile FourCCtor &rhs);
+
+  int x;
+};
+
+FourCCtor::FourCCtor (void)
+{
+  x = 2;
+}
+
+FourCCtor::FourCCtor (FourCCtor &rhs)
+{
+  x = 3;
+}
+
+FourCCtor::FourCCtor (const FourCCtor &rhs)
+{
+  x = 4;
+}
+
+FourCCtor::FourCCtor (volatile FourCCtor &rhs)
+{
+  x = 5;
+}
+
+FourCCtor::FourCCtor (const volatile FourCCtor &rhs)
+{
+  x = 6;
+}
+
+int
+cbvFourCCtor (FourCCtor arg)
+{
+  arg.x += 10;  // intentionally modify
+  return arg.x;
+}
+
+class TwoMCtor {
+public:
+  TwoMCtor (void);
+
+  /* Even though one move ctor is defaulted, the other
+     is explicit.  */
+  TwoMCtor (const TwoMCtor &&rhs);
+  TwoMCtor (TwoMCtor &&rhs) = default;
+
+  int x;
+};
+
+TwoMCtor::TwoMCtor (void)
+{
+  x = 2;
+}
+
+TwoMCtor::TwoMCtor (const TwoMCtor &&rhs)
+{
+  x = 3;
+}
+
+int
+cbvTwoMCtor (TwoMCtor arg)
+{
+  arg.x += 10;  // intentionally modify
+  return arg.x;
+}
+
+class TwoMCtorAndCCtor {
+public:
+  TwoMCtorAndCCtor (void);
+
+  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &rhs) = default;
+
+  /* Even though one move ctor is defaulted, the other
+     is explicit.  This makes the type pass-by-ref.  */
+  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs);
+  TwoMCtorAndCCtor (TwoMCtorAndCCtor &&rhs) = default;
+
+  int x;
+};
+
+TwoMCtorAndCCtor::TwoMCtorAndCCtor (void)
+{
+  x = 2;
+}
+
+TwoMCtorAndCCtor::TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs)
+{
+  x = 4;
+}
+
+int
+cbvTwoMCtorAndCCtor (TwoMCtorAndCCtor arg)
+{
+  arg.x += 10;  // intentionally modify
+  return arg.x;
+}
+
+ArrayContainerByVal arrayContainerByVal;
+ArrayContainerByRef arrayContainerByRef;
+Dynamic dynamic;
+Inlined inlined;
+// Cannot stack-allocate DtorDel
+DtorDel *dtorDel;
+FourCCtor fourCctor_c0v0;
+const FourCCtor fourCctor_c1v0;
+volatile FourCCtor fourCctor_c0v1;
+const volatile FourCCtor fourCctor_c1v1;
+TwoMCtor twoMctor;
+TwoMCtorAndCCtor twoMctorAndCctor;
+
+int
+main (void)
+{
+  int v;
+  dtorDel = new DtorDel;
+  /* Explicitly call the cbv function to make sure the compiler
+     will not omit any code in the binary.  */
+  v = cbvArrayContainerByVal (arrayContainerByVal);
+  v = cbvArrayContainerByRef (arrayContainerByRef);
+  v = cbvDynamic (dynamic);
+  v = cbvInlined (inlined);
+  v = cbvFourCCtor (fourCctor_c0v0);
+  v = cbvFourCCtor (fourCctor_c1v0);
+  v = cbvFourCCtor (fourCctor_c0v1);
+  v = cbvFourCCtor (fourCctor_c1v1);
+  /* v = cbvTwoMCtor (twoMctor); */ // This is illegal, cctor is deleted
+  v = cbvTwoMCtorAndCCtor (twoMctorAndCctor);
+
+  /* stop here */
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.exp b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
new file mode 100644
index 0000000..7cce886
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
@@ -0,0 +1,114 @@
+# Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that GDB can call C++ functions whose parameters have
+# object type, and are either passed by value or implicitly by reference.
+#
+# This is a companion test to pass-by-ref.exp.  In this test, the input
+# is manually-written.  In pass-by-ref.exp, the test input is generated.
+#
+# We include tests for classes that
+# - contain arrays as fields,
+# - are dynamic (i.e. have virtual methods)
+# - have inlined copy ctor
+# - have deleted destructor
+
+if {[skip_cplus_tests]} {
+    untested "c++ test skipped"
+    continue
+}
+
+standard_testfile .cc
+
+set options {debug c++ additional_flags=-std=c++11}
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
+    return -1
+}
+
+if {![runto_main]} {
+    untested "failed to run to main"
+    return -1
+}
+
+set bp_location [gdb_get_line_number "stop here"]
+gdb_breakpoint $bp_location
+gdb_continue_to_breakpoint "end of main" ".*return .*;"
+
+gdb_test "print cbvArrayContainerByVal (arrayContainerByVal)" "6" \
+    "call cbvArrayContainerByVal"
+gdb_test "print arrayContainerByVal.items\[0\].x" "2" \
+    "cbv argument 'arrayContainerByVal' should not change"
+
+gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" "7" \
+    "call cbvArrayContainerByRef"
+gdb_test "print arrayContainerByRef.items\[0\].x" "2" \
+    "cbv argument 'arrayContainerByRef' should not change"
+
+gdb_test "print cbvDynamic (dynamic)" "48" \
+    "call cbvDynamic"
+gdb_test "print dynamic.x" "2" \
+    "cbv argument 'dynamic' should not change"
+
+set sig "\"Inlined\:\:Inlined\\(.*Inlined const\&\\)\""
+gdb_test "print cbvInlined (inlined)" \
+    "expression cannot be evaluated .* \\(maybe inlined\\?\\)"
+
+gdb_test "print cbvDtorDel (*dtorDel)" \
+    ".* cannot be evaluated .* 'DtorDel' is not destructible" \
+    "type not destructible"
+
+# Test that GDB calls the correct copy ctor
+gdb_test "print cbvFourCCtor (fourCctor_c0v0)" "13" \
+    "call cbvFourCCtor (c0v0)"
+gdb_test "print fourCctor_c0v0.x" "2" \
+    "cbv argument 'twoCctor_c0v0' should not change"
+
+gdb_test "print cbvFourCCtor (fourCctor_c1v0)" "14" \
+    "call cbvFourCCtor (c1v0)"
+gdb_test "print fourCctor_c1v0.x" "2" \
+    "cbv argument 'twoCctor_c1v0' should not change"
+
+gdb_test "print cbvFourCCtor (fourCctor_c0v1)" "15" \
+    "call cbvFourCCtor (c0v1)"
+gdb_test "print fourCctor_c0v1.x" "2" \
+    "cbv argument 'twoCctor_c0v1' should not change"
+
+gdb_test "print cbvFourCCtor (fourCctor_c1v1)" "16" \
+    "call cbvFourCCtor (c1v1)"
+gdb_test "print fourCctor_c1v1.x" "2" \
+    "cbv argument 'twoCctor_c1v1' should not change"
+
+gdb_test "print cbvTwoMCtor (twoMctor)" \
+    ".* cannot be evaluated .* 'TwoMCtor' is not copy constructible" \
+    "copy ctor is implicitly deleted"
+
+gdb_test "print cbvTwoMCtorAndCCtor (twoMctorAndCctor)" "12" \
+    "call cbvTwoMCtorAndCCtor"
+gdb_test "print twoMctorAndCctor.x" "2" \
+    "cbv argument 'twoMctorAndCtor' should not change"
+
+# Test that we get a breakpoint from the cctor during infcall and
+# we can examine arguments.  This is a test that the dummy frame
+# of the copy constructor is set up correctly by the infcall mechanism.
+set bp_location [gdb_get_line_number "ByRef-cctor"]
+gdb_breakpoint $bp_location
+gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" \
+    ".*The program being debugged stopped.*" \
+    "call cbvArrayContainerByRef with BP"
+gdb_test "backtrace" [multi_line \
+    "#0  ByRef\:\:ByRef .* at .*$srcfile:$bp_location" \
+    "#1  .* ArrayContainerByRef::ArrayContainerByRef .*" \
+    "#2  <function called from gdb>" \
+    "#3  main.*"]
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.cc b/gdb/testsuite/gdb.cp/pass-by-ref.cc
deleted file mode 100644
index bbe450a..0000000
--- a/gdb/testsuite/gdb.cp/pass-by-ref.cc
+++ /dev/null
@@ -1,79 +0,0 @@
-/* This testcase is part of GDB, the GNU debugger.
-
-   Copyright 2007-2019 Free Software Foundation, Inc.
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-class Obj {
-public:
-  Obj ();
-  Obj (const Obj &);
-  ~Obj ();
-  int var[2];
-};
-
-int foo (Obj arg)
-{
-  return arg.var[0] + arg.var[1];
-}
-
-Obj::Obj ()
-{
-  var[0] = 1;
-  var[1] = 2;
-}
-
-Obj::Obj (const Obj &obj)
-{
-  var[0] = obj.var[0];
-  var[1] = obj.var[1];
-}
-
-Obj::~Obj ()
-{
-
-}
-
-struct Derived : public Obj
-{
-  int other;
-};
-
-int blap (Derived arg)
-{
-  return foo (arg);
-}
-
-struct Container
-{
-  Obj obj;
-};
-
-int blip (Container arg)
-{
-  return foo (arg.obj);
-}
-
-Obj global_obj;
-Derived global_derived;
-Container global_container;
-
-int
-main ()
-{
-  int bar = foo (global_obj);
-  blap (global_derived);
-  blip (global_container);
-  return bar;
-}
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.exp b/gdb/testsuite/gdb.cp/pass-by-ref.exp
index 94dd345..f44be77 100644
--- a/gdb/testsuite/gdb.cp/pass-by-ref.exp
+++ b/gdb/testsuite/gdb.cp/pass-by-ref.exp
@@ -14,20 +14,395 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # Check that GDB can call C++ functions whose parameters have
-# object type, but are passed by reference.
+# object type, and are either passed by value or implicitly by reference.
+#
+# Suppose F is a function that has a call-by-value parameter whose
+# type is class C.  When calling F with an argument A, a copy of A should
+# be created and passed to F.  If C is a trivially-copyable type, A can
+# be copied by a straightforward memory copy.  However, roughly speaking,
+# if C has a user-defined copy constructor and/or a user-defined
+# destructor, the copy ctor should be used to initialize the copy of A
+# before calling F, and a reference to that copy is passed to F.  After
+# the function returns, the destructor should be called to destruct the
+# copy.  In this case, C is said to be a 'pass-by-reference' type.
+# Determining whether C is pass-by-ref depends on
+# how the copy ctor, destructor, and the move ctor of C are defined.
+# First of all, C is not copy constructible if its copy constructor is
+# explicitly or implicitly deleted.  In this case, it would be illegal
+# to pass values of type C to a function.  C is pass-by-value, if all of
+# its copy ctor, dtor, and move ctor are trivially defined.
+# Otherwise, it is pass-by-ref.
+#
+# To cover the many possible combinations, this test generates classes
+# that contain three special functions:
+#   (1) a copy constructor,
+#   (2) a destructor, and
+#   (3) a move constructor.
+# A special function is in one of the following states:
+#  * explicit: The function is explicitly defined by the user.
+#  * defaultedIn: The function is defaulted inside the class decl,
+#      using the 'default' keyword.
+#  * defaultedOut: The function is declared inside the class decl,
+#      and defaulted outside using the 'default' keyword.
+#  * deleted: The function is explicitly deleted by the user,
+#      using the 'delete' keyword.
+#  * absent: The function is not declared by the user (i.e. it does not
+#      exist in the source.  The compiler generates (or deletes) the
+#      definition in this case.
+#
+# The C++ ABI decides if a class is pass-by-value or pass-by-ref
+# (i.e.  trivially copyable or not) first at the language level, based
+# on the state of the special functions.  Then, at the target level, a
+# class may be determined to be pass-by-ref because of its size
+# (e.g.  if it is too large to fit on registers).  For this reason, this
+# test generates both a small and a large version for the same
+# combination of special function states.
+#
+# A class is not trivially-copyable if a base class or a field is not
+# trivially-copyable, even though the class definition itself seems
+# trivial.  To test these cases, we also generate derived classes and
+# container classes.
+#
+# The generated code is placed in the test output directory.
+#
+# The companion test file pass-by-ref-2.exp also contains
+# manually-written cases.
 
-if { [skip_cplus_tests] } { continue }
+if {[skip_cplus_tests]} {
+    untested "c++ test skipped"
+    continue
+}
 
+# The program source is generated in the output directory.
+# We use standard_testfile here to set convenience variables.
 standard_testfile .cc
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+# Some constant values used when generating the source
+
+set SMALL    2
+set LARGE    150
+set ORIGINAL 2
+set CUSTOM   3
+set ADDED    4
+set TRACE    5
+
+
+# Return 1 if the class whose special function states are STATES
+# is copyable.  Otherwise return 0.
+
+proc is_copy_constructible { states } {
+    set cctor [lindex $states 0]
+    set dtor  [lindex $states 1]
+    set mctor [lindex $states 2]
+
+    if {$cctor == "deleted" || ($cctor == "absent" && $mctor != "absent")} {
+	return 0
+    }
+    return 1
+}
+
+# Generate a declaration and an out-of-class definition for a function
+# with the provided signature.  The STATE should be one of the following:
+# - explicit, defaultedIn, defaultedOut, deleted, absent
+
+proc generate_member_function { classname signature length state } {
+    set declaration ""
+    set definition ""
+
+    global CUSTOM
+    global TRACE
+
+    switch $state {
+	explicit {
+	    set declaration "$signature;\n"
+	    set definition "$classname\:\:$signature
+                            {
+                              data\[0\] = $CUSTOM;
+                              data\[[expr $length - 1]\] = $CUSTOM;
+                              tracer = $TRACE;
+                            }\n"
+	}
+	defaultedIn {
+	    set declaration "$signature = default;\n"
+	}
+	defaultedOut {
+	    set declaration "$signature;\n"
+	    set definition "$classname\:\:$signature = default;\n"
+	}
+	deleted {
+	    set declaration "$signature = delete;\n"
+	}
+	default {
+	    # function is not user-defined in this case
+	}
+    }
+
+    return [list $declaration $definition]
+}
+
+# Generate a C++ class with the given CLASSNAME and LENGTH-many
+# integer elements.  The STATES is an array of 3 items
+# containing the desired state of the special functions
+# in this order:
+# copy constructor, destructor, move constructor
+
+proc generate_class { classname length states } {
+    set declarations ""
+    set definitions ""
+    set classname "${classname}_[join $states _]"
+
+    for {set i 0} {$i < [llength $states]} {incr i} {
+	set sig ""
+	switch $i {
+	    0 {set sig "$classname (const $classname \&rhs)"}
+	    1 {set sig "\~$classname (void)"}
+	    2 {set sig "$classname ($classname \&\&rhs)"}
+	}
+
+	set state [lindex $states $i]
+	set code [generate_member_function $classname $sig $length $state]
+	append declarations [lindex $code 0]
+	append definitions [lindex $code 1]
+    }
+
+    global ORIGINAL
+
+    return "
+    /*** C++ class $classname ***/
+    class ${classname} {
+    public:
+        $classname (void);
+        $declarations
+
+        int data\[$length\];
+    };
+
+    $classname\:\:$classname (void)
+    {
+        data\[0\] = $ORIGINAL;
+        data\[[expr $length - 1]\] = $ORIGINAL;
+    }
+
+    $definitions
+
+    $classname ${classname}_var; /* global var */
+
+    template int cbv<$classname> ($classname arg);"
+}
+
+# Generate a small C++ class
+
+proc generate_small_class { states } {
+    global SMALL
+    return [generate_class Small $SMALL $states];
+}
+
+# Generate a large C++ class
+
+proc generate_large_class { states } {
+    global LARGE
+    return [generate_class Large $LARGE $states];
+}
+
+# Generate a class that derives from a small class
+
+proc generate_derived_class { states } {
+    set base "Small_[join $states _]"
+    set classname "Derived_[join $states _]"
+
+    return "
+    /*** Class derived from $base ***/
+    class $classname : public $base {
+    public:
+    };
+
+    $classname ${classname}_var; /* global var */
+
+    template int cbv<$classname> ($classname arg);"
+}
+
+# Generate a class that contains a small class item
+
+proc generate_container_class { states } {
+    set contained "Small_[join $states _]"
+    set classname "Container_[join $states _]"
+
+    return "
+    /*** Class that contains $contained ***/
+    class $classname {
+    public:
+        $contained item;
+    };
+
+    $classname ${classname}_var; /* global var */
+
+    template int cbv_container<$classname> ($classname arg);"
+}
+
+# Generate useful statements that use a class in the debugee program
+
+proc generate_stmts { classprefix states {cbvfun "cbv"}} {
+    set classname "${classprefix}_[join $states _]"
+
+    # Having an explicit call to the cbv function in the debugee program
+    # ensures that the compiler will emit necessary function in the binary.
+    if {[is_copy_constructible $states]} {
+	set cbvcall "$cbvfun<$classname> (${classname}_var);\n"
+    } else {
+	set cbvcall ""
+    }
+
+    return "$cbvcall"
+}
+
+# Generate the complete debugee program
+
+proc generate_program { classes stmts } {
+    global ADDED
+
+    return "
+    /*** THIS FILE IS GENERATED BY THE TEST.  ***/
+
+    static int tracer = 0;
+
+    /* The call-by-value function.  */
+    template <class T>
+    int
+    cbv (T arg)
+    {
+      arg.data\[0\] += $ADDED; // intentionally modify the arg
+      return arg.data\[0\];
+    }
+
+    template <class T>
+    int
+    cbv_container (T arg)
+    {
+      arg.item.data\[0\] += $ADDED;  // intentionally modify
+      return arg.item.data\[0\];
+    }
+
+    $classes
+
+    int
+    main (void)
+    {
+      $stmts
+
+      /* stop here */
+
+      return 0;
+    }"
+}
+
+# Compute all the combinations of special function states.
+# We do not contain the 'deleted' state for the destructor,
+# because it is illegal to have stack-allocated objects
+# whose destructor have been deleted.  This case is covered
+# in pass-by-ref-2 via heap-allocated objects.
+
+set options_nodelete [list absent explicit defaultedIn defaultedOut]
+set options [concat $options_nodelete {deleted}]
+set all_combinations {}
+
+foreach cctor $options {
+    foreach dtor $options_nodelete {
+	foreach mctor $options {
+	    lappend all_combinations [list $cctor $dtor $mctor]
+	}
+    }
+}
+
+# Generate the classes.
+
+set classes ""
+set stmts ""
+
+foreach state $all_combinations {
+    append classes [generate_small_class $state]
+    append stmts [generate_stmts "Small" $state]
+
+    append classes [generate_large_class $state]
+    append stmts [generate_stmts "Large" $state]
+
+    append classes [generate_derived_class $state]
+    append stmts [generate_stmts "Derived" $state]
+
+    append classes [generate_container_class $state]
+    append stmts [generate_stmts "Container" $state "cbv_container"]
+}
+
+# Generate the program code and compile
+set program [generate_program $classes $stmts]
+set srcfile [standard_output_file ${srcfile}]
+gdb_produce_source $srcfile $program
+
+set options {debug c++ additional_flags=-std=c++11}
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
     return -1
 }
 
-if ![runto_main] then {
+if {![runto_main]} {
+    untested "failed to run to main"
     return -1
 }
 
-gdb_test "print foo (global_obj)" " = 3" "call function in obj"
-gdb_test "print blap (global_derived)" " = 3" "call function in derived"
-gdb_test "print blip (global_container)" " = 3" "call function in container"
+set bp_location [gdb_get_line_number "stop here"]
+gdb_breakpoint $bp_location
+gdb_continue_to_breakpoint "end of main" ".*return .*;"
+
+# Do the checks for a given class whose name is prefixed with PREFIX,
+# and whose special functions have the states given in STATES.
+# The name of the call-by-value function and the expression to access
+# the data field can be specified explicitly if the default values
+# do not work.
+
+proc test_for_class { prefix states cbvfun data_field length} {
+    set name "${prefix}_[join $states _]"
+
+    set cctor [lindex $states 0]
+    set dtor  [lindex $states 1]
+    set mctor [lindex $states 2]
+
+    global ORIGINAL
+    global CUSTOM
+    global ADDED
+    global TRACE
+
+    with_test_prefix $name {
+	if {[is_copy_constructible $states]} {
+	    set expected [expr {$ORIGINAL + $ADDED}]
+	    if {$cctor == "explicit"} {
+		set expected [expr {$CUSTOM + $ADDED}]
+	    }
+	    if {$dtor == "explicit"} {
+		gdb_test "print tracer = 0" " = 0" "reset the tracer"
+	    }
+	    gdb_test "print ${cbvfun}<$name> (${name}_var)" " = $expected" \
+		"call '$cbvfun'"
+	    gdb_test "print ${name}_var.${data_field}\[0\]" " = $ORIGINAL" \
+		"cbv argument should not change (item 0)"
+	    if {$length > 1} {
+		set last_index [expr $length - 1]
+		gdb_test "print ${name}_var.${data_field}\[$last_index\]" \
+		    " = $ORIGINAL" \
+		    "cbv argument should not change (item $last_index)"
+	    }
+	    if {$dtor == "explicit"} {
+		gdb_test "print tracer" " = $TRACE" \
+		    "destructor should be called"
+	    }
+	} else {
+	    gdb_test "print ${cbvfun}<$name> (${name}_var)" \
+		".* cannot be evaluated .* '${name}' is not copy constructible" \
+		"calling '$cbvfun' should be refused"
+	}
+    }
+}
+
+foreach state $all_combinations {
+    test_for_class "Small"     $state "cbv"           "data"      $SMALL
+    test_for_class "Large"     $state "cbv"           "data"      $LARGE
+    test_for_class "Derived"   $state "cbv"           "data"      1
+    test_for_class "Container" $state "cbv_container" "item.data" 1
+}

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 3
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
                   ` (5 preceding siblings ...)
  2019-12-14  9:53 ` [review v3] " Tankut Baris Aktemur (Code Review)
@ 2019-12-14  9:56 ` Tankut Baris Aktemur (Code Review)
  2019-12-20  7:29 ` Tankut Baris Aktemur (Code Review)
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-12-14  9:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Tankut Baris Aktemur has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................


Patch Set 3:

(1 comment)

| --- gdb/testsuite/gdb.cp/pass-by-ref.exp
| +++ gdb/testsuite/gdb.cp/pass-by-ref.exp
| @@ -24,4 +343,18 @@ foreach state $all_combinations {
| +    append stmts [generate_stmts "Derived" $state]
| +
| +    append classes [generate_container_class $state]
| +    append stmts [generate_stmts "Container" $state "cbv_container"]
| +}
| +
| +# Generate the program code and compile
| +set program [generate_program $classes $stmts]
| +set testfolder [file dirname $testfile]
| +gdb_produce_source ${srcdir}/${subdir}/$srcfile $program

PS2, Line 352:

I updated the patch to delete the source from the repository and to
generate it in the test's output directory instead.  Does this look
OK?

| +
| +set options {debug c++ additional_flags=-std=c++11}
| +if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
|      return -1
|  }
|  
| -if ![runto_main] then {
| +if {![runto_main]} {
| +    untested "failed to run to main"

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 3
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Sat, 14 Dec 2019 09:56:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
                   ` (6 preceding siblings ...)
  2019-12-14  9:56 ` Tankut Baris Aktemur (Code Review)
@ 2019-12-20  7:29 ` Tankut Baris Aktemur (Code Review)
  2019-12-20 15:54 ` Tom Tromey (Code Review)
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-12-20  7:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Tankut Baris Aktemur has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................


Patch Set 3:

> I updated the patch to delete the source from the repository and
> to generate it in the test's output directory instead.
> Does this look OK?

Any chance for a review to close the series?


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 3
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Fri, 20 Dec 2019 07:29:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
                   ` (7 preceding siblings ...)
  2019-12-20  7:29 ` Tankut Baris Aktemur (Code Review)
@ 2019-12-20 15:54 ` Tom Tromey (Code Review)
  2019-12-20 16:47 ` [pushed] " Sourceware to Gerrit sync (Code Review)
  2019-12-20 16:47 ` Sourceware to Gerrit sync (Code Review)
  10 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey (Code Review) @ 2019-12-20 15:54 UTC (permalink / raw)
  To: Tankut Baris Aktemur, gdb-patches

Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................


Patch Set 3: Code-Review+2

Thank you for doing this.  This is ok.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 3
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Fri, 20 Dec 2019 15:54:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [pushed] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
                   ` (8 preceding siblings ...)
  2019-12-20 15:54 ` Tom Tromey (Code Review)
@ 2019-12-20 16:47 ` Sourceware to Gerrit sync (Code Review)
  2019-12-20 16:47 ` Sourceware to Gerrit sync (Code Review)
  10 siblings, 0 replies; 25+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-12-20 16:47 UTC (permalink / raw)
  To: Tankut Baris Aktemur, gdb-patches; +Cc: Tom Tromey

Sourceware to Gerrit sync has submitted this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................

testsuite, cp: increase the coverage of testing pass-by-ref arguments

Extend testcases for GDB's infcall of call-by-value functions that
take aggregate values as parameters.  In particular, existing test has
been substantially extended with class definitions whose definitions
of copy constructor, destructor, and move constructor functions are a
combination of

(1) explicitly defined by the user,
(2) defaulted inside the class declaration,
(3) defaulted outside the class declaration,
(4) deleted
(5) not defined in the source.

For each combination, a small and a large class is generated as well
as a derived class and a container class.  Additionally, the following
manually-written cases are provided:

- a dynamic class (i.e. class with a virtual method)
- classes that contain an array field
- a class whose copy ctor is inlined
- a class whose destructor is deleted
- classes with multiple copy and/or move ctors

Test cases check whether GDB makes the right decision to pass an
object by value or implicitly by reference, whether really a copy of
the argument is passed, and whether the copy constructor and
destructor of the clone of the argument are invoked properly.

The input program pass-by-ref.cc is generated in the test's output
directory.  The input program pass-by-ref-2.cc is manually-written.

Tests have been verified on the X86_64 architecture with
GCC 7.4.0, 8.2.0, and 9.2.1.

gdb/testsuite/ChangeLog:
2019-12-20  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.cp/pass-by-ref.cc: Delete.  Generated in the output
	directory instead.
	* gdb.cp/pass-by-ref.exp: Extend with more cases.
	* gdb.cp/pass-by-ref-2.cc: New file.
	* gdb.cp/pass-by-ref-2.exp: New file.

Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
---
M gdb/testsuite/ChangeLog
A gdb/testsuite/gdb.cp/pass-by-ref-2.cc
A gdb/testsuite/gdb.cp/pass-by-ref-2.exp
D gdb/testsuite/gdb.cp/pass-by-ref.cc
M gdb/testsuite/gdb.cp/pass-by-ref.exp
5 files changed, 799 insertions(+), 86 deletions(-)


diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 4476549..c93edc6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2019-12-20  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+
+	* gdb.cp/pass-by-ref.cc: Delete.  Generated in the output
+	directory instead.
+	* gdb.cp/pass-by-ref.exp: Extend with more cases.
+	* gdb.cp/pass-by-ref-2.cc: New file.
+	* gdb.cp/pass-by-ref-2.exp: New file.
+
 2019-12-20  Tom Tromey  <tom@tromey.com>
 
 	* gdb.tui/list-before.exp: New file.
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.cc b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
new file mode 100644
index 0000000..1cd5a16
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
@@ -0,0 +1,295 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+class ByVal {
+public:
+  ByVal (void);
+
+  int x;
+};
+
+ByVal::ByVal (void)
+{
+  x = 2;
+}
+
+class ByRef {
+public:
+  ByRef (void);
+
+  ByRef (const ByRef &rhs);
+
+  int x;
+};
+
+ByRef::ByRef (void)
+{
+  x = 2;
+}
+
+ByRef::ByRef (const ByRef &rhs)
+{
+  x = 3; /* ByRef-cctor */
+}
+
+class ArrayContainerByVal {
+public:
+  ByVal items[2];
+};
+
+int
+cbvArrayContainerByVal (ArrayContainerByVal arg)
+{
+  arg.items[0].x += 4;  // intentionally modify
+  return arg.items[0].x;
+}
+
+class ArrayContainerByRef {
+public:
+  ByRef items[2];
+};
+
+int
+cbvArrayContainerByRef (ArrayContainerByRef arg)
+{
+  arg.items[0].x += 4;  // intentionally modify
+  return arg.items[0].x;
+}
+
+class DynamicBase {
+public:
+  DynamicBase (void);
+
+  virtual int get (void);
+
+  int x;
+};
+
+DynamicBase::DynamicBase (void)
+{
+  x = 2;
+}
+
+int
+DynamicBase::get (void)
+{
+  return 42;
+}
+
+class Dynamic : public DynamicBase {
+public:
+  virtual int get (void);
+};
+
+int
+Dynamic::get (void)
+{
+  return 9999;
+}
+
+int
+cbvDynamic (DynamicBase arg)
+{
+  arg.x += 4;  // intentionally modify
+  return arg.x + arg.get ();
+}
+
+class Inlined {
+public:
+  Inlined (void);
+
+  __attribute__((always_inline))
+  Inlined (const Inlined &rhs)
+  {
+    x = 3;
+  }
+
+  int x;
+};
+
+Inlined::Inlined (void)
+{
+  x = 2;
+}
+
+int
+cbvInlined (Inlined arg)
+{
+  arg.x += 4;  // intentionally modify
+  return arg.x;
+}
+
+class DtorDel {
+public:
+  DtorDel (void);
+
+  ~DtorDel (void) = delete;
+
+  int x;
+};
+
+DtorDel::DtorDel (void)
+{
+  x = 2;
+}
+
+int
+cbvDtorDel (DtorDel arg)
+{
+  // Calling this method should be rejected
+  return arg.x;
+}
+
+class FourCCtor {
+public:
+  FourCCtor (void);
+
+  FourCCtor (FourCCtor &rhs);
+  FourCCtor (const FourCCtor &rhs);
+  FourCCtor (volatile FourCCtor &rhs);
+  FourCCtor (const volatile FourCCtor &rhs);
+
+  int x;
+};
+
+FourCCtor::FourCCtor (void)
+{
+  x = 2;
+}
+
+FourCCtor::FourCCtor (FourCCtor &rhs)
+{
+  x = 3;
+}
+
+FourCCtor::FourCCtor (const FourCCtor &rhs)
+{
+  x = 4;
+}
+
+FourCCtor::FourCCtor (volatile FourCCtor &rhs)
+{
+  x = 5;
+}
+
+FourCCtor::FourCCtor (const volatile FourCCtor &rhs)
+{
+  x = 6;
+}
+
+int
+cbvFourCCtor (FourCCtor arg)
+{
+  arg.x += 10;  // intentionally modify
+  return arg.x;
+}
+
+class TwoMCtor {
+public:
+  TwoMCtor (void);
+
+  /* Even though one move ctor is defaulted, the other
+     is explicit.  */
+  TwoMCtor (const TwoMCtor &&rhs);
+  TwoMCtor (TwoMCtor &&rhs) = default;
+
+  int x;
+};
+
+TwoMCtor::TwoMCtor (void)
+{
+  x = 2;
+}
+
+TwoMCtor::TwoMCtor (const TwoMCtor &&rhs)
+{
+  x = 3;
+}
+
+int
+cbvTwoMCtor (TwoMCtor arg)
+{
+  arg.x += 10;  // intentionally modify
+  return arg.x;
+}
+
+class TwoMCtorAndCCtor {
+public:
+  TwoMCtorAndCCtor (void);
+
+  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &rhs) = default;
+
+  /* Even though one move ctor is defaulted, the other
+     is explicit.  This makes the type pass-by-ref.  */
+  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs);
+  TwoMCtorAndCCtor (TwoMCtorAndCCtor &&rhs) = default;
+
+  int x;
+};
+
+TwoMCtorAndCCtor::TwoMCtorAndCCtor (void)
+{
+  x = 2;
+}
+
+TwoMCtorAndCCtor::TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs)
+{
+  x = 4;
+}
+
+int
+cbvTwoMCtorAndCCtor (TwoMCtorAndCCtor arg)
+{
+  arg.x += 10;  // intentionally modify
+  return arg.x;
+}
+
+ArrayContainerByVal arrayContainerByVal;
+ArrayContainerByRef arrayContainerByRef;
+Dynamic dynamic;
+Inlined inlined;
+// Cannot stack-allocate DtorDel
+DtorDel *dtorDel;
+FourCCtor fourCctor_c0v0;
+const FourCCtor fourCctor_c1v0;
+volatile FourCCtor fourCctor_c0v1;
+const volatile FourCCtor fourCctor_c1v1;
+TwoMCtor twoMctor;
+TwoMCtorAndCCtor twoMctorAndCctor;
+
+int
+main (void)
+{
+  int v;
+  dtorDel = new DtorDel;
+  /* Explicitly call the cbv function to make sure the compiler
+     will not omit any code in the binary.  */
+  v = cbvArrayContainerByVal (arrayContainerByVal);
+  v = cbvArrayContainerByRef (arrayContainerByRef);
+  v = cbvDynamic (dynamic);
+  v = cbvInlined (inlined);
+  v = cbvFourCCtor (fourCctor_c0v0);
+  v = cbvFourCCtor (fourCctor_c1v0);
+  v = cbvFourCCtor (fourCctor_c0v1);
+  v = cbvFourCCtor (fourCctor_c1v1);
+  /* v = cbvTwoMCtor (twoMctor); */ // This is illegal, cctor is deleted
+  v = cbvTwoMCtorAndCCtor (twoMctorAndCctor);
+
+  /* stop here */
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.exp b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
new file mode 100644
index 0000000..7cce886
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
@@ -0,0 +1,114 @@
+# Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that GDB can call C++ functions whose parameters have
+# object type, and are either passed by value or implicitly by reference.
+#
+# This is a companion test to pass-by-ref.exp.  In this test, the input
+# is manually-written.  In pass-by-ref.exp, the test input is generated.
+#
+# We include tests for classes that
+# - contain arrays as fields,
+# - are dynamic (i.e. have virtual methods)
+# - have inlined copy ctor
+# - have deleted destructor
+
+if {[skip_cplus_tests]} {
+    untested "c++ test skipped"
+    continue
+}
+
+standard_testfile .cc
+
+set options {debug c++ additional_flags=-std=c++11}
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
+    return -1
+}
+
+if {![runto_main]} {
+    untested "failed to run to main"
+    return -1
+}
+
+set bp_location [gdb_get_line_number "stop here"]
+gdb_breakpoint $bp_location
+gdb_continue_to_breakpoint "end of main" ".*return .*;"
+
+gdb_test "print cbvArrayContainerByVal (arrayContainerByVal)" "6" \
+    "call cbvArrayContainerByVal"
+gdb_test "print arrayContainerByVal.items\[0\].x" "2" \
+    "cbv argument 'arrayContainerByVal' should not change"
+
+gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" "7" \
+    "call cbvArrayContainerByRef"
+gdb_test "print arrayContainerByRef.items\[0\].x" "2" \
+    "cbv argument 'arrayContainerByRef' should not change"
+
+gdb_test "print cbvDynamic (dynamic)" "48" \
+    "call cbvDynamic"
+gdb_test "print dynamic.x" "2" \
+    "cbv argument 'dynamic' should not change"
+
+set sig "\"Inlined\:\:Inlined\\(.*Inlined const\&\\)\""
+gdb_test "print cbvInlined (inlined)" \
+    "expression cannot be evaluated .* \\(maybe inlined\\?\\)"
+
+gdb_test "print cbvDtorDel (*dtorDel)" \
+    ".* cannot be evaluated .* 'DtorDel' is not destructible" \
+    "type not destructible"
+
+# Test that GDB calls the correct copy ctor
+gdb_test "print cbvFourCCtor (fourCctor_c0v0)" "13" \
+    "call cbvFourCCtor (c0v0)"
+gdb_test "print fourCctor_c0v0.x" "2" \
+    "cbv argument 'twoCctor_c0v0' should not change"
+
+gdb_test "print cbvFourCCtor (fourCctor_c1v0)" "14" \
+    "call cbvFourCCtor (c1v0)"
+gdb_test "print fourCctor_c1v0.x" "2" \
+    "cbv argument 'twoCctor_c1v0' should not change"
+
+gdb_test "print cbvFourCCtor (fourCctor_c0v1)" "15" \
+    "call cbvFourCCtor (c0v1)"
+gdb_test "print fourCctor_c0v1.x" "2" \
+    "cbv argument 'twoCctor_c0v1' should not change"
+
+gdb_test "print cbvFourCCtor (fourCctor_c1v1)" "16" \
+    "call cbvFourCCtor (c1v1)"
+gdb_test "print fourCctor_c1v1.x" "2" \
+    "cbv argument 'twoCctor_c1v1' should not change"
+
+gdb_test "print cbvTwoMCtor (twoMctor)" \
+    ".* cannot be evaluated .* 'TwoMCtor' is not copy constructible" \
+    "copy ctor is implicitly deleted"
+
+gdb_test "print cbvTwoMCtorAndCCtor (twoMctorAndCctor)" "12" \
+    "call cbvTwoMCtorAndCCtor"
+gdb_test "print twoMctorAndCctor.x" "2" \
+    "cbv argument 'twoMctorAndCtor' should not change"
+
+# Test that we get a breakpoint from the cctor during infcall and
+# we can examine arguments.  This is a test that the dummy frame
+# of the copy constructor is set up correctly by the infcall mechanism.
+set bp_location [gdb_get_line_number "ByRef-cctor"]
+gdb_breakpoint $bp_location
+gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" \
+    ".*The program being debugged stopped.*" \
+    "call cbvArrayContainerByRef with BP"
+gdb_test "backtrace" [multi_line \
+    "#0  ByRef\:\:ByRef .* at .*$srcfile:$bp_location" \
+    "#1  .* ArrayContainerByRef::ArrayContainerByRef .*" \
+    "#2  <function called from gdb>" \
+    "#3  main.*"]
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.cc b/gdb/testsuite/gdb.cp/pass-by-ref.cc
deleted file mode 100644
index bbe450a..0000000
--- a/gdb/testsuite/gdb.cp/pass-by-ref.cc
+++ /dev/null
@@ -1,79 +0,0 @@
-/* This testcase is part of GDB, the GNU debugger.
-
-   Copyright 2007-2019 Free Software Foundation, Inc.
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-class Obj {
-public:
-  Obj ();
-  Obj (const Obj &);
-  ~Obj ();
-  int var[2];
-};
-
-int foo (Obj arg)
-{
-  return arg.var[0] + arg.var[1];
-}
-
-Obj::Obj ()
-{
-  var[0] = 1;
-  var[1] = 2;
-}
-
-Obj::Obj (const Obj &obj)
-{
-  var[0] = obj.var[0];
-  var[1] = obj.var[1];
-}
-
-Obj::~Obj ()
-{
-
-}
-
-struct Derived : public Obj
-{
-  int other;
-};
-
-int blap (Derived arg)
-{
-  return foo (arg);
-}
-
-struct Container
-{
-  Obj obj;
-};
-
-int blip (Container arg)
-{
-  return foo (arg.obj);
-}
-
-Obj global_obj;
-Derived global_derived;
-Container global_container;
-
-int
-main ()
-{
-  int bar = foo (global_obj);
-  blap (global_derived);
-  blip (global_container);
-  return bar;
-}
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.exp b/gdb/testsuite/gdb.cp/pass-by-ref.exp
index 94dd345..f44be77 100644
--- a/gdb/testsuite/gdb.cp/pass-by-ref.exp
+++ b/gdb/testsuite/gdb.cp/pass-by-ref.exp
@@ -14,20 +14,395 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # Check that GDB can call C++ functions whose parameters have
-# object type, but are passed by reference.
+# object type, and are either passed by value or implicitly by reference.
+#
+# Suppose F is a function that has a call-by-value parameter whose
+# type is class C.  When calling F with an argument A, a copy of A should
+# be created and passed to F.  If C is a trivially-copyable type, A can
+# be copied by a straightforward memory copy.  However, roughly speaking,
+# if C has a user-defined copy constructor and/or a user-defined
+# destructor, the copy ctor should be used to initialize the copy of A
+# before calling F, and a reference to that copy is passed to F.  After
+# the function returns, the destructor should be called to destruct the
+# copy.  In this case, C is said to be a 'pass-by-reference' type.
+# Determining whether C is pass-by-ref depends on
+# how the copy ctor, destructor, and the move ctor of C are defined.
+# First of all, C is not copy constructible if its copy constructor is
+# explicitly or implicitly deleted.  In this case, it would be illegal
+# to pass values of type C to a function.  C is pass-by-value, if all of
+# its copy ctor, dtor, and move ctor are trivially defined.
+# Otherwise, it is pass-by-ref.
+#
+# To cover the many possible combinations, this test generates classes
+# that contain three special functions:
+#   (1) a copy constructor,
+#   (2) a destructor, and
+#   (3) a move constructor.
+# A special function is in one of the following states:
+#  * explicit: The function is explicitly defined by the user.
+#  * defaultedIn: The function is defaulted inside the class decl,
+#      using the 'default' keyword.
+#  * defaultedOut: The function is declared inside the class decl,
+#      and defaulted outside using the 'default' keyword.
+#  * deleted: The function is explicitly deleted by the user,
+#      using the 'delete' keyword.
+#  * absent: The function is not declared by the user (i.e. it does not
+#      exist in the source.  The compiler generates (or deletes) the
+#      definition in this case.
+#
+# The C++ ABI decides if a class is pass-by-value or pass-by-ref
+# (i.e.  trivially copyable or not) first at the language level, based
+# on the state of the special functions.  Then, at the target level, a
+# class may be determined to be pass-by-ref because of its size
+# (e.g.  if it is too large to fit on registers).  For this reason, this
+# test generates both a small and a large version for the same
+# combination of special function states.
+#
+# A class is not trivially-copyable if a base class or a field is not
+# trivially-copyable, even though the class definition itself seems
+# trivial.  To test these cases, we also generate derived classes and
+# container classes.
+#
+# The generated code is placed in the test output directory.
+#
+# The companion test file pass-by-ref-2.exp also contains
+# manually-written cases.
 
-if { [skip_cplus_tests] } { continue }
+if {[skip_cplus_tests]} {
+    untested "c++ test skipped"
+    continue
+}
 
+# The program source is generated in the output directory.
+# We use standard_testfile here to set convenience variables.
 standard_testfile .cc
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+# Some constant values used when generating the source
+
+set SMALL    2
+set LARGE    150
+set ORIGINAL 2
+set CUSTOM   3
+set ADDED    4
+set TRACE    5
+
+
+# Return 1 if the class whose special function states are STATES
+# is copyable.  Otherwise return 0.
+
+proc is_copy_constructible { states } {
+    set cctor [lindex $states 0]
+    set dtor  [lindex $states 1]
+    set mctor [lindex $states 2]
+
+    if {$cctor == "deleted" || ($cctor == "absent" && $mctor != "absent")} {
+	return 0
+    }
+    return 1
+}
+
+# Generate a declaration and an out-of-class definition for a function
+# with the provided signature.  The STATE should be one of the following:
+# - explicit, defaultedIn, defaultedOut, deleted, absent
+
+proc generate_member_function { classname signature length state } {
+    set declaration ""
+    set definition ""
+
+    global CUSTOM
+    global TRACE
+
+    switch $state {
+	explicit {
+	    set declaration "$signature;\n"
+	    set definition "$classname\:\:$signature
+                            {
+                              data\[0\] = $CUSTOM;
+                              data\[[expr $length - 1]\] = $CUSTOM;
+                              tracer = $TRACE;
+                            }\n"
+	}
+	defaultedIn {
+	    set declaration "$signature = default;\n"
+	}
+	defaultedOut {
+	    set declaration "$signature;\n"
+	    set definition "$classname\:\:$signature = default;\n"
+	}
+	deleted {
+	    set declaration "$signature = delete;\n"
+	}
+	default {
+	    # function is not user-defined in this case
+	}
+    }
+
+    return [list $declaration $definition]
+}
+
+# Generate a C++ class with the given CLASSNAME and LENGTH-many
+# integer elements.  The STATES is an array of 3 items
+# containing the desired state of the special functions
+# in this order:
+# copy constructor, destructor, move constructor
+
+proc generate_class { classname length states } {
+    set declarations ""
+    set definitions ""
+    set classname "${classname}_[join $states _]"
+
+    for {set i 0} {$i < [llength $states]} {incr i} {
+	set sig ""
+	switch $i {
+	    0 {set sig "$classname (const $classname \&rhs)"}
+	    1 {set sig "\~$classname (void)"}
+	    2 {set sig "$classname ($classname \&\&rhs)"}
+	}
+
+	set state [lindex $states $i]
+	set code [generate_member_function $classname $sig $length $state]
+	append declarations [lindex $code 0]
+	append definitions [lindex $code 1]
+    }
+
+    global ORIGINAL
+
+    return "
+    /*** C++ class $classname ***/
+    class ${classname} {
+    public:
+        $classname (void);
+        $declarations
+
+        int data\[$length\];
+    };
+
+    $classname\:\:$classname (void)
+    {
+        data\[0\] = $ORIGINAL;
+        data\[[expr $length - 1]\] = $ORIGINAL;
+    }
+
+    $definitions
+
+    $classname ${classname}_var; /* global var */
+
+    template int cbv<$classname> ($classname arg);"
+}
+
+# Generate a small C++ class
+
+proc generate_small_class { states } {
+    global SMALL
+    return [generate_class Small $SMALL $states];
+}
+
+# Generate a large C++ class
+
+proc generate_large_class { states } {
+    global LARGE
+    return [generate_class Large $LARGE $states];
+}
+
+# Generate a class that derives from a small class
+
+proc generate_derived_class { states } {
+    set base "Small_[join $states _]"
+    set classname "Derived_[join $states _]"
+
+    return "
+    /*** Class derived from $base ***/
+    class $classname : public $base {
+    public:
+    };
+
+    $classname ${classname}_var; /* global var */
+
+    template int cbv<$classname> ($classname arg);"
+}
+
+# Generate a class that contains a small class item
+
+proc generate_container_class { states } {
+    set contained "Small_[join $states _]"
+    set classname "Container_[join $states _]"
+
+    return "
+    /*** Class that contains $contained ***/
+    class $classname {
+    public:
+        $contained item;
+    };
+
+    $classname ${classname}_var; /* global var */
+
+    template int cbv_container<$classname> ($classname arg);"
+}
+
+# Generate useful statements that use a class in the debugee program
+
+proc generate_stmts { classprefix states {cbvfun "cbv"}} {
+    set classname "${classprefix}_[join $states _]"
+
+    # Having an explicit call to the cbv function in the debugee program
+    # ensures that the compiler will emit necessary function in the binary.
+    if {[is_copy_constructible $states]} {
+	set cbvcall "$cbvfun<$classname> (${classname}_var);\n"
+    } else {
+	set cbvcall ""
+    }
+
+    return "$cbvcall"
+}
+
+# Generate the complete debugee program
+
+proc generate_program { classes stmts } {
+    global ADDED
+
+    return "
+    /*** THIS FILE IS GENERATED BY THE TEST.  ***/
+
+    static int tracer = 0;
+
+    /* The call-by-value function.  */
+    template <class T>
+    int
+    cbv (T arg)
+    {
+      arg.data\[0\] += $ADDED; // intentionally modify the arg
+      return arg.data\[0\];
+    }
+
+    template <class T>
+    int
+    cbv_container (T arg)
+    {
+      arg.item.data\[0\] += $ADDED;  // intentionally modify
+      return arg.item.data\[0\];
+    }
+
+    $classes
+
+    int
+    main (void)
+    {
+      $stmts
+
+      /* stop here */
+
+      return 0;
+    }"
+}
+
+# Compute all the combinations of special function states.
+# We do not contain the 'deleted' state for the destructor,
+# because it is illegal to have stack-allocated objects
+# whose destructor have been deleted.  This case is covered
+# in pass-by-ref-2 via heap-allocated objects.
+
+set options_nodelete [list absent explicit defaultedIn defaultedOut]
+set options [concat $options_nodelete {deleted}]
+set all_combinations {}
+
+foreach cctor $options {
+    foreach dtor $options_nodelete {
+	foreach mctor $options {
+	    lappend all_combinations [list $cctor $dtor $mctor]
+	}
+    }
+}
+
+# Generate the classes.
+
+set classes ""
+set stmts ""
+
+foreach state $all_combinations {
+    append classes [generate_small_class $state]
+    append stmts [generate_stmts "Small" $state]
+
+    append classes [generate_large_class $state]
+    append stmts [generate_stmts "Large" $state]
+
+    append classes [generate_derived_class $state]
+    append stmts [generate_stmts "Derived" $state]
+
+    append classes [generate_container_class $state]
+    append stmts [generate_stmts "Container" $state "cbv_container"]
+}
+
+# Generate the program code and compile
+set program [generate_program $classes $stmts]
+set srcfile [standard_output_file ${srcfile}]
+gdb_produce_source $srcfile $program
+
+set options {debug c++ additional_flags=-std=c++11}
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
     return -1
 }
 
-if ![runto_main] then {
+if {![runto_main]} {
+    untested "failed to run to main"
     return -1
 }
 
-gdb_test "print foo (global_obj)" " = 3" "call function in obj"
-gdb_test "print blap (global_derived)" " = 3" "call function in derived"
-gdb_test "print blip (global_container)" " = 3" "call function in container"
+set bp_location [gdb_get_line_number "stop here"]
+gdb_breakpoint $bp_location
+gdb_continue_to_breakpoint "end of main" ".*return .*;"
+
+# Do the checks for a given class whose name is prefixed with PREFIX,
+# and whose special functions have the states given in STATES.
+# The name of the call-by-value function and the expression to access
+# the data field can be specified explicitly if the default values
+# do not work.
+
+proc test_for_class { prefix states cbvfun data_field length} {
+    set name "${prefix}_[join $states _]"
+
+    set cctor [lindex $states 0]
+    set dtor  [lindex $states 1]
+    set mctor [lindex $states 2]
+
+    global ORIGINAL
+    global CUSTOM
+    global ADDED
+    global TRACE
+
+    with_test_prefix $name {
+	if {[is_copy_constructible $states]} {
+	    set expected [expr {$ORIGINAL + $ADDED}]
+	    if {$cctor == "explicit"} {
+		set expected [expr {$CUSTOM + $ADDED}]
+	    }
+	    if {$dtor == "explicit"} {
+		gdb_test "print tracer = 0" " = 0" "reset the tracer"
+	    }
+	    gdb_test "print ${cbvfun}<$name> (${name}_var)" " = $expected" \
+		"call '$cbvfun'"
+	    gdb_test "print ${name}_var.${data_field}\[0\]" " = $ORIGINAL" \
+		"cbv argument should not change (item 0)"
+	    if {$length > 1} {
+		set last_index [expr $length - 1]
+		gdb_test "print ${name}_var.${data_field}\[$last_index\]" \
+		    " = $ORIGINAL" \
+		    "cbv argument should not change (item $last_index)"
+	    }
+	    if {$dtor == "explicit"} {
+		gdb_test "print tracer" " = $TRACE" \
+		    "destructor should be called"
+	    }
+	} else {
+	    gdb_test "print ${cbvfun}<$name> (${name}_var)" \
+		".* cannot be evaluated .* '${name}' is not copy constructible" \
+		"calling '$cbvfun' should be refused"
+	}
+    }
+}
+
+foreach state $all_combinations {
+    test_for_class "Small"     $state "cbv"           "data"      $SMALL
+    test_for_class "Large"     $state "cbv"           "data"      $LARGE
+    test_for_class "Derived"   $state "cbv"           "data"      1
+    test_for_class "Container" $state "cbv_container" "item.data" 1
+}

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 4
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: merged

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [pushed] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
                   ` (9 preceding siblings ...)
  2019-12-20 16:47 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-12-20 16:47 ` Sourceware to Gerrit sync (Code Review)
  10 siblings, 0 replies; 25+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-12-20 16:47 UTC (permalink / raw)
  To: Tankut Baris Aktemur, Tom Tromey, gdb-patches

The original change was created by Tankut Baris Aktemur.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
......................................................................

testsuite, cp: increase the coverage of testing pass-by-ref arguments

Extend testcases for GDB's infcall of call-by-value functions that
take aggregate values as parameters.  In particular, existing test has
been substantially extended with class definitions whose definitions
of copy constructor, destructor, and move constructor functions are a
combination of

(1) explicitly defined by the user,
(2) defaulted inside the class declaration,
(3) defaulted outside the class declaration,
(4) deleted
(5) not defined in the source.

For each combination, a small and a large class is generated as well
as a derived class and a container class.  Additionally, the following
manually-written cases are provided:

- a dynamic class (i.e. class with a virtual method)
- classes that contain an array field
- a class whose copy ctor is inlined
- a class whose destructor is deleted
- classes with multiple copy and/or move ctors

Test cases check whether GDB makes the right decision to pass an
object by value or implicitly by reference, whether really a copy of
the argument is passed, and whether the copy constructor and
destructor of the clone of the argument are invoked properly.

The input program pass-by-ref.cc is generated in the test's output
directory.  The input program pass-by-ref-2.cc is manually-written.

Tests have been verified on the X86_64 architecture with
GCC 7.4.0, 8.2.0, and 9.2.1.

gdb/testsuite/ChangeLog:
2019-12-20  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.cp/pass-by-ref.cc: Delete.  Generated in the output
	directory instead.
	* gdb.cp/pass-by-ref.exp: Extend with more cases.
	* gdb.cp/pass-by-ref-2.cc: New file.
	* gdb.cp/pass-by-ref-2.exp: New file.

Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
---
M gdb/testsuite/ChangeLog
A gdb/testsuite/gdb.cp/pass-by-ref-2.cc
A gdb/testsuite/gdb.cp/pass-by-ref-2.exp
D gdb/testsuite/gdb.cp/pass-by-ref.cc
M gdb/testsuite/gdb.cp/pass-by-ref.exp
5 files changed, 799 insertions(+), 86 deletions(-)



diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 4476549..c93edc6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2019-12-20  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+
+	* gdb.cp/pass-by-ref.cc: Delete.  Generated in the output
+	directory instead.
+	* gdb.cp/pass-by-ref.exp: Extend with more cases.
+	* gdb.cp/pass-by-ref-2.cc: New file.
+	* gdb.cp/pass-by-ref-2.exp: New file.
+
 2019-12-20  Tom Tromey  <tom@tromey.com>
 
 	* gdb.tui/list-before.exp: New file.
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.cc b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
new file mode 100644
index 0000000..1cd5a16
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
@@ -0,0 +1,295 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+class ByVal {
+public:
+  ByVal (void);
+
+  int x;
+};
+
+ByVal::ByVal (void)
+{
+  x = 2;
+}
+
+class ByRef {
+public:
+  ByRef (void);
+
+  ByRef (const ByRef &rhs);
+
+  int x;
+};
+
+ByRef::ByRef (void)
+{
+  x = 2;
+}
+
+ByRef::ByRef (const ByRef &rhs)
+{
+  x = 3; /* ByRef-cctor */
+}
+
+class ArrayContainerByVal {
+public:
+  ByVal items[2];
+};
+
+int
+cbvArrayContainerByVal (ArrayContainerByVal arg)
+{
+  arg.items[0].x += 4;  // intentionally modify
+  return arg.items[0].x;
+}
+
+class ArrayContainerByRef {
+public:
+  ByRef items[2];
+};
+
+int
+cbvArrayContainerByRef (ArrayContainerByRef arg)
+{
+  arg.items[0].x += 4;  // intentionally modify
+  return arg.items[0].x;
+}
+
+class DynamicBase {
+public:
+  DynamicBase (void);
+
+  virtual int get (void);
+
+  int x;
+};
+
+DynamicBase::DynamicBase (void)
+{
+  x = 2;
+}
+
+int
+DynamicBase::get (void)
+{
+  return 42;
+}
+
+class Dynamic : public DynamicBase {
+public:
+  virtual int get (void);
+};
+
+int
+Dynamic::get (void)
+{
+  return 9999;
+}
+
+int
+cbvDynamic (DynamicBase arg)
+{
+  arg.x += 4;  // intentionally modify
+  return arg.x + arg.get ();
+}
+
+class Inlined {
+public:
+  Inlined (void);
+
+  __attribute__((always_inline))
+  Inlined (const Inlined &rhs)
+  {
+    x = 3;
+  }
+
+  int x;
+};
+
+Inlined::Inlined (void)
+{
+  x = 2;
+}
+
+int
+cbvInlined (Inlined arg)
+{
+  arg.x += 4;  // intentionally modify
+  return arg.x;
+}
+
+class DtorDel {
+public:
+  DtorDel (void);
+
+  ~DtorDel (void) = delete;
+
+  int x;
+};
+
+DtorDel::DtorDel (void)
+{
+  x = 2;
+}
+
+int
+cbvDtorDel (DtorDel arg)
+{
+  // Calling this method should be rejected
+  return arg.x;
+}
+
+class FourCCtor {
+public:
+  FourCCtor (void);
+
+  FourCCtor (FourCCtor &rhs);
+  FourCCtor (const FourCCtor &rhs);
+  FourCCtor (volatile FourCCtor &rhs);
+  FourCCtor (const volatile FourCCtor &rhs);
+
+  int x;
+};
+
+FourCCtor::FourCCtor (void)
+{
+  x = 2;
+}
+
+FourCCtor::FourCCtor (FourCCtor &rhs)
+{
+  x = 3;
+}
+
+FourCCtor::FourCCtor (const FourCCtor &rhs)
+{
+  x = 4;
+}
+
+FourCCtor::FourCCtor (volatile FourCCtor &rhs)
+{
+  x = 5;
+}
+
+FourCCtor::FourCCtor (const volatile FourCCtor &rhs)
+{
+  x = 6;
+}
+
+int
+cbvFourCCtor (FourCCtor arg)
+{
+  arg.x += 10;  // intentionally modify
+  return arg.x;
+}
+
+class TwoMCtor {
+public:
+  TwoMCtor (void);
+
+  /* Even though one move ctor is defaulted, the other
+     is explicit.  */
+  TwoMCtor (const TwoMCtor &&rhs);
+  TwoMCtor (TwoMCtor &&rhs) = default;
+
+  int x;
+};
+
+TwoMCtor::TwoMCtor (void)
+{
+  x = 2;
+}
+
+TwoMCtor::TwoMCtor (const TwoMCtor &&rhs)
+{
+  x = 3;
+}
+
+int
+cbvTwoMCtor (TwoMCtor arg)
+{
+  arg.x += 10;  // intentionally modify
+  return arg.x;
+}
+
+class TwoMCtorAndCCtor {
+public:
+  TwoMCtorAndCCtor (void);
+
+  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &rhs) = default;
+
+  /* Even though one move ctor is defaulted, the other
+     is explicit.  This makes the type pass-by-ref.  */
+  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs);
+  TwoMCtorAndCCtor (TwoMCtorAndCCtor &&rhs) = default;
+
+  int x;
+};
+
+TwoMCtorAndCCtor::TwoMCtorAndCCtor (void)
+{
+  x = 2;
+}
+
+TwoMCtorAndCCtor::TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs)
+{
+  x = 4;
+}
+
+int
+cbvTwoMCtorAndCCtor (TwoMCtorAndCCtor arg)
+{
+  arg.x += 10;  // intentionally modify
+  return arg.x;
+}
+
+ArrayContainerByVal arrayContainerByVal;
+ArrayContainerByRef arrayContainerByRef;
+Dynamic dynamic;
+Inlined inlined;
+// Cannot stack-allocate DtorDel
+DtorDel *dtorDel;
+FourCCtor fourCctor_c0v0;
+const FourCCtor fourCctor_c1v0;
+volatile FourCCtor fourCctor_c0v1;
+const volatile FourCCtor fourCctor_c1v1;
+TwoMCtor twoMctor;
+TwoMCtorAndCCtor twoMctorAndCctor;
+
+int
+main (void)
+{
+  int v;
+  dtorDel = new DtorDel;
+  /* Explicitly call the cbv function to make sure the compiler
+     will not omit any code in the binary.  */
+  v = cbvArrayContainerByVal (arrayContainerByVal);
+  v = cbvArrayContainerByRef (arrayContainerByRef);
+  v = cbvDynamic (dynamic);
+  v = cbvInlined (inlined);
+  v = cbvFourCCtor (fourCctor_c0v0);
+  v = cbvFourCCtor (fourCctor_c1v0);
+  v = cbvFourCCtor (fourCctor_c0v1);
+  v = cbvFourCCtor (fourCctor_c1v1);
+  /* v = cbvTwoMCtor (twoMctor); */ // This is illegal, cctor is deleted
+  v = cbvTwoMCtorAndCCtor (twoMctorAndCctor);
+
+  /* stop here */
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.exp b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
new file mode 100644
index 0000000..7cce886
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
@@ -0,0 +1,114 @@
+# Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that GDB can call C++ functions whose parameters have
+# object type, and are either passed by value or implicitly by reference.
+#
+# This is a companion test to pass-by-ref.exp.  In this test, the input
+# is manually-written.  In pass-by-ref.exp, the test input is generated.
+#
+# We include tests for classes that
+# - contain arrays as fields,
+# - are dynamic (i.e. have virtual methods)
+# - have inlined copy ctor
+# - have deleted destructor
+
+if {[skip_cplus_tests]} {
+    untested "c++ test skipped"
+    continue
+}
+
+standard_testfile .cc
+
+set options {debug c++ additional_flags=-std=c++11}
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
+    return -1
+}
+
+if {![runto_main]} {
+    untested "failed to run to main"
+    return -1
+}
+
+set bp_location [gdb_get_line_number "stop here"]
+gdb_breakpoint $bp_location
+gdb_continue_to_breakpoint "end of main" ".*return .*;"
+
+gdb_test "print cbvArrayContainerByVal (arrayContainerByVal)" "6" \
+    "call cbvArrayContainerByVal"
+gdb_test "print arrayContainerByVal.items\[0\].x" "2" \
+    "cbv argument 'arrayContainerByVal' should not change"
+
+gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" "7" \
+    "call cbvArrayContainerByRef"
+gdb_test "print arrayContainerByRef.items\[0\].x" "2" \
+    "cbv argument 'arrayContainerByRef' should not change"
+
+gdb_test "print cbvDynamic (dynamic)" "48" \
+    "call cbvDynamic"
+gdb_test "print dynamic.x" "2" \
+    "cbv argument 'dynamic' should not change"
+
+set sig "\"Inlined\:\:Inlined\\(.*Inlined const\&\\)\""
+gdb_test "print cbvInlined (inlined)" \
+    "expression cannot be evaluated .* \\(maybe inlined\\?\\)"
+
+gdb_test "print cbvDtorDel (*dtorDel)" \
+    ".* cannot be evaluated .* 'DtorDel' is not destructible" \
+    "type not destructible"
+
+# Test that GDB calls the correct copy ctor
+gdb_test "print cbvFourCCtor (fourCctor_c0v0)" "13" \
+    "call cbvFourCCtor (c0v0)"
+gdb_test "print fourCctor_c0v0.x" "2" \
+    "cbv argument 'twoCctor_c0v0' should not change"
+
+gdb_test "print cbvFourCCtor (fourCctor_c1v0)" "14" \
+    "call cbvFourCCtor (c1v0)"
+gdb_test "print fourCctor_c1v0.x" "2" \
+    "cbv argument 'twoCctor_c1v0' should not change"
+
+gdb_test "print cbvFourCCtor (fourCctor_c0v1)" "15" \
+    "call cbvFourCCtor (c0v1)"
+gdb_test "print fourCctor_c0v1.x" "2" \
+    "cbv argument 'twoCctor_c0v1' should not change"
+
+gdb_test "print cbvFourCCtor (fourCctor_c1v1)" "16" \
+    "call cbvFourCCtor (c1v1)"
+gdb_test "print fourCctor_c1v1.x" "2" \
+    "cbv argument 'twoCctor_c1v1' should not change"
+
+gdb_test "print cbvTwoMCtor (twoMctor)" \
+    ".* cannot be evaluated .* 'TwoMCtor' is not copy constructible" \
+    "copy ctor is implicitly deleted"
+
+gdb_test "print cbvTwoMCtorAndCCtor (twoMctorAndCctor)" "12" \
+    "call cbvTwoMCtorAndCCtor"
+gdb_test "print twoMctorAndCctor.x" "2" \
+    "cbv argument 'twoMctorAndCtor' should not change"
+
+# Test that we get a breakpoint from the cctor during infcall and
+# we can examine arguments.  This is a test that the dummy frame
+# of the copy constructor is set up correctly by the infcall mechanism.
+set bp_location [gdb_get_line_number "ByRef-cctor"]
+gdb_breakpoint $bp_location
+gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" \
+    ".*The program being debugged stopped.*" \
+    "call cbvArrayContainerByRef with BP"
+gdb_test "backtrace" [multi_line \
+    "#0  ByRef\:\:ByRef .* at .*$srcfile:$bp_location" \
+    "#1  .* ArrayContainerByRef::ArrayContainerByRef .*" \
+    "#2  <function called from gdb>" \
+    "#3  main.*"]
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.cc b/gdb/testsuite/gdb.cp/pass-by-ref.cc
deleted file mode 100644
index bbe450a..0000000
--- a/gdb/testsuite/gdb.cp/pass-by-ref.cc
+++ /dev/null
@@ -1,79 +0,0 @@
-/* This testcase is part of GDB, the GNU debugger.
-
-   Copyright 2007-2019 Free Software Foundation, Inc.
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-class Obj {
-public:
-  Obj ();
-  Obj (const Obj &);
-  ~Obj ();
-  int var[2];
-};
-
-int foo (Obj arg)
-{
-  return arg.var[0] + arg.var[1];
-}
-
-Obj::Obj ()
-{
-  var[0] = 1;
-  var[1] = 2;
-}
-
-Obj::Obj (const Obj &obj)
-{
-  var[0] = obj.var[0];
-  var[1] = obj.var[1];
-}
-
-Obj::~Obj ()
-{
-
-}
-
-struct Derived : public Obj
-{
-  int other;
-};
-
-int blap (Derived arg)
-{
-  return foo (arg);
-}
-
-struct Container
-{
-  Obj obj;
-};
-
-int blip (Container arg)
-{
-  return foo (arg.obj);
-}
-
-Obj global_obj;
-Derived global_derived;
-Container global_container;
-
-int
-main ()
-{
-  int bar = foo (global_obj);
-  blap (global_derived);
-  blip (global_container);
-  return bar;
-}
diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.exp b/gdb/testsuite/gdb.cp/pass-by-ref.exp
index 94dd345..f44be77 100644
--- a/gdb/testsuite/gdb.cp/pass-by-ref.exp
+++ b/gdb/testsuite/gdb.cp/pass-by-ref.exp
@@ -14,20 +14,395 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # Check that GDB can call C++ functions whose parameters have
-# object type, but are passed by reference.
+# object type, and are either passed by value or implicitly by reference.
+#
+# Suppose F is a function that has a call-by-value parameter whose
+# type is class C.  When calling F with an argument A, a copy of A should
+# be created and passed to F.  If C is a trivially-copyable type, A can
+# be copied by a straightforward memory copy.  However, roughly speaking,
+# if C has a user-defined copy constructor and/or a user-defined
+# destructor, the copy ctor should be used to initialize the copy of A
+# before calling F, and a reference to that copy is passed to F.  After
+# the function returns, the destructor should be called to destruct the
+# copy.  In this case, C is said to be a 'pass-by-reference' type.
+# Determining whether C is pass-by-ref depends on
+# how the copy ctor, destructor, and the move ctor of C are defined.
+# First of all, C is not copy constructible if its copy constructor is
+# explicitly or implicitly deleted.  In this case, it would be illegal
+# to pass values of type C to a function.  C is pass-by-value, if all of
+# its copy ctor, dtor, and move ctor are trivially defined.
+# Otherwise, it is pass-by-ref.
+#
+# To cover the many possible combinations, this test generates classes
+# that contain three special functions:
+#   (1) a copy constructor,
+#   (2) a destructor, and
+#   (3) a move constructor.
+# A special function is in one of the following states:
+#  * explicit: The function is explicitly defined by the user.
+#  * defaultedIn: The function is defaulted inside the class decl,
+#      using the 'default' keyword.
+#  * defaultedOut: The function is declared inside the class decl,
+#      and defaulted outside using the 'default' keyword.
+#  * deleted: The function is explicitly deleted by the user,
+#      using the 'delete' keyword.
+#  * absent: The function is not declared by the user (i.e. it does not
+#      exist in the source.  The compiler generates (or deletes) the
+#      definition in this case.
+#
+# The C++ ABI decides if a class is pass-by-value or pass-by-ref
+# (i.e.  trivially copyable or not) first at the language level, based
+# on the state of the special functions.  Then, at the target level, a
+# class may be determined to be pass-by-ref because of its size
+# (e.g.  if it is too large to fit on registers).  For this reason, this
+# test generates both a small and a large version for the same
+# combination of special function states.
+#
+# A class is not trivially-copyable if a base class or a field is not
+# trivially-copyable, even though the class definition itself seems
+# trivial.  To test these cases, we also generate derived classes and
+# container classes.
+#
+# The generated code is placed in the test output directory.
+#
+# The companion test file pass-by-ref-2.exp also contains
+# manually-written cases.
 
-if { [skip_cplus_tests] } { continue }
+if {[skip_cplus_tests]} {
+    untested "c++ test skipped"
+    continue
+}
 
+# The program source is generated in the output directory.
+# We use standard_testfile here to set convenience variables.
 standard_testfile .cc
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+# Some constant values used when generating the source
+
+set SMALL    2
+set LARGE    150
+set ORIGINAL 2
+set CUSTOM   3
+set ADDED    4
+set TRACE    5
+
+
+# Return 1 if the class whose special function states are STATES
+# is copyable.  Otherwise return 0.
+
+proc is_copy_constructible { states } {
+    set cctor [lindex $states 0]
+    set dtor  [lindex $states 1]
+    set mctor [lindex $states 2]
+
+    if {$cctor == "deleted" || ($cctor == "absent" && $mctor != "absent")} {
+	return 0
+    }
+    return 1
+}
+
+# Generate a declaration and an out-of-class definition for a function
+# with the provided signature.  The STATE should be one of the following:
+# - explicit, defaultedIn, defaultedOut, deleted, absent
+
+proc generate_member_function { classname signature length state } {
+    set declaration ""
+    set definition ""
+
+    global CUSTOM
+    global TRACE
+
+    switch $state {
+	explicit {
+	    set declaration "$signature;\n"
+	    set definition "$classname\:\:$signature
+                            {
+                              data\[0\] = $CUSTOM;
+                              data\[[expr $length - 1]\] = $CUSTOM;
+                              tracer = $TRACE;
+                            }\n"
+	}
+	defaultedIn {
+	    set declaration "$signature = default;\n"
+	}
+	defaultedOut {
+	    set declaration "$signature;\n"
+	    set definition "$classname\:\:$signature = default;\n"
+	}
+	deleted {
+	    set declaration "$signature = delete;\n"
+	}
+	default {
+	    # function is not user-defined in this case
+	}
+    }
+
+    return [list $declaration $definition]
+}
+
+# Generate a C++ class with the given CLASSNAME and LENGTH-many
+# integer elements.  The STATES is an array of 3 items
+# containing the desired state of the special functions
+# in this order:
+# copy constructor, destructor, move constructor
+
+proc generate_class { classname length states } {
+    set declarations ""
+    set definitions ""
+    set classname "${classname}_[join $states _]"
+
+    for {set i 0} {$i < [llength $states]} {incr i} {
+	set sig ""
+	switch $i {
+	    0 {set sig "$classname (const $classname \&rhs)"}
+	    1 {set sig "\~$classname (void)"}
+	    2 {set sig "$classname ($classname \&\&rhs)"}
+	}
+
+	set state [lindex $states $i]
+	set code [generate_member_function $classname $sig $length $state]
+	append declarations [lindex $code 0]
+	append definitions [lindex $code 1]
+    }
+
+    global ORIGINAL
+
+    return "
+    /*** C++ class $classname ***/
+    class ${classname} {
+    public:
+        $classname (void);
+        $declarations
+
+        int data\[$length\];
+    };
+
+    $classname\:\:$classname (void)
+    {
+        data\[0\] = $ORIGINAL;
+        data\[[expr $length - 1]\] = $ORIGINAL;
+    }
+
+    $definitions
+
+    $classname ${classname}_var; /* global var */
+
+    template int cbv<$classname> ($classname arg);"
+}
+
+# Generate a small C++ class
+
+proc generate_small_class { states } {
+    global SMALL
+    return [generate_class Small $SMALL $states];
+}
+
+# Generate a large C++ class
+
+proc generate_large_class { states } {
+    global LARGE
+    return [generate_class Large $LARGE $states];
+}
+
+# Generate a class that derives from a small class
+
+proc generate_derived_class { states } {
+    set base "Small_[join $states _]"
+    set classname "Derived_[join $states _]"
+
+    return "
+    /*** Class derived from $base ***/
+    class $classname : public $base {
+    public:
+    };
+
+    $classname ${classname}_var; /* global var */
+
+    template int cbv<$classname> ($classname arg);"
+}
+
+# Generate a class that contains a small class item
+
+proc generate_container_class { states } {
+    set contained "Small_[join $states _]"
+    set classname "Container_[join $states _]"
+
+    return "
+    /*** Class that contains $contained ***/
+    class $classname {
+    public:
+        $contained item;
+    };
+
+    $classname ${classname}_var; /* global var */
+
+    template int cbv_container<$classname> ($classname arg);"
+}
+
+# Generate useful statements that use a class in the debugee program
+
+proc generate_stmts { classprefix states {cbvfun "cbv"}} {
+    set classname "${classprefix}_[join $states _]"
+
+    # Having an explicit call to the cbv function in the debugee program
+    # ensures that the compiler will emit necessary function in the binary.
+    if {[is_copy_constructible $states]} {
+	set cbvcall "$cbvfun<$classname> (${classname}_var);\n"
+    } else {
+	set cbvcall ""
+    }
+
+    return "$cbvcall"
+}
+
+# Generate the complete debugee program
+
+proc generate_program { classes stmts } {
+    global ADDED
+
+    return "
+    /*** THIS FILE IS GENERATED BY THE TEST.  ***/
+
+    static int tracer = 0;
+
+    /* The call-by-value function.  */
+    template <class T>
+    int
+    cbv (T arg)
+    {
+      arg.data\[0\] += $ADDED; // intentionally modify the arg
+      return arg.data\[0\];
+    }
+
+    template <class T>
+    int
+    cbv_container (T arg)
+    {
+      arg.item.data\[0\] += $ADDED;  // intentionally modify
+      return arg.item.data\[0\];
+    }
+
+    $classes
+
+    int
+    main (void)
+    {
+      $stmts
+
+      /* stop here */
+
+      return 0;
+    }"
+}
+
+# Compute all the combinations of special function states.
+# We do not contain the 'deleted' state for the destructor,
+# because it is illegal to have stack-allocated objects
+# whose destructor have been deleted.  This case is covered
+# in pass-by-ref-2 via heap-allocated objects.
+
+set options_nodelete [list absent explicit defaultedIn defaultedOut]
+set options [concat $options_nodelete {deleted}]
+set all_combinations {}
+
+foreach cctor $options {
+    foreach dtor $options_nodelete {
+	foreach mctor $options {
+	    lappend all_combinations [list $cctor $dtor $mctor]
+	}
+    }
+}
+
+# Generate the classes.
+
+set classes ""
+set stmts ""
+
+foreach state $all_combinations {
+    append classes [generate_small_class $state]
+    append stmts [generate_stmts "Small" $state]
+
+    append classes [generate_large_class $state]
+    append stmts [generate_stmts "Large" $state]
+
+    append classes [generate_derived_class $state]
+    append stmts [generate_stmts "Derived" $state]
+
+    append classes [generate_container_class $state]
+    append stmts [generate_stmts "Container" $state "cbv_container"]
+}
+
+# Generate the program code and compile
+set program [generate_program $classes $stmts]
+set srcfile [standard_output_file ${srcfile}]
+gdb_produce_source $srcfile $program
+
+set options {debug c++ additional_flags=-std=c++11}
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
     return -1
 }
 
-if ![runto_main] then {
+if {![runto_main]} {
+    untested "failed to run to main"
     return -1
 }
 
-gdb_test "print foo (global_obj)" " = 3" "call function in obj"
-gdb_test "print blap (global_derived)" " = 3" "call function in derived"
-gdb_test "print blip (global_container)" " = 3" "call function in container"
+set bp_location [gdb_get_line_number "stop here"]
+gdb_breakpoint $bp_location
+gdb_continue_to_breakpoint "end of main" ".*return .*;"
+
+# Do the checks for a given class whose name is prefixed with PREFIX,
+# and whose special functions have the states given in STATES.
+# The name of the call-by-value function and the expression to access
+# the data field can be specified explicitly if the default values
+# do not work.
+
+proc test_for_class { prefix states cbvfun data_field length} {
+    set name "${prefix}_[join $states _]"
+
+    set cctor [lindex $states 0]
+    set dtor  [lindex $states 1]
+    set mctor [lindex $states 2]
+
+    global ORIGINAL
+    global CUSTOM
+    global ADDED
+    global TRACE
+
+    with_test_prefix $name {
+	if {[is_copy_constructible $states]} {
+	    set expected [expr {$ORIGINAL + $ADDED}]
+	    if {$cctor == "explicit"} {
+		set expected [expr {$CUSTOM + $ADDED}]
+	    }
+	    if {$dtor == "explicit"} {
+		gdb_test "print tracer = 0" " = 0" "reset the tracer"
+	    }
+	    gdb_test "print ${cbvfun}<$name> (${name}_var)" " = $expected" \
+		"call '$cbvfun'"
+	    gdb_test "print ${name}_var.${data_field}\[0\]" " = $ORIGINAL" \
+		"cbv argument should not change (item 0)"
+	    if {$length > 1} {
+		set last_index [expr $length - 1]
+		gdb_test "print ${name}_var.${data_field}\[$last_index\]" \
+		    " = $ORIGINAL" \
+		    "cbv argument should not change (item $last_index)"
+	    }
+	    if {$dtor == "explicit"} {
+		gdb_test "print tracer" " = $TRACE" \
+		    "destructor should be called"
+	    }
+	} else {
+	    gdb_test "print ${cbvfun}<$name> (${name}_var)" \
+		".* cannot be evaluated .* '${name}' is not copy constructible" \
+		"calling '$cbvfun' should be refused"
+	}
+    }
+}
+
+foreach state $all_combinations {
+    test_for_class "Small"     $state "cbv"           "data"      $SMALL
+    test_for_class "Large"     $state "cbv"           "data"      $LARGE
+    test_for_class "Derived"   $state "cbv"           "data"      1
+    test_for_class "Container" $state "cbv_container" "item.data" 1
+}

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
Gerrit-Change-Number: 142
Gerrit-PatchSet: 4
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2019-12-14  9:53 ` [review v3] " Tankut Baris Aktemur (Code Review)
@ 2020-01-13 18:58   ` Luis Machado
  2020-01-13 19:38     ` Aktemur, Tankut Baris
  0 siblings, 1 reply; 25+ messages in thread
From: Luis Machado @ 2020-01-13 18:58 UTC (permalink / raw)
  To: tankut.baris.aktemur, tromey, gdb-patches,
	Tankut Baris Aktemur (Code Review)

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.

On 12/14/19 6:53 AM, Tankut Baris Aktemur (Code Review) wrote:
> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
> ......................................................................
> 
> testsuite, cp: increase the coverage of testing pass-by-ref arguments
> 
> Extend testcases for GDB's infcall of call-by-value functions that
> take aggregate values as parameters.  In particular, existing test has
> been substantially extended with class definitions whose definitions
> of copy constructor, destructor, and move constructor functions are a
> combination of
> 
> (1) explicitly defined by the user,
> (2) defaulted inside the class declaration,
> (3) defaulted outside the class declaration,
> (4) deleted
> (5) not defined in the source.
> 
> For each combination, a small and a large class is generated as well
> as a derived class and a container class.  Additionally, the following
> manually-written cases are provided:
> 
> - a dynamic class (i.e. class with a virtual method)
> - classes that contain an array field
> - a class whose copy ctor is inlined
> - a class whose destructor is deleted
> - classes with multiple copy and/or move ctors
> 
> Test cases check whether GDB makes the right decision to pass an
> object by value or implicitly by reference, whether really a copy of
> the argument is passed, and whether the copy constructor and
> destructor of the clone of the argument are invoked properly.
> 
> The input program pass-by-ref.cc is generated in the test's output
> directory.  The input program pass-by-ref-2.cc is manually-written.
> 
> Tests have been verified on the X86_64 architecture with
> GCC 7.4.0, 8.2.0, and 9.2.1.
> 
> gdb/testsuite/ChangeLog:
> 2019-11-07  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> 
> 	* gdb.cp/pass-by-ref.cc: Delete.  Generated in the output
> 	directory instead.
> 	* gdb.cp/pass-by-ref.exp: Extend with more cases.
> 	* gdb.cp/pass-by-ref-2.cc: New file.
> 	* gdb.cp/pass-by-ref-2.exp: New file.
> 
> Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
> ---
> A gdb/testsuite/gdb.cp/pass-by-ref-2.cc
> A gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> D gdb/testsuite/gdb.cp/pass-by-ref.cc
> M gdb/testsuite/gdb.cp/pass-by-ref.exp
> 4 files changed, 791 insertions(+), 86 deletions(-)
> 
> 
> 
> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.cc b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
> new file mode 100644
> index 0000000..1cd5a16
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
> @@ -0,0 +1,295 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +class ByVal {
> +public:
> +  ByVal (void);
> +
> +  int x;
> +};
> +
> +ByVal::ByVal (void)
> +{
> +  x = 2;
> +}
> +
> +class ByRef {
> +public:
> +  ByRef (void);
> +
> +  ByRef (const ByRef &rhs);
> +
> +  int x;
> +};
> +
> +ByRef::ByRef (void)
> +{
> +  x = 2;
> +}
> +
> +ByRef::ByRef (const ByRef &rhs)
> +{
> +  x = 3; /* ByRef-cctor */
> +}
> +
> +class ArrayContainerByVal {
> +public:
> +  ByVal items[2];
> +};
> +
> +int
> +cbvArrayContainerByVal (ArrayContainerByVal arg)
> +{
> +  arg.items[0].x += 4;  // intentionally modify
> +  return arg.items[0].x;
> +}
> +
> +class ArrayContainerByRef {
> +public:
> +  ByRef items[2];
> +};
> +
> +int
> +cbvArrayContainerByRef (ArrayContainerByRef arg)
> +{
> +  arg.items[0].x += 4;  // intentionally modify
> +  return arg.items[0].x;
> +}
> +
> +class DynamicBase {
> +public:
> +  DynamicBase (void);
> +
> +  virtual int get (void);
> +
> +  int x;
> +};
> +
> +DynamicBase::DynamicBase (void)
> +{
> +  x = 2;
> +}
> +
> +int
> +DynamicBase::get (void)
> +{
> +  return 42;
> +}
> +
> +class Dynamic : public DynamicBase {
> +public:
> +  virtual int get (void);
> +};
> +
> +int
> +Dynamic::get (void)
> +{
> +  return 9999;
> +}
> +
> +int
> +cbvDynamic (DynamicBase arg)
> +{
> +  arg.x += 4;  // intentionally modify
> +  return arg.x + arg.get ();
> +}
> +
> +class Inlined {
> +public:
> +  Inlined (void);
> +
> +  __attribute__((always_inline))
> +  Inlined (const Inlined &rhs)
> +  {
> +    x = 3;
> +  }
> +
> +  int x;
> +};
> +
> +Inlined::Inlined (void)
> +{
> +  x = 2;
> +}
> +
> +int
> +cbvInlined (Inlined arg)
> +{
> +  arg.x += 4;  // intentionally modify
> +  return arg.x;
> +}
> +
> +class DtorDel {
> +public:
> +  DtorDel (void);
> +
> +  ~DtorDel (void) = delete;
> +
> +  int x;
> +};
> +
> +DtorDel::DtorDel (void)
> +{
> +  x = 2;
> +}
> +
> +int
> +cbvDtorDel (DtorDel arg)
> +{
> +  // Calling this method should be rejected
> +  return arg.x;
> +}
> +
> +class FourCCtor {
> +public:
> +  FourCCtor (void);
> +
> +  FourCCtor (FourCCtor &rhs);
> +  FourCCtor (const FourCCtor &rhs);
> +  FourCCtor (volatile FourCCtor &rhs);
> +  FourCCtor (const volatile FourCCtor &rhs);
> +
> +  int x;
> +};
> +
> +FourCCtor::FourCCtor (void)
> +{
> +  x = 2;
> +}
> +
> +FourCCtor::FourCCtor (FourCCtor &rhs)
> +{
> +  x = 3;
> +}
> +
> +FourCCtor::FourCCtor (const FourCCtor &rhs)
> +{
> +  x = 4;
> +}
> +
> +FourCCtor::FourCCtor (volatile FourCCtor &rhs)
> +{
> +  x = 5;
> +}
> +
> +FourCCtor::FourCCtor (const volatile FourCCtor &rhs)
> +{
> +  x = 6;
> +}
> +
> +int
> +cbvFourCCtor (FourCCtor arg)
> +{
> +  arg.x += 10;  // intentionally modify
> +  return arg.x;
> +}
> +
> +class TwoMCtor {
> +public:
> +  TwoMCtor (void);
> +
> +  /* Even though one move ctor is defaulted, the other
> +     is explicit.  */
> +  TwoMCtor (const TwoMCtor &&rhs);
> +  TwoMCtor (TwoMCtor &&rhs) = default;
> +
> +  int x;
> +};
> +
> +TwoMCtor::TwoMCtor (void)
> +{
> +  x = 2;
> +}
> +
> +TwoMCtor::TwoMCtor (const TwoMCtor &&rhs)
> +{
> +  x = 3;
> +}
> +
> +int
> +cbvTwoMCtor (TwoMCtor arg)
> +{
> +  arg.x += 10;  // intentionally modify
> +  return arg.x;
> +}
> +
> +class TwoMCtorAndCCtor {
> +public:
> +  TwoMCtorAndCCtor (void);
> +
> +  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &rhs) = default;
> +
> +  /* Even though one move ctor is defaulted, the other
> +     is explicit.  This makes the type pass-by-ref.  */
> +  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs);
> +  TwoMCtorAndCCtor (TwoMCtorAndCCtor &&rhs) = default;
> +
> +  int x;
> +};
> +
> +TwoMCtorAndCCtor::TwoMCtorAndCCtor (void)
> +{
> +  x = 2;
> +}
> +
> +TwoMCtorAndCCtor::TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs)
> +{
> +  x = 4;
> +}
> +
> +int
> +cbvTwoMCtorAndCCtor (TwoMCtorAndCCtor arg)
> +{
> +  arg.x += 10;  // intentionally modify
> +  return arg.x;
> +}
> +
> +ArrayContainerByVal arrayContainerByVal;
> +ArrayContainerByRef arrayContainerByRef;
> +Dynamic dynamic;
> +Inlined inlined;
> +// Cannot stack-allocate DtorDel
> +DtorDel *dtorDel;
> +FourCCtor fourCctor_c0v0;
> +const FourCCtor fourCctor_c1v0;
> +volatile FourCCtor fourCctor_c0v1;
> +const volatile FourCCtor fourCctor_c1v1;
> +TwoMCtor twoMctor;
> +TwoMCtorAndCCtor twoMctorAndCctor;
> +
> +int
> +main (void)
> +{
> +  int v;
> +  dtorDel = new DtorDel;
> +  /* Explicitly call the cbv function to make sure the compiler
> +     will not omit any code in the binary.  */
> +  v = cbvArrayContainerByVal (arrayContainerByVal);
> +  v = cbvArrayContainerByRef (arrayContainerByRef);
> +  v = cbvDynamic (dynamic);
> +  v = cbvInlined (inlined);
> +  v = cbvFourCCtor (fourCctor_c0v0);
> +  v = cbvFourCCtor (fourCctor_c1v0);
> +  v = cbvFourCCtor (fourCctor_c0v1);
> +  v = cbvFourCCtor (fourCctor_c1v1);
> +  /* v = cbvTwoMCtor (twoMctor); */ // This is illegal, cctor is deleted
> +  v = cbvTwoMCtorAndCCtor (twoMctorAndCctor);
> +
> +  /* stop here */
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.exp b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> new file mode 100644
> index 0000000..7cce886
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> @@ -0,0 +1,114 @@
> +# Copyright 2019 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Check that GDB can call C++ functions whose parameters have
> +# object type, and are either passed by value or implicitly by reference.
> +#
> +# This is a companion test to pass-by-ref.exp.  In this test, the input
> +# is manually-written.  In pass-by-ref.exp, the test input is generated.
> +#
> +# We include tests for classes that
> +# - contain arrays as fields,
> +# - are dynamic (i.e. have virtual methods)
> +# - have inlined copy ctor
> +# - have deleted destructor
> +
> +if {[skip_cplus_tests]} {
> +    untested "c++ test skipped"
> +    continue
> +}
> +
> +standard_testfile .cc
> +
> +set options {debug c++ additional_flags=-std=c++11}
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
> +    return -1
> +}
> +
> +if {![runto_main]} {
> +    untested "failed to run to main"
> +    return -1
> +}
> +
> +set bp_location [gdb_get_line_number "stop here"]
> +gdb_breakpoint $bp_location
> +gdb_continue_to_breakpoint "end of main" ".*return .*;"
> +
> +gdb_test "print cbvArrayContainerByVal (arrayContainerByVal)" "6" \
> +    "call cbvArrayContainerByVal"
> +gdb_test "print arrayContainerByVal.items\[0\].x" "2" \
> +    "cbv argument 'arrayContainerByVal' should not change"
> +
> +gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" "7" \
> +    "call cbvArrayContainerByRef"
> +gdb_test "print arrayContainerByRef.items\[0\].x" "2" \
> +    "cbv argument 'arrayContainerByRef' should not change"
> +
> +gdb_test "print cbvDynamic (dynamic)" "48" \
> +    "call cbvDynamic"
> +gdb_test "print dynamic.x" "2" \
> +    "cbv argument 'dynamic' should not change"
> +
> +set sig "\"Inlined\:\:Inlined\\(.*Inlined const\&\\)\""
> +gdb_test "print cbvInlined (inlined)" \
> +    "expression cannot be evaluated .* \\(maybe inlined\\?\\)"
> +
> +gdb_test "print cbvDtorDel (*dtorDel)" \
> +    ".* cannot be evaluated .* 'DtorDel' is not destructible" \
> +    "type not destructible"
> +
> +# Test that GDB calls the correct copy ctor
> +gdb_test "print cbvFourCCtor (fourCctor_c0v0)" "13" \
> +    "call cbvFourCCtor (c0v0)"
> +gdb_test "print fourCctor_c0v0.x" "2" \
> +    "cbv argument 'twoCctor_c0v0' should not change"
> +
> +gdb_test "print cbvFourCCtor (fourCctor_c1v0)" "14" \
> +    "call cbvFourCCtor (c1v0)"
> +gdb_test "print fourCctor_c1v0.x" "2" \
> +    "cbv argument 'twoCctor_c1v0' should not change"
> +
> +gdb_test "print cbvFourCCtor (fourCctor_c0v1)" "15" \
> +    "call cbvFourCCtor (c0v1)"
> +gdb_test "print fourCctor_c0v1.x" "2" \
> +    "cbv argument 'twoCctor_c0v1' should not change"
> +
> +gdb_test "print cbvFourCCtor (fourCctor_c1v1)" "16" \
> +    "call cbvFourCCtor (c1v1)"
> +gdb_test "print fourCctor_c1v1.x" "2" \
> +    "cbv argument 'twoCctor_c1v1' should not change"
> +
> +gdb_test "print cbvTwoMCtor (twoMctor)" \
> +    ".* cannot be evaluated .* 'TwoMCtor' is not copy constructible" \
> +    "copy ctor is implicitly deleted"
> +
> +gdb_test "print cbvTwoMCtorAndCCtor (twoMctorAndCctor)" "12" \
> +    "call cbvTwoMCtorAndCCtor"
> +gdb_test "print twoMctorAndCctor.x" "2" \
> +    "cbv argument 'twoMctorAndCtor' should not change"
> +
> +# Test that we get a breakpoint from the cctor during infcall and
> +# we can examine arguments.  This is a test that the dummy frame
> +# of the copy constructor is set up correctly by the infcall mechanism.
> +set bp_location [gdb_get_line_number "ByRef-cctor"]
> +gdb_breakpoint $bp_location
> +gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" \
> +    ".*The program being debugged stopped.*" \
> +    "call cbvArrayContainerByRef with BP"
> +gdb_test "backtrace" [multi_line \
> +    "#0  ByRef\:\:ByRef .* at .*$srcfile:$bp_location" \
> +    "#1  .* ArrayContainerByRef::ArrayContainerByRef .*" \
> +    "#2  <function called from gdb>" \
> +    "#3  main.*"]
> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.cc b/gdb/testsuite/gdb.cp/pass-by-ref.cc
> deleted file mode 100644
> index bbe450a..0000000
> --- a/gdb/testsuite/gdb.cp/pass-by-ref.cc
> +++ /dev/null
> @@ -1,79 +0,0 @@
> -/* This testcase is part of GDB, the GNU debugger.
> -
> -   Copyright 2007-2019 Free Software Foundation, Inc.
> -
> -   This program is free software; you can redistribute it and/or modify
> -   it under the terms of the GNU General Public License as published by
> -   the Free Software Foundation; either version 3 of the License, or
> -   (at your option) any later version.
> -
> -   This program is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> -   GNU General Public License for more details.
> -
> -   You should have received a copy of the GNU General Public License
> -   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> -
> -class Obj {
> -public:
> -  Obj ();
> -  Obj (const Obj &);
> -  ~Obj ();
> -  int var[2];
> -};
> -
> -int foo (Obj arg)
> -{
> -  return arg.var[0] + arg.var[1];
> -}
> -
> -Obj::Obj ()
> -{
> -  var[0] = 1;
> -  var[1] = 2;
> -}
> -
> -Obj::Obj (const Obj &obj)
> -{
> -  var[0] = obj.var[0];
> -  var[1] = obj.var[1];
> -}
> -
> -Obj::~Obj ()
> -{
> -
> -}
> -
> -struct Derived : public Obj
> -{
> -  int other;
> -};
> -
> -int blap (Derived arg)
> -{
> -  return foo (arg);
> -}
> -
> -struct Container
> -{
> -  Obj obj;
> -};
> -
> -int blip (Container arg)
> -{
> -  return foo (arg.obj);
> -}
> -
> -Obj global_obj;
> -Derived global_derived;
> -Container global_container;
> -
> -int
> -main ()
> -{
> -  int bar = foo (global_obj);
> -  blap (global_derived);
> -  blip (global_container);
> -  return bar;
> -}
> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.exp b/gdb/testsuite/gdb.cp/pass-by-ref.exp
> index 94dd345..f44be77 100644
> --- a/gdb/testsuite/gdb.cp/pass-by-ref.exp
> +++ b/gdb/testsuite/gdb.cp/pass-by-ref.exp
> @@ -14,20 +14,395 @@
>   # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   
>   # Check that GDB can call C++ functions whose parameters have
> -# object type, but are passed by reference.
> +# object type, and are either passed by value or implicitly by reference.
> +#
> +# Suppose F is a function that has a call-by-value parameter whose
> +# type is class C.  When calling F with an argument A, a copy of A should
> +# be created and passed to F.  If C is a trivially-copyable type, A can
> +# be copied by a straightforward memory copy.  However, roughly speaking,
> +# if C has a user-defined copy constructor and/or a user-defined
> +# destructor, the copy ctor should be used to initialize the copy of A
> +# before calling F, and a reference to that copy is passed to F.  After
> +# the function returns, the destructor should be called to destruct the
> +# copy.  In this case, C is said to be a 'pass-by-reference' type.
> +# Determining whether C is pass-by-ref depends on
> +# how the copy ctor, destructor, and the move ctor of C are defined.
> +# First of all, C is not copy constructible if its copy constructor is
> +# explicitly or implicitly deleted.  In this case, it would be illegal
> +# to pass values of type C to a function.  C is pass-by-value, if all of
> +# its copy ctor, dtor, and move ctor are trivially defined.
> +# Otherwise, it is pass-by-ref.
> +#
> +# To cover the many possible combinations, this test generates classes
> +# that contain three special functions:
> +#   (1) a copy constructor,
> +#   (2) a destructor, and
> +#   (3) a move constructor.
> +# A special function is in one of the following states:
> +#  * explicit: The function is explicitly defined by the user.
> +#  * defaultedIn: The function is defaulted inside the class decl,
> +#      using the 'default' keyword.
> +#  * defaultedOut: The function is declared inside the class decl,
> +#      and defaulted outside using the 'default' keyword.
> +#  * deleted: The function is explicitly deleted by the user,
> +#      using the 'delete' keyword.
> +#  * absent: The function is not declared by the user (i.e. it does not
> +#      exist in the source.  The compiler generates (or deletes) the
> +#      definition in this case.
> +#
> +# The C++ ABI decides if a class is pass-by-value or pass-by-ref
> +# (i.e.  trivially copyable or not) first at the language level, based
> +# on the state of the special functions.  Then, at the target level, a
> +# class may be determined to be pass-by-ref because of its size
> +# (e.g.  if it is too large to fit on registers).  For this reason, this
> +# test generates both a small and a large version for the same
> +# combination of special function states.
> +#
> +# A class is not trivially-copyable if a base class or a field is not
> +# trivially-copyable, even though the class definition itself seems
> +# trivial.  To test these cases, we also generate derived classes and
> +# container classes.
> +#
> +# The generated code is placed in the test output directory.
> +#
> +# The companion test file pass-by-ref-2.exp also contains
> +# manually-written cases.
>   
> -if { [skip_cplus_tests] } { continue }
> +if {[skip_cplus_tests]} {
> +    untested "c++ test skipped"
> +    continue
> +}
>   
> +# The program source is generated in the output directory.
> +# We use standard_testfile here to set convenience variables.
>   standard_testfile .cc
>   
> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
> +# Some constant values used when generating the source
> +
> +set SMALL    2
> +set LARGE    150
> +set ORIGINAL 2
> +set CUSTOM   3
> +set ADDED    4
> +set TRACE    5
> +
> +
> +# Return 1 if the class whose special function states are STATES
> +# is copyable.  Otherwise return 0.
> +
> +proc is_copy_constructible { states } {
> +    set cctor [lindex $states 0]
> +    set dtor  [lindex $states 1]
> +    set mctor [lindex $states 2]
> +
> +    if {$cctor == "deleted" || ($cctor == "absent" && $mctor != "absent")} {
> +	return 0
> +    }
> +    return 1
> +}
> +
> +# Generate a declaration and an out-of-class definition for a function
> +# with the provided signature.  The STATE should be one of the following:
> +# - explicit, defaultedIn, defaultedOut, deleted, absent
> +
> +proc generate_member_function { classname signature length state } {
> +    set declaration ""
> +    set definition ""
> +
> +    global CUSTOM
> +    global TRACE
> +
> +    switch $state {
> +	explicit {
> +	    set declaration "$signature;\n"
> +	    set definition "$classname\:\:$signature
> +                            {
> +                              data\[0\] = $CUSTOM;
> +                              data\[[expr $length - 1]\] = $CUSTOM;
> +                              tracer = $TRACE;
> +                            }\n"
> +	}
> +	defaultedIn {
> +	    set declaration "$signature = default;\n"
> +	}
> +	defaultedOut {
> +	    set declaration "$signature;\n"
> +	    set definition "$classname\:\:$signature = default;\n"
> +	}
> +	deleted {
> +	    set declaration "$signature = delete;\n"
> +	}
> +	default {
> +	    # function is not user-defined in this case
> +	}
> +    }
> +
> +    return [list $declaration $definition]
> +}
> +
> +# Generate a C++ class with the given CLASSNAME and LENGTH-many
> +# integer elements.  The STATES is an array of 3 items
> +# containing the desired state of the special functions
> +# in this order:
> +# copy constructor, destructor, move constructor
> +
> +proc generate_class { classname length states } {
> +    set declarations ""
> +    set definitions ""
> +    set classname "${classname}_[join $states _]"
> +
> +    for {set i 0} {$i < [llength $states]} {incr i} {
> +	set sig ""
> +	switch $i {
> +	    0 {set sig "$classname (const $classname \&rhs)"}
> +	    1 {set sig "\~$classname (void)"}
> +	    2 {set sig "$classname ($classname \&\&rhs)"}
> +	}
> +
> +	set state [lindex $states $i]
> +	set code [generate_member_function $classname $sig $length $state]
> +	append declarations [lindex $code 0]
> +	append definitions [lindex $code 1]
> +    }
> +
> +    global ORIGINAL
> +
> +    return "
> +    /*** C++ class $classname ***/
> +    class ${classname} {
> +    public:
> +        $classname (void);
> +        $declarations
> +
> +        int data\[$length\];
> +    };
> +
> +    $classname\:\:$classname (void)
> +    {
> +        data\[0\] = $ORIGINAL;
> +        data\[[expr $length - 1]\] = $ORIGINAL;
> +    }
> +
> +    $definitions
> +
> +    $classname ${classname}_var; /* global var */
> +
> +    template int cbv<$classname> ($classname arg);"
> +}
> +
> +# Generate a small C++ class
> +
> +proc generate_small_class { states } {
> +    global SMALL
> +    return [generate_class Small $SMALL $states];
> +}
> +
> +# Generate a large C++ class
> +
> +proc generate_large_class { states } {
> +    global LARGE
> +    return [generate_class Large $LARGE $states];
> +}
> +
> +# Generate a class that derives from a small class
> +
> +proc generate_derived_class { states } {
> +    set base "Small_[join $states _]"
> +    set classname "Derived_[join $states _]"
> +
> +    return "
> +    /*** Class derived from $base ***/
> +    class $classname : public $base {
> +    public:
> +    };
> +
> +    $classname ${classname}_var; /* global var */
> +
> +    template int cbv<$classname> ($classname arg);"
> +}
> +
> +# Generate a class that contains a small class item
> +
> +proc generate_container_class { states } {
> +    set contained "Small_[join $states _]"
> +    set classname "Container_[join $states _]"
> +
> +    return "
> +    /*** Class that contains $contained ***/
> +    class $classname {
> +    public:
> +        $contained item;
> +    };
> +
> +    $classname ${classname}_var; /* global var */
> +
> +    template int cbv_container<$classname> ($classname arg);"
> +}
> +
> +# Generate useful statements that use a class in the debugee program
> +
> +proc generate_stmts { classprefix states {cbvfun "cbv"}} {
> +    set classname "${classprefix}_[join $states _]"
> +
> +    # Having an explicit call to the cbv function in the debugee program
> +    # ensures that the compiler will emit necessary function in the binary.
> +    if {[is_copy_constructible $states]} {
> +	set cbvcall "$cbvfun<$classname> (${classname}_var);\n"
> +    } else {
> +	set cbvcall ""
> +    }
> +
> +    return "$cbvcall"
> +}
> +
> +# Generate the complete debugee program
> +
> +proc generate_program { classes stmts } {
> +    global ADDED
> +
> +    return "
> +    /*** THIS FILE IS GENERATED BY THE TEST.  ***/
> +
> +    static int tracer = 0;
> +
> +    /* The call-by-value function.  */
> +    template <class T>
> +    int
> +    cbv (T arg)
> +    {
> +      arg.data\[0\] += $ADDED; // intentionally modify the arg
> +      return arg.data\[0\];
> +    }
> +
> +    template <class T>
> +    int
> +    cbv_container (T arg)
> +    {
> +      arg.item.data\[0\] += $ADDED;  // intentionally modify
> +      return arg.item.data\[0\];
> +    }
> +
> +    $classes
> +
> +    int
> +    main (void)
> +    {
> +      $stmts
> +
> +      /* stop here */
> +
> +      return 0;
> +    }"
> +}
> +
> +# Compute all the combinations of special function states.
> +# We do not contain the 'deleted' state for the destructor,
> +# because it is illegal to have stack-allocated objects
> +# whose destructor have been deleted.  This case is covered
> +# in pass-by-ref-2 via heap-allocated objects.
> +
> +set options_nodelete [list absent explicit defaultedIn defaultedOut]
> +set options [concat $options_nodelete {deleted}]
> +set all_combinations {}
> +
> +foreach cctor $options {
> +    foreach dtor $options_nodelete {
> +	foreach mctor $options {
> +	    lappend all_combinations [list $cctor $dtor $mctor]
> +	}
> +    }
> +}
> +
> +# Generate the classes.
> +
> +set classes ""
> +set stmts ""
> +
> +foreach state $all_combinations {
> +    append classes [generate_small_class $state]
> +    append stmts [generate_stmts "Small" $state]
> +
> +    append classes [generate_large_class $state]
> +    append stmts [generate_stmts "Large" $state]
> +
> +    append classes [generate_derived_class $state]
> +    append stmts [generate_stmts "Derived" $state]
> +
> +    append classes [generate_container_class $state]
> +    append stmts [generate_stmts "Container" $state "cbv_container"]
> +}
> +
> +# Generate the program code and compile
> +set program [generate_program $classes $stmts]
> +set srcfile [standard_output_file ${srcfile}]
> +gdb_produce_source $srcfile $program
> +
> +set options {debug c++ additional_flags=-std=c++11}
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
>       return -1
>   }
>   
> -if ![runto_main] then {
> +if {![runto_main]} {
> +    untested "failed to run to main"
>       return -1
>   }
>   
> -gdb_test "print foo (global_obj)" " = 3" "call function in obj"
> -gdb_test "print blap (global_derived)" " = 3" "call function in derived"
> -gdb_test "print blip (global_container)" " = 3" "call function in container"
> +set bp_location [gdb_get_line_number "stop here"]
> +gdb_breakpoint $bp_location
> +gdb_continue_to_breakpoint "end of main" ".*return .*;"
> +
> +# Do the checks for a given class whose name is prefixed with PREFIX,
> +# and whose special functions have the states given in STATES.
> +# The name of the call-by-value function and the expression to access
> +# the data field can be specified explicitly if the default values
> +# do not work.
> +
> +proc test_for_class { prefix states cbvfun data_field length} {
> +    set name "${prefix}_[join $states _]"
> +
> +    set cctor [lindex $states 0]
> +    set dtor  [lindex $states 1]
> +    set mctor [lindex $states 2]
> +
> +    global ORIGINAL
> +    global CUSTOM
> +    global ADDED
> +    global TRACE
> +
> +    with_test_prefix $name {
> +	if {[is_copy_constructible $states]} {
> +	    set expected [expr {$ORIGINAL + $ADDED}]
> +	    if {$cctor == "explicit"} {
> +		set expected [expr {$CUSTOM + $ADDED}]
> +	    }
> +	    if {$dtor == "explicit"} {
> +		gdb_test "print tracer = 0" " = 0" "reset the tracer"
> +	    }
> +	    gdb_test "print ${cbvfun}<$name> (${name}_var)" " = $expected" \
> +		"call '$cbvfun'"
> +	    gdb_test "print ${name}_var.${data_field}\[0\]" " = $ORIGINAL" \
> +		"cbv argument should not change (item 0)"
> +	    if {$length > 1} {
> +		set last_index [expr $length - 1]
> +		gdb_test "print ${name}_var.${data_field}\[$last_index\]" \
> +		    " = $ORIGINAL" \
> +		    "cbv argument should not change (item $last_index)"
> +	    }
> +	    if {$dtor == "explicit"} {
> +		gdb_test "print tracer" " = $TRACE" \
> +		    "destructor should be called"
> +	    }
> +	} else {
> +	    gdb_test "print ${cbvfun}<$name> (${name}_var)" \
> +		".* cannot be evaluated .* '${name}' is not copy constructible" \
> +		"calling '$cbvfun' should be refused"
> +	}
> +    }
> +}
> +
> +foreach state $all_combinations {
> +    test_for_class "Small"     $state "cbv"           "data"      $SMALL
> +    test_for_class "Large"     $state "cbv"           "data"      $LARGE
> +    test_for_class "Derived"   $state "cbv"           "data"      1
> +    test_for_class "Container" $state "cbv_container" "item.data" 1
> +}
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2020-01-13 18:58   ` Luis Machado
@ 2020-01-13 19:38     ` Aktemur, Tankut Baris
  2020-01-13 19:40       ` Luis Machado
  0 siblings, 1 reply; 25+ messages in thread
From: Aktemur, Tankut Baris @ 2020-01-13 19:38 UTC (permalink / raw)
  To: Luis Machado, tromey, gdb-patches, Tankut Baris Aktemur (Code Review)

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

-Baris

> On 12/14/19 6:53 AM, Tankut Baris Aktemur (Code Review) wrote:
> > Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
> > ......................................................................
> >
> > testsuite, cp: increase the coverage of testing pass-by-ref arguments
> >
> > Extend testcases for GDB's infcall of call-by-value functions that
> > take aggregate values as parameters.  In particular, existing test has
> > been substantially extended with class definitions whose definitions
> > of copy constructor, destructor, and move constructor functions are a
> > combination of
> >
> > (1) explicitly defined by the user,
> > (2) defaulted inside the class declaration,
> > (3) defaulted outside the class declaration,
> > (4) deleted
> > (5) not defined in the source.
> >
> > For each combination, a small and a large class is generated as well
> > as a derived class and a container class.  Additionally, the following
> > manually-written cases are provided:
> >
> > - a dynamic class (i.e. class with a virtual method)
> > - classes that contain an array field
> > - a class whose copy ctor is inlined
> > - a class whose destructor is deleted
> > - classes with multiple copy and/or move ctors
> >
> > Test cases check whether GDB makes the right decision to pass an
> > object by value or implicitly by reference, whether really a copy of
> > the argument is passed, and whether the copy constructor and
> > destructor of the clone of the argument are invoked properly.
> >
> > The input program pass-by-ref.cc is generated in the test's output
> > directory.  The input program pass-by-ref-2.cc is manually-written.
> >
> > Tests have been verified on the X86_64 architecture with
> > GCC 7.4.0, 8.2.0, and 9.2.1.
> >
> > gdb/testsuite/ChangeLog:
> > 2019-11-07  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> >
> > 	* gdb.cp/pass-by-ref.cc: Delete.  Generated in the output
> > 	directory instead.
> > 	* gdb.cp/pass-by-ref.exp: Extend with more cases.
> > 	* gdb.cp/pass-by-ref-2.cc: New file.
> > 	* gdb.cp/pass-by-ref-2.exp: New file.
> >
> > Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
> > ---
> > A gdb/testsuite/gdb.cp/pass-by-ref-2.cc
> > A gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> > D gdb/testsuite/gdb.cp/pass-by-ref.cc
> > M gdb/testsuite/gdb.cp/pass-by-ref.exp
> > 4 files changed, 791 insertions(+), 86 deletions(-)
> >
> >
> >
> > diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.cc b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
> > new file mode 100644
> > index 0000000..1cd5a16
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
> > @@ -0,0 +1,295 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2019 Free Software Foundation, Inc.
> > +
> > +   This program is free software; you can redistribute it and/or modify
> > +   it under the terms of the GNU General Public License as published by
> > +   the Free Software Foundation; either version 3 of the License, or
> > +   (at your option) any later version.
> > +
> > +   This program is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +   GNU General Public License for more details.
> > +
> > +   You should have received a copy of the GNU General Public License
> > +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> > +
> > +class ByVal {
> > +public:
> > +  ByVal (void);
> > +
> > +  int x;
> > +};
> > +
> > +ByVal::ByVal (void)
> > +{
> > +  x = 2;
> > +}
> > +
> > +class ByRef {
> > +public:
> > +  ByRef (void);
> > +
> > +  ByRef (const ByRef &rhs);
> > +
> > +  int x;
> > +};
> > +
> > +ByRef::ByRef (void)
> > +{
> > +  x = 2;
> > +}
> > +
> > +ByRef::ByRef (const ByRef &rhs)
> > +{
> > +  x = 3; /* ByRef-cctor */
> > +}
> > +
> > +class ArrayContainerByVal {
> > +public:
> > +  ByVal items[2];
> > +};
> > +
> > +int
> > +cbvArrayContainerByVal (ArrayContainerByVal arg)
> > +{
> > +  arg.items[0].x += 4;  // intentionally modify
> > +  return arg.items[0].x;
> > +}
> > +
> > +class ArrayContainerByRef {
> > +public:
> > +  ByRef items[2];
> > +};
> > +
> > +int
> > +cbvArrayContainerByRef (ArrayContainerByRef arg)
> > +{
> > +  arg.items[0].x += 4;  // intentionally modify
> > +  return arg.items[0].x;
> > +}
> > +
> > +class DynamicBase {
> > +public:
> > +  DynamicBase (void);
> > +
> > +  virtual int get (void);
> > +
> > +  int x;
> > +};
> > +
> > +DynamicBase::DynamicBase (void)
> > +{
> > +  x = 2;
> > +}
> > +
> > +int
> > +DynamicBase::get (void)
> > +{
> > +  return 42;
> > +}
> > +
> > +class Dynamic : public DynamicBase {
> > +public:
> > +  virtual int get (void);
> > +};
> > +
> > +int
> > +Dynamic::get (void)
> > +{
> > +  return 9999;
> > +}
> > +
> > +int
> > +cbvDynamic (DynamicBase arg)
> > +{
> > +  arg.x += 4;  // intentionally modify
> > +  return arg.x + arg.get ();
> > +}
> > +
> > +class Inlined {
> > +public:
> > +  Inlined (void);
> > +
> > +  __attribute__((always_inline))
> > +  Inlined (const Inlined &rhs)
> > +  {
> > +    x = 3;
> > +  }
> > +
> > +  int x;
> > +};
> > +
> > +Inlined::Inlined (void)
> > +{
> > +  x = 2;
> > +}
> > +
> > +int
> > +cbvInlined (Inlined arg)
> > +{
> > +  arg.x += 4;  // intentionally modify
> > +  return arg.x;
> > +}
> > +
> > +class DtorDel {
> > +public:
> > +  DtorDel (void);
> > +
> > +  ~DtorDel (void) = delete;
> > +
> > +  int x;
> > +};
> > +
> > +DtorDel::DtorDel (void)
> > +{
> > +  x = 2;
> > +}
> > +
> > +int
> > +cbvDtorDel (DtorDel arg)
> > +{
> > +  // Calling this method should be rejected
> > +  return arg.x;
> > +}
> > +
> > +class FourCCtor {
> > +public:
> > +  FourCCtor (void);
> > +
> > +  FourCCtor (FourCCtor &rhs);
> > +  FourCCtor (const FourCCtor &rhs);
> > +  FourCCtor (volatile FourCCtor &rhs);
> > +  FourCCtor (const volatile FourCCtor &rhs);
> > +
> > +  int x;
> > +};
> > +
> > +FourCCtor::FourCCtor (void)
> > +{
> > +  x = 2;
> > +}
> > +
> > +FourCCtor::FourCCtor (FourCCtor &rhs)
> > +{
> > +  x = 3;
> > +}
> > +
> > +FourCCtor::FourCCtor (const FourCCtor &rhs)
> > +{
> > +  x = 4;
> > +}
> > +
> > +FourCCtor::FourCCtor (volatile FourCCtor &rhs)
> > +{
> > +  x = 5;
> > +}
> > +
> > +FourCCtor::FourCCtor (const volatile FourCCtor &rhs)
> > +{
> > +  x = 6;
> > +}
> > +
> > +int
> > +cbvFourCCtor (FourCCtor arg)
> > +{
> > +  arg.x += 10;  // intentionally modify
> > +  return arg.x;
> > +}
> > +
> > +class TwoMCtor {
> > +public:
> > +  TwoMCtor (void);
> > +
> > +  /* Even though one move ctor is defaulted, the other
> > +     is explicit.  */
> > +  TwoMCtor (const TwoMCtor &&rhs);
> > +  TwoMCtor (TwoMCtor &&rhs) = default;
> > +
> > +  int x;
> > +};
> > +
> > +TwoMCtor::TwoMCtor (void)
> > +{
> > +  x = 2;
> > +}
> > +
> > +TwoMCtor::TwoMCtor (const TwoMCtor &&rhs)
> > +{
> > +  x = 3;
> > +}
> > +
> > +int
> > +cbvTwoMCtor (TwoMCtor arg)
> > +{
> > +  arg.x += 10;  // intentionally modify
> > +  return arg.x;
> > +}
> > +
> > +class TwoMCtorAndCCtor {
> > +public:
> > +  TwoMCtorAndCCtor (void);
> > +
> > +  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &rhs) = default;
> > +
> > +  /* Even though one move ctor is defaulted, the other
> > +     is explicit.  This makes the type pass-by-ref.  */
> > +  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs);
> > +  TwoMCtorAndCCtor (TwoMCtorAndCCtor &&rhs) = default;
> > +
> > +  int x;
> > +};
> > +
> > +TwoMCtorAndCCtor::TwoMCtorAndCCtor (void)
> > +{
> > +  x = 2;
> > +}
> > +
> > +TwoMCtorAndCCtor::TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs)
> > +{
> > +  x = 4;
> > +}
> > +
> > +int
> > +cbvTwoMCtorAndCCtor (TwoMCtorAndCCtor arg)
> > +{
> > +  arg.x += 10;  // intentionally modify
> > +  return arg.x;
> > +}
> > +
> > +ArrayContainerByVal arrayContainerByVal;
> > +ArrayContainerByRef arrayContainerByRef;
> > +Dynamic dynamic;
> > +Inlined inlined;
> > +// Cannot stack-allocate DtorDel
> > +DtorDel *dtorDel;
> > +FourCCtor fourCctor_c0v0;
> > +const FourCCtor fourCctor_c1v0;
> > +volatile FourCCtor fourCctor_c0v1;
> > +const volatile FourCCtor fourCctor_c1v1;
> > +TwoMCtor twoMctor;
> > +TwoMCtorAndCCtor twoMctorAndCctor;
> > +
> > +int
> > +main (void)
> > +{
> > +  int v;
> > +  dtorDel = new DtorDel;
> > +  /* Explicitly call the cbv function to make sure the compiler
> > +     will not omit any code in the binary.  */
> > +  v = cbvArrayContainerByVal (arrayContainerByVal);
> > +  v = cbvArrayContainerByRef (arrayContainerByRef);
> > +  v = cbvDynamic (dynamic);
> > +  v = cbvInlined (inlined);
> > +  v = cbvFourCCtor (fourCctor_c0v0);
> > +  v = cbvFourCCtor (fourCctor_c1v0);
> > +  v = cbvFourCCtor (fourCctor_c0v1);
> > +  v = cbvFourCCtor (fourCctor_c1v1);
> > +  /* v = cbvTwoMCtor (twoMctor); */ // This is illegal, cctor is deleted
> > +  v = cbvTwoMCtorAndCCtor (twoMctorAndCctor);
> > +
> > +  /* stop here */
> > +
> > +  return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.exp b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> > new file mode 100644
> > index 0000000..7cce886
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> > @@ -0,0 +1,114 @@
> > +# Copyright 2019 Free Software Foundation, Inc.
> > +
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 3 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > +
> > +# Check that GDB can call C++ functions whose parameters have
> > +# object type, and are either passed by value or implicitly by reference.
> > +#
> > +# This is a companion test to pass-by-ref.exp.  In this test, the input
> > +# is manually-written.  In pass-by-ref.exp, the test input is generated.
> > +#
> > +# We include tests for classes that
> > +# - contain arrays as fields,
> > +# - are dynamic (i.e. have virtual methods)
> > +# - have inlined copy ctor
> > +# - have deleted destructor
> > +
> > +if {[skip_cplus_tests]} {
> > +    untested "c++ test skipped"
> > +    continue
> > +}
> > +
> > +standard_testfile .cc
> > +
> > +set options {debug c++ additional_flags=-std=c++11}
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
> > +    return -1
> > +}
> > +
> > +if {![runto_main]} {
> > +    untested "failed to run to main"
> > +    return -1
> > +}
> > +
> > +set bp_location [gdb_get_line_number "stop here"]
> > +gdb_breakpoint $bp_location
> > +gdb_continue_to_breakpoint "end of main" ".*return .*;"
> > +
> > +gdb_test "print cbvArrayContainerByVal (arrayContainerByVal)" "6" \
> > +    "call cbvArrayContainerByVal"
> > +gdb_test "print arrayContainerByVal.items\[0\].x" "2" \
> > +    "cbv argument 'arrayContainerByVal' should not change"
> > +
> > +gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" "7" \
> > +    "call cbvArrayContainerByRef"
> > +gdb_test "print arrayContainerByRef.items\[0\].x" "2" \
> > +    "cbv argument 'arrayContainerByRef' should not change"
> > +
> > +gdb_test "print cbvDynamic (dynamic)" "48" \
> > +    "call cbvDynamic"
> > +gdb_test "print dynamic.x" "2" \
> > +    "cbv argument 'dynamic' should not change"
> > +
> > +set sig "\"Inlined\:\:Inlined\\(.*Inlined const\&\\)\""
> > +gdb_test "print cbvInlined (inlined)" \
> > +    "expression cannot be evaluated .* \\(maybe inlined\\?\\)"
> > +
> > +gdb_test "print cbvDtorDel (*dtorDel)" \
> > +    ".* cannot be evaluated .* 'DtorDel' is not destructible" \
> > +    "type not destructible"
> > +
> > +# Test that GDB calls the correct copy ctor
> > +gdb_test "print cbvFourCCtor (fourCctor_c0v0)" "13" \
> > +    "call cbvFourCCtor (c0v0)"
> > +gdb_test "print fourCctor_c0v0.x" "2" \
> > +    "cbv argument 'twoCctor_c0v0' should not change"
> > +
> > +gdb_test "print cbvFourCCtor (fourCctor_c1v0)" "14" \
> > +    "call cbvFourCCtor (c1v0)"
> > +gdb_test "print fourCctor_c1v0.x" "2" \
> > +    "cbv argument 'twoCctor_c1v0' should not change"
> > +
> > +gdb_test "print cbvFourCCtor (fourCctor_c0v1)" "15" \
> > +    "call cbvFourCCtor (c0v1)"
> > +gdb_test "print fourCctor_c0v1.x" "2" \
> > +    "cbv argument 'twoCctor_c0v1' should not change"
> > +
> > +gdb_test "print cbvFourCCtor (fourCctor_c1v1)" "16" \
> > +    "call cbvFourCCtor (c1v1)"
> > +gdb_test "print fourCctor_c1v1.x" "2" \
> > +    "cbv argument 'twoCctor_c1v1' should not change"
> > +
> > +gdb_test "print cbvTwoMCtor (twoMctor)" \
> > +    ".* cannot be evaluated .* 'TwoMCtor' is not copy constructible" \
> > +    "copy ctor is implicitly deleted"
> > +
> > +gdb_test "print cbvTwoMCtorAndCCtor (twoMctorAndCctor)" "12" \
> > +    "call cbvTwoMCtorAndCCtor"
> > +gdb_test "print twoMctorAndCctor.x" "2" \
> > +    "cbv argument 'twoMctorAndCtor' should not change"
> > +
> > +# Test that we get a breakpoint from the cctor during infcall and
> > +# we can examine arguments.  This is a test that the dummy frame
> > +# of the copy constructor is set up correctly by the infcall mechanism.
> > +set bp_location [gdb_get_line_number "ByRef-cctor"]
> > +gdb_breakpoint $bp_location
> > +gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" \
> > +    ".*The program being debugged stopped.*" \
> > +    "call cbvArrayContainerByRef with BP"
> > +gdb_test "backtrace" [multi_line \
> > +    "#0  ByRef\:\:ByRef .* at .*$srcfile:$bp_location" \
> > +    "#1  .* ArrayContainerByRef::ArrayContainerByRef .*" \
> > +    "#2  <function called from gdb>" \
> > +    "#3  main.*"]
> > diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.cc b/gdb/testsuite/gdb.cp/pass-by-ref.cc
> > deleted file mode 100644
> > index bbe450a..0000000
> > --- a/gdb/testsuite/gdb.cp/pass-by-ref.cc
> > +++ /dev/null
> > @@ -1,79 +0,0 @@
> > -/* This testcase is part of GDB, the GNU debugger.
> > -
> > -   Copyright 2007-2019 Free Software Foundation, Inc.
> > -
> > -   This program is free software; you can redistribute it and/or modify
> > -   it under the terms of the GNU General Public License as published by
> > -   the Free Software Foundation; either version 3 of the License, or
> > -   (at your option) any later version.
> > -
> > -   This program is distributed in the hope that it will be useful,
> > -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > -   GNU General Public License for more details.
> > -
> > -   You should have received a copy of the GNU General Public License
> > -   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> > -
> > -class Obj {
> > -public:
> > -  Obj ();
> > -  Obj (const Obj &);
> > -  ~Obj ();
> > -  int var[2];
> > -};
> > -
> > -int foo (Obj arg)
> > -{
> > -  return arg.var[0] + arg.var[1];
> > -}
> > -
> > -Obj::Obj ()
> > -{
> > -  var[0] = 1;
> > -  var[1] = 2;
> > -}
> > -
> > -Obj::Obj (const Obj &obj)
> > -{
> > -  var[0] = obj.var[0];
> > -  var[1] = obj.var[1];
> > -}
> > -
> > -Obj::~Obj ()
> > -{
> > -
> > -}
> > -
> > -struct Derived : public Obj
> > -{
> > -  int other;
> > -};
> > -
> > -int blap (Derived arg)
> > -{
> > -  return foo (arg);
> > -}
> > -
> > -struct Container
> > -{
> > -  Obj obj;
> > -};
> > -
> > -int blip (Container arg)
> > -{
> > -  return foo (arg.obj);
> > -}
> > -
> > -Obj global_obj;
> > -Derived global_derived;
> > -Container global_container;
> > -
> > -int
> > -main ()
> > -{
> > -  int bar = foo (global_obj);
> > -  blap (global_derived);
> > -  blip (global_container);
> > -  return bar;
> > -}
> > diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.exp b/gdb/testsuite/gdb.cp/pass-by-ref.exp
> > index 94dd345..f44be77 100644
> > --- a/gdb/testsuite/gdb.cp/pass-by-ref.exp
> > +++ b/gdb/testsuite/gdb.cp/pass-by-ref.exp
> > @@ -14,20 +14,395 @@
> >   # along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >
> >   # Check that GDB can call C++ functions whose parameters have
> > -# object type, but are passed by reference.
> > +# object type, and are either passed by value or implicitly by reference.
> > +#
> > +# Suppose F is a function that has a call-by-value parameter whose
> > +# type is class C.  When calling F with an argument A, a copy of A should
> > +# be created and passed to F.  If C is a trivially-copyable type, A can
> > +# be copied by a straightforward memory copy.  However, roughly speaking,
> > +# if C has a user-defined copy constructor and/or a user-defined
> > +# destructor, the copy ctor should be used to initialize the copy of A
> > +# before calling F, and a reference to that copy is passed to F.  After
> > +# the function returns, the destructor should be called to destruct the
> > +# copy.  In this case, C is said to be a 'pass-by-reference' type.
> > +# Determining whether C is pass-by-ref depends on
> > +# how the copy ctor, destructor, and the move ctor of C are defined.
> > +# First of all, C is not copy constructible if its copy constructor is
> > +# explicitly or implicitly deleted.  In this case, it would be illegal
> > +# to pass values of type C to a function.  C is pass-by-value, if all of
> > +# its copy ctor, dtor, and move ctor are trivially defined.
> > +# Otherwise, it is pass-by-ref.
> > +#
> > +# To cover the many possible combinations, this test generates classes
> > +# that contain three special functions:
> > +#   (1) a copy constructor,
> > +#   (2) a destructor, and
> > +#   (3) a move constructor.
> > +# A special function is in one of the following states:
> > +#  * explicit: The function is explicitly defined by the user.
> > +#  * defaultedIn: The function is defaulted inside the class decl,
> > +#      using the 'default' keyword.
> > +#  * defaultedOut: The function is declared inside the class decl,
> > +#      and defaulted outside using the 'default' keyword.
> > +#  * deleted: The function is explicitly deleted by the user,
> > +#      using the 'delete' keyword.
> > +#  * absent: The function is not declared by the user (i.e. it does not
> > +#      exist in the source.  The compiler generates (or deletes) the
> > +#      definition in this case.
> > +#
> > +# The C++ ABI decides if a class is pass-by-value or pass-by-ref
> > +# (i.e.  trivially copyable or not) first at the language level, based
> > +# on the state of the special functions.  Then, at the target level, a
> > +# class may be determined to be pass-by-ref because of its size
> > +# (e.g.  if it is too large to fit on registers).  For this reason, this
> > +# test generates both a small and a large version for the same
> > +# combination of special function states.
> > +#
> > +# A class is not trivially-copyable if a base class or a field is not
> > +# trivially-copyable, even though the class definition itself seems
> > +# trivial.  To test these cases, we also generate derived classes and
> > +# container classes.
> > +#
> > +# The generated code is placed in the test output directory.
> > +#
> > +# The companion test file pass-by-ref-2.exp also contains
> > +# manually-written cases.
> >
> > -if { [skip_cplus_tests] } { continue }
> > +if {[skip_cplus_tests]} {
> > +    untested "c++ test skipped"
> > +    continue
> > +}
> >
> > +# The program source is generated in the output directory.
> > +# We use standard_testfile here to set convenience variables.
> >   standard_testfile .cc
> >
> > -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
> > +# Some constant values used when generating the source
> > +
> > +set SMALL    2
> > +set LARGE    150
> > +set ORIGINAL 2
> > +set CUSTOM   3
> > +set ADDED    4
> > +set TRACE    5
> > +
> > +
> > +# Return 1 if the class whose special function states are STATES
> > +# is copyable.  Otherwise return 0.
> > +
> > +proc is_copy_constructible { states } {
> > +    set cctor [lindex $states 0]
> > +    set dtor  [lindex $states 1]
> > +    set mctor [lindex $states 2]
> > +
> > +    if {$cctor == "deleted" || ($cctor == "absent" && $mctor != "absent")} {
> > +	return 0
> > +    }
> > +    return 1
> > +}
> > +
> > +# Generate a declaration and an out-of-class definition for a function
> > +# with the provided signature.  The STATE should be one of the following:
> > +# - explicit, defaultedIn, defaultedOut, deleted, absent
> > +
> > +proc generate_member_function { classname signature length state } {
> > +    set declaration ""
> > +    set definition ""
> > +
> > +    global CUSTOM
> > +    global TRACE
> > +
> > +    switch $state {
> > +	explicit {
> > +	    set declaration "$signature;\n"
> > +	    set definition "$classname\:\:$signature
> > +                            {
> > +                              data\[0\] = $CUSTOM;
> > +                              data\[[expr $length - 1]\] = $CUSTOM;
> > +                              tracer = $TRACE;
> > +                            }\n"
> > +	}
> > +	defaultedIn {
> > +	    set declaration "$signature = default;\n"
> > +	}
> > +	defaultedOut {
> > +	    set declaration "$signature;\n"
> > +	    set definition "$classname\:\:$signature = default;\n"
> > +	}
> > +	deleted {
> > +	    set declaration "$signature = delete;\n"
> > +	}
> > +	default {
> > +	    # function is not user-defined in this case
> > +	}
> > +    }
> > +
> > +    return [list $declaration $definition]
> > +}
> > +
> > +# Generate a C++ class with the given CLASSNAME and LENGTH-many
> > +# integer elements.  The STATES is an array of 3 items
> > +# containing the desired state of the special functions
> > +# in this order:
> > +# copy constructor, destructor, move constructor
> > +
> > +proc generate_class { classname length states } {
> > +    set declarations ""
> > +    set definitions ""
> > +    set classname "${classname}_[join $states _]"
> > +
> > +    for {set i 0} {$i < [llength $states]} {incr i} {
> > +	set sig ""
> > +	switch $i {
> > +	    0 {set sig "$classname (const $classname \&rhs)"}
> > +	    1 {set sig "\~$classname (void)"}
> > +	    2 {set sig "$classname ($classname \&\&rhs)"}
> > +	}
> > +
> > +	set state [lindex $states $i]
> > +	set code [generate_member_function $classname $sig $length $state]
> > +	append declarations [lindex $code 0]
> > +	append definitions [lindex $code 1]
> > +    }
> > +
> > +    global ORIGINAL
> > +
> > +    return "
> > +    /*** C++ class $classname ***/
> > +    class ${classname} {
> > +    public:
> > +        $classname (void);
> > +        $declarations
> > +
> > +        int data\[$length\];
> > +    };
> > +
> > +    $classname\:\:$classname (void)
> > +    {
> > +        data\[0\] = $ORIGINAL;
> > +        data\[[expr $length - 1]\] = $ORIGINAL;
> > +    }
> > +
> > +    $definitions
> > +
> > +    $classname ${classname}_var; /* global var */
> > +
> > +    template int cbv<$classname> ($classname arg);"
> > +}
> > +
> > +# Generate a small C++ class
> > +
> > +proc generate_small_class { states } {
> > +    global SMALL
> > +    return [generate_class Small $SMALL $states];
> > +}
> > +
> > +# Generate a large C++ class
> > +
> > +proc generate_large_class { states } {
> > +    global LARGE
> > +    return [generate_class Large $LARGE $states];
> > +}
> > +
> > +# Generate a class that derives from a small class
> > +
> > +proc generate_derived_class { states } {
> > +    set base "Small_[join $states _]"
> > +    set classname "Derived_[join $states _]"
> > +
> > +    return "
> > +    /*** Class derived from $base ***/
> > +    class $classname : public $base {
> > +    public:
> > +    };
> > +
> > +    $classname ${classname}_var; /* global var */
> > +
> > +    template int cbv<$classname> ($classname arg);"
> > +}
> > +
> > +# Generate a class that contains a small class item
> > +
> > +proc generate_container_class { states } {
> > +    set contained "Small_[join $states _]"
> > +    set classname "Container_[join $states _]"
> > +
> > +    return "
> > +    /*** Class that contains $contained ***/
> > +    class $classname {
> > +    public:
> > +        $contained item;
> > +    };
> > +
> > +    $classname ${classname}_var; /* global var */
> > +
> > +    template int cbv_container<$classname> ($classname arg);"
> > +}
> > +
> > +# Generate useful statements that use a class in the debugee program
> > +
> > +proc generate_stmts { classprefix states {cbvfun "cbv"}} {
> > +    set classname "${classprefix}_[join $states _]"
> > +
> > +    # Having an explicit call to the cbv function in the debugee program
> > +    # ensures that the compiler will emit necessary function in the binary.
> > +    if {[is_copy_constructible $states]} {
> > +	set cbvcall "$cbvfun<$classname> (${classname}_var);\n"
> > +    } else {
> > +	set cbvcall ""
> > +    }
> > +
> > +    return "$cbvcall"
> > +}
> > +
> > +# Generate the complete debugee program
> > +
> > +proc generate_program { classes stmts } {
> > +    global ADDED
> > +
> > +    return "
> > +    /*** THIS FILE IS GENERATED BY THE TEST.  ***/
> > +
> > +    static int tracer = 0;
> > +
> > +    /* The call-by-value function.  */
> > +    template <class T>
> > +    int
> > +    cbv (T arg)
> > +    {
> > +      arg.data\[0\] += $ADDED; // intentionally modify the arg
> > +      return arg.data\[0\];
> > +    }
> > +
> > +    template <class T>
> > +    int
> > +    cbv_container (T arg)
> > +    {
> > +      arg.item.data\[0\] += $ADDED;  // intentionally modify
> > +      return arg.item.data\[0\];
> > +    }
> > +
> > +    $classes
> > +
> > +    int
> > +    main (void)
> > +    {
> > +      $stmts
> > +
> > +      /* stop here */
> > +
> > +      return 0;
> > +    }"
> > +}
> > +
> > +# Compute all the combinations of special function states.
> > +# We do not contain the 'deleted' state for the destructor,
> > +# because it is illegal to have stack-allocated objects
> > +# whose destructor have been deleted.  This case is covered
> > +# in pass-by-ref-2 via heap-allocated objects.
> > +
> > +set options_nodelete [list absent explicit defaultedIn defaultedOut]
> > +set options [concat $options_nodelete {deleted}]
> > +set all_combinations {}
> > +
> > +foreach cctor $options {
> > +    foreach dtor $options_nodelete {
> > +	foreach mctor $options {
> > +	    lappend all_combinations [list $cctor $dtor $mctor]
> > +	}
> > +    }
> > +}
> > +
> > +# Generate the classes.
> > +
> > +set classes ""
> > +set stmts ""
> > +
> > +foreach state $all_combinations {
> > +    append classes [generate_small_class $state]
> > +    append stmts [generate_stmts "Small" $state]
> > +
> > +    append classes [generate_large_class $state]
> > +    append stmts [generate_stmts "Large" $state]
> > +
> > +    append classes [generate_derived_class $state]
> > +    append stmts [generate_stmts "Derived" $state]
> > +
> > +    append classes [generate_container_class $state]
> > +    append stmts [generate_stmts "Container" $state "cbv_container"]
> > +}
> > +
> > +# Generate the program code and compile
> > +set program [generate_program $classes $stmts]
> > +set srcfile [standard_output_file ${srcfile}]
> > +gdb_produce_source $srcfile $program
> > +
> > +set options {debug c++ additional_flags=-std=c++11}
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
> >       return -1
> >   }
> >
> > -if ![runto_main] then {
> > +if {![runto_main]} {
> > +    untested "failed to run to main"
> >       return -1
> >   }
> >
> > -gdb_test "print foo (global_obj)" " = 3" "call function in obj"
> > -gdb_test "print blap (global_derived)" " = 3" "call function in derived"
> > -gdb_test "print blip (global_container)" " = 3" "call function in container"
> > +set bp_location [gdb_get_line_number "stop here"]
> > +gdb_breakpoint $bp_location
> > +gdb_continue_to_breakpoint "end of main" ".*return .*;"
> > +
> > +# Do the checks for a given class whose name is prefixed with PREFIX,
> > +# and whose special functions have the states given in STATES.
> > +# The name of the call-by-value function and the expression to access
> > +# the data field can be specified explicitly if the default values
> > +# do not work.
> > +
> > +proc test_for_class { prefix states cbvfun data_field length} {
> > +    set name "${prefix}_[join $states _]"
> > +
> > +    set cctor [lindex $states 0]
> > +    set dtor  [lindex $states 1]
> > +    set mctor [lindex $states 2]
> > +
> > +    global ORIGINAL
> > +    global CUSTOM
> > +    global ADDED
> > +    global TRACE
> > +
> > +    with_test_prefix $name {
> > +	if {[is_copy_constructible $states]} {
> > +	    set expected [expr {$ORIGINAL + $ADDED}]
> > +	    if {$cctor == "explicit"} {
> > +		set expected [expr {$CUSTOM + $ADDED}]
> > +	    }
> > +	    if {$dtor == "explicit"} {
> > +		gdb_test "print tracer = 0" " = 0" "reset the tracer"
> > +	    }
> > +	    gdb_test "print ${cbvfun}<$name> (${name}_var)" " = $expected" \
> > +		"call '$cbvfun'"
> > +	    gdb_test "print ${name}_var.${data_field}\[0\]" " = $ORIGINAL" \
> > +		"cbv argument should not change (item 0)"
> > +	    if {$length > 1} {
> > +		set last_index [expr $length - 1]
> > +		gdb_test "print ${name}_var.${data_field}\[$last_index\]" \
> > +		    " = $ORIGINAL" \
> > +		    "cbv argument should not change (item $last_index)"
> > +	    }
> > +	    if {$dtor == "explicit"} {
> > +		gdb_test "print tracer" " = $TRACE" \
> > +		    "destructor should be called"
> > +	    }
> > +	} else {
> > +	    gdb_test "print ${cbvfun}<$name> (${name}_var)" \
> > +		".* cannot be evaluated .* '${name}' is not copy constructible" \
> > +		"calling '$cbvfun' should be refused"
> > +	}
> > +    }
> > +}
> > +
> > +foreach state $all_combinations {
> > +    test_for_class "Small"     $state "cbv"           "data"      $SMALL
> > +    test_for_class "Large"     $state "cbv"           "data"      $LARGE
> > +    test_for_class "Derived"   $state "cbv"           "data"      1
> > +    test_for_class "Container" $state "cbv_container" "item.data" 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2020-01-13 19:38     ` Aktemur, Tankut Baris
@ 2020-01-13 19:40       ` Luis Machado
  2020-01-13 20:21         ` Aktemur, Tankut Baris
  0 siblings, 1 reply; 25+ messages in thread
From: Luis Machado @ 2020-01-13 19:40 UTC (permalink / raw)
  To: Aktemur, Tankut Baris, tromey, gdb-patches,
	Tankut Baris Aktemur (Code Review)

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

> 
> Thanks for the suggestion.
> 
> -Baris
> 
>> On 12/14/19 6:53 AM, Tankut Baris Aktemur (Code Review) wrote:
>>> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
>>> ......................................................................
>>>
>>> testsuite, cp: increase the coverage of testing pass-by-ref arguments
>>>
>>> Extend testcases for GDB's infcall of call-by-value functions that
>>> take aggregate values as parameters.  In particular, existing test has
>>> been substantially extended with class definitions whose definitions
>>> of copy constructor, destructor, and move constructor functions are a
>>> combination of
>>>
>>> (1) explicitly defined by the user,
>>> (2) defaulted inside the class declaration,
>>> (3) defaulted outside the class declaration,
>>> (4) deleted
>>> (5) not defined in the source.
>>>
>>> For each combination, a small and a large class is generated as well
>>> as a derived class and a container class.  Additionally, the following
>>> manually-written cases are provided:
>>>
>>> - a dynamic class (i.e. class with a virtual method)
>>> - classes that contain an array field
>>> - a class whose copy ctor is inlined
>>> - a class whose destructor is deleted
>>> - classes with multiple copy and/or move ctors
>>>
>>> Test cases check whether GDB makes the right decision to pass an
>>> object by value or implicitly by reference, whether really a copy of
>>> the argument is passed, and whether the copy constructor and
>>> destructor of the clone of the argument are invoked properly.
>>>
>>> The input program pass-by-ref.cc is generated in the test's output
>>> directory.  The input program pass-by-ref-2.cc is manually-written.
>>>
>>> Tests have been verified on the X86_64 architecture with
>>> GCC 7.4.0, 8.2.0, and 9.2.1.
>>>
>>> gdb/testsuite/ChangeLog:
>>> 2019-11-07  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
>>>
>>> 	* gdb.cp/pass-by-ref.cc: Delete.  Generated in the output
>>> 	directory instead.
>>> 	* gdb.cp/pass-by-ref.exp: Extend with more cases.
>>> 	* gdb.cp/pass-by-ref-2.cc: New file.
>>> 	* gdb.cp/pass-by-ref-2.exp: New file.
>>>
>>> Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
>>> ---
>>> A gdb/testsuite/gdb.cp/pass-by-ref-2.cc
>>> A gdb/testsuite/gdb.cp/pass-by-ref-2.exp
>>> D gdb/testsuite/gdb.cp/pass-by-ref.cc
>>> M gdb/testsuite/gdb.cp/pass-by-ref.exp
>>> 4 files changed, 791 insertions(+), 86 deletions(-)
>>>
>>>
>>>
>>> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.cc b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
>>> new file mode 100644
>>> index 0000000..1cd5a16
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
>>> @@ -0,0 +1,295 @@
>>> +/* This testcase is part of GDB, the GNU debugger.
>>> +
>>> +   Copyright 2019 Free Software Foundation, Inc.
>>> +
>>> +   This program is free software; you can redistribute it and/or modify
>>> +   it under the terms of the GNU General Public License as published by
>>> +   the Free Software Foundation; either version 3 of the License, or
>>> +   (at your option) any later version.
>>> +
>>> +   This program is distributed in the hope that it will be useful,
>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +   GNU General Public License for more details.
>>> +
>>> +   You should have received a copy of the GNU General Public License
>>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>> +
>>> +class ByVal {
>>> +public:
>>> +  ByVal (void);
>>> +
>>> +  int x;
>>> +};
>>> +
>>> +ByVal::ByVal (void)
>>> +{
>>> +  x = 2;
>>> +}
>>> +
>>> +class ByRef {
>>> +public:
>>> +  ByRef (void);
>>> +
>>> +  ByRef (const ByRef &rhs);
>>> +
>>> +  int x;
>>> +};
>>> +
>>> +ByRef::ByRef (void)
>>> +{
>>> +  x = 2;
>>> +}
>>> +
>>> +ByRef::ByRef (const ByRef &rhs)
>>> +{
>>> +  x = 3; /* ByRef-cctor */
>>> +}
>>> +
>>> +class ArrayContainerByVal {
>>> +public:
>>> +  ByVal items[2];
>>> +};
>>> +
>>> +int
>>> +cbvArrayContainerByVal (ArrayContainerByVal arg)
>>> +{
>>> +  arg.items[0].x += 4;  // intentionally modify
>>> +  return arg.items[0].x;
>>> +}
>>> +
>>> +class ArrayContainerByRef {
>>> +public:
>>> +  ByRef items[2];
>>> +};
>>> +
>>> +int
>>> +cbvArrayContainerByRef (ArrayContainerByRef arg)
>>> +{
>>> +  arg.items[0].x += 4;  // intentionally modify
>>> +  return arg.items[0].x;
>>> +}
>>> +
>>> +class DynamicBase {
>>> +public:
>>> +  DynamicBase (void);
>>> +
>>> +  virtual int get (void);
>>> +
>>> +  int x;
>>> +};
>>> +
>>> +DynamicBase::DynamicBase (void)
>>> +{
>>> +  x = 2;
>>> +}
>>> +
>>> +int
>>> +DynamicBase::get (void)
>>> +{
>>> +  return 42;
>>> +}
>>> +
>>> +class Dynamic : public DynamicBase {
>>> +public:
>>> +  virtual int get (void);
>>> +};
>>> +
>>> +int
>>> +Dynamic::get (void)
>>> +{
>>> +  return 9999;
>>> +}
>>> +
>>> +int
>>> +cbvDynamic (DynamicBase arg)
>>> +{
>>> +  arg.x += 4;  // intentionally modify
>>> +  return arg.x + arg.get ();
>>> +}
>>> +
>>> +class Inlined {
>>> +public:
>>> +  Inlined (void);
>>> +
>>> +  __attribute__((always_inline))
>>> +  Inlined (const Inlined &rhs)
>>> +  {
>>> +    x = 3;
>>> +  }
>>> +
>>> +  int x;
>>> +};
>>> +
>>> +Inlined::Inlined (void)
>>> +{
>>> +  x = 2;
>>> +}
>>> +
>>> +int
>>> +cbvInlined (Inlined arg)
>>> +{
>>> +  arg.x += 4;  // intentionally modify
>>> +  return arg.x;
>>> +}
>>> +
>>> +class DtorDel {
>>> +public:
>>> +  DtorDel (void);
>>> +
>>> +  ~DtorDel (void) = delete;
>>> +
>>> +  int x;
>>> +};
>>> +
>>> +DtorDel::DtorDel (void)
>>> +{
>>> +  x = 2;
>>> +}
>>> +
>>> +int
>>> +cbvDtorDel (DtorDel arg)
>>> +{
>>> +  // Calling this method should be rejected
>>> +  return arg.x;
>>> +}
>>> +
>>> +class FourCCtor {
>>> +public:
>>> +  FourCCtor (void);
>>> +
>>> +  FourCCtor (FourCCtor &rhs);
>>> +  FourCCtor (const FourCCtor &rhs);
>>> +  FourCCtor (volatile FourCCtor &rhs);
>>> +  FourCCtor (const volatile FourCCtor &rhs);
>>> +
>>> +  int x;
>>> +};
>>> +
>>> +FourCCtor::FourCCtor (void)
>>> +{
>>> +  x = 2;
>>> +}
>>> +
>>> +FourCCtor::FourCCtor (FourCCtor &rhs)
>>> +{
>>> +  x = 3;
>>> +}
>>> +
>>> +FourCCtor::FourCCtor (const FourCCtor &rhs)
>>> +{
>>> +  x = 4;
>>> +}
>>> +
>>> +FourCCtor::FourCCtor (volatile FourCCtor &rhs)
>>> +{
>>> +  x = 5;
>>> +}
>>> +
>>> +FourCCtor::FourCCtor (const volatile FourCCtor &rhs)
>>> +{
>>> +  x = 6;
>>> +}
>>> +
>>> +int
>>> +cbvFourCCtor (FourCCtor arg)
>>> +{
>>> +  arg.x += 10;  // intentionally modify
>>> +  return arg.x;
>>> +}
>>> +
>>> +class TwoMCtor {
>>> +public:
>>> +  TwoMCtor (void);
>>> +
>>> +  /* Even though one move ctor is defaulted, the other
>>> +     is explicit.  */
>>> +  TwoMCtor (const TwoMCtor &&rhs);
>>> +  TwoMCtor (TwoMCtor &&rhs) = default;
>>> +
>>> +  int x;
>>> +};
>>> +
>>> +TwoMCtor::TwoMCtor (void)
>>> +{
>>> +  x = 2;
>>> +}
>>> +
>>> +TwoMCtor::TwoMCtor (const TwoMCtor &&rhs)
>>> +{
>>> +  x = 3;
>>> +}
>>> +
>>> +int
>>> +cbvTwoMCtor (TwoMCtor arg)
>>> +{
>>> +  arg.x += 10;  // intentionally modify
>>> +  return arg.x;
>>> +}
>>> +
>>> +class TwoMCtorAndCCtor {
>>> +public:
>>> +  TwoMCtorAndCCtor (void);
>>> +
>>> +  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &rhs) = default;
>>> +
>>> +  /* Even though one move ctor is defaulted, the other
>>> +     is explicit.  This makes the type pass-by-ref.  */
>>> +  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs);
>>> +  TwoMCtorAndCCtor (TwoMCtorAndCCtor &&rhs) = default;
>>> +
>>> +  int x;
>>> +};
>>> +
>>> +TwoMCtorAndCCtor::TwoMCtorAndCCtor (void)
>>> +{
>>> +  x = 2;
>>> +}
>>> +
>>> +TwoMCtorAndCCtor::TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs)
>>> +{
>>> +  x = 4;
>>> +}
>>> +
>>> +int
>>> +cbvTwoMCtorAndCCtor (TwoMCtorAndCCtor arg)
>>> +{
>>> +  arg.x += 10;  // intentionally modify
>>> +  return arg.x;
>>> +}
>>> +
>>> +ArrayContainerByVal arrayContainerByVal;
>>> +ArrayContainerByRef arrayContainerByRef;
>>> +Dynamic dynamic;
>>> +Inlined inlined;
>>> +// Cannot stack-allocate DtorDel
>>> +DtorDel *dtorDel;
>>> +FourCCtor fourCctor_c0v0;
>>> +const FourCCtor fourCctor_c1v0;
>>> +volatile FourCCtor fourCctor_c0v1;
>>> +const volatile FourCCtor fourCctor_c1v1;
>>> +TwoMCtor twoMctor;
>>> +TwoMCtorAndCCtor twoMctorAndCctor;
>>> +
>>> +int
>>> +main (void)
>>> +{
>>> +  int v;
>>> +  dtorDel = new DtorDel;
>>> +  /* Explicitly call the cbv function to make sure the compiler
>>> +     will not omit any code in the binary.  */
>>> +  v = cbvArrayContainerByVal (arrayContainerByVal);
>>> +  v = cbvArrayContainerByRef (arrayContainerByRef);
>>> +  v = cbvDynamic (dynamic);
>>> +  v = cbvInlined (inlined);
>>> +  v = cbvFourCCtor (fourCctor_c0v0);
>>> +  v = cbvFourCCtor (fourCctor_c1v0);
>>> +  v = cbvFourCCtor (fourCctor_c0v1);
>>> +  v = cbvFourCCtor (fourCctor_c1v1);
>>> +  /* v = cbvTwoMCtor (twoMctor); */ // This is illegal, cctor is deleted
>>> +  v = cbvTwoMCtorAndCCtor (twoMctorAndCctor);
>>> +
>>> +  /* stop here */
>>> +
>>> +  return 0;
>>> +}
>>> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.exp b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
>>> new file mode 100644
>>> index 0000000..7cce886
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
>>> @@ -0,0 +1,114 @@
>>> +# Copyright 2019 Free Software Foundation, Inc.
>>> +
>>> +# This program is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 3 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> +
>>> +# Check that GDB can call C++ functions whose parameters have
>>> +# object type, and are either passed by value or implicitly by reference.
>>> +#
>>> +# This is a companion test to pass-by-ref.exp.  In this test, the input
>>> +# is manually-written.  In pass-by-ref.exp, the test input is generated.
>>> +#
>>> +# We include tests for classes that
>>> +# - contain arrays as fields,
>>> +# - are dynamic (i.e. have virtual methods)
>>> +# - have inlined copy ctor
>>> +# - have deleted destructor
>>> +
>>> +if {[skip_cplus_tests]} {
>>> +    untested "c++ test skipped"
>>> +    continue
>>> +}
>>> +
>>> +standard_testfile .cc
>>> +
>>> +set options {debug c++ additional_flags=-std=c++11}
>>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
>>> +    return -1
>>> +}
>>> +
>>> +if {![runto_main]} {
>>> +    untested "failed to run to main"
>>> +    return -1
>>> +}
>>> +
>>> +set bp_location [gdb_get_line_number "stop here"]
>>> +gdb_breakpoint $bp_location
>>> +gdb_continue_to_breakpoint "end of main" ".*return .*;"
>>> +
>>> +gdb_test "print cbvArrayContainerByVal (arrayContainerByVal)" "6" \
>>> +    "call cbvArrayContainerByVal"
>>> +gdb_test "print arrayContainerByVal.items\[0\].x" "2" \
>>> +    "cbv argument 'arrayContainerByVal' should not change"
>>> +
>>> +gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" "7" \
>>> +    "call cbvArrayContainerByRef"
>>> +gdb_test "print arrayContainerByRef.items\[0\].x" "2" \
>>> +    "cbv argument 'arrayContainerByRef' should not change"
>>> +
>>> +gdb_test "print cbvDynamic (dynamic)" "48" \
>>> +    "call cbvDynamic"
>>> +gdb_test "print dynamic.x" "2" \
>>> +    "cbv argument 'dynamic' should not change"
>>> +
>>> +set sig "\"Inlined\:\:Inlined\\(.*Inlined const\&\\)\""
>>> +gdb_test "print cbvInlined (inlined)" \
>>> +    "expression cannot be evaluated .* \\(maybe inlined\\?\\)"
>>> +
>>> +gdb_test "print cbvDtorDel (*dtorDel)" \
>>> +    ".* cannot be evaluated .* 'DtorDel' is not destructible" \
>>> +    "type not destructible"
>>> +
>>> +# Test that GDB calls the correct copy ctor
>>> +gdb_test "print cbvFourCCtor (fourCctor_c0v0)" "13" \
>>> +    "call cbvFourCCtor (c0v0)"
>>> +gdb_test "print fourCctor_c0v0.x" "2" \
>>> +    "cbv argument 'twoCctor_c0v0' should not change"
>>> +
>>> +gdb_test "print cbvFourCCtor (fourCctor_c1v0)" "14" \
>>> +    "call cbvFourCCtor (c1v0)"
>>> +gdb_test "print fourCctor_c1v0.x" "2" \
>>> +    "cbv argument 'twoCctor_c1v0' should not change"
>>> +
>>> +gdb_test "print cbvFourCCtor (fourCctor_c0v1)" "15" \
>>> +    "call cbvFourCCtor (c0v1)"
>>> +gdb_test "print fourCctor_c0v1.x" "2" \
>>> +    "cbv argument 'twoCctor_c0v1' should not change"
>>> +
>>> +gdb_test "print cbvFourCCtor (fourCctor_c1v1)" "16" \
>>> +    "call cbvFourCCtor (c1v1)"
>>> +gdb_test "print fourCctor_c1v1.x" "2" \
>>> +    "cbv argument 'twoCctor_c1v1' should not change"
>>> +
>>> +gdb_test "print cbvTwoMCtor (twoMctor)" \
>>> +    ".* cannot be evaluated .* 'TwoMCtor' is not copy constructible" \
>>> +    "copy ctor is implicitly deleted"
>>> +
>>> +gdb_test "print cbvTwoMCtorAndCCtor (twoMctorAndCctor)" "12" \
>>> +    "call cbvTwoMCtorAndCCtor"
>>> +gdb_test "print twoMctorAndCctor.x" "2" \
>>> +    "cbv argument 'twoMctorAndCtor' should not change"
>>> +
>>> +# Test that we get a breakpoint from the cctor during infcall and
>>> +# we can examine arguments.  This is a test that the dummy frame
>>> +# of the copy constructor is set up correctly by the infcall mechanism.
>>> +set bp_location [gdb_get_line_number "ByRef-cctor"]
>>> +gdb_breakpoint $bp_location
>>> +gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" \
>>> +    ".*The program being debugged stopped.*" \
>>> +    "call cbvArrayContainerByRef with BP"
>>> +gdb_test "backtrace" [multi_line \
>>> +    "#0  ByRef\:\:ByRef .* at .*$srcfile:$bp_location" \
>>> +    "#1  .* ArrayContainerByRef::ArrayContainerByRef .*" \
>>> +    "#2  <function called from gdb>" \
>>> +    "#3  main.*"]
>>> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.cc b/gdb/testsuite/gdb.cp/pass-by-ref.cc
>>> deleted file mode 100644
>>> index bbe450a..0000000
>>> --- a/gdb/testsuite/gdb.cp/pass-by-ref.cc
>>> +++ /dev/null
>>> @@ -1,79 +0,0 @@
>>> -/* This testcase is part of GDB, the GNU debugger.
>>> -
>>> -   Copyright 2007-2019 Free Software Foundation, Inc.
>>> -
>>> -   This program is free software; you can redistribute it and/or modify
>>> -   it under the terms of the GNU General Public License as published by
>>> -   the Free Software Foundation; either version 3 of the License, or
>>> -   (at your option) any later version.
>>> -
>>> -   This program is distributed in the hope that it will be useful,
>>> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> -   GNU General Public License for more details.
>>> -
>>> -   You should have received a copy of the GNU General Public License
>>> -   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>> -
>>> -class Obj {
>>> -public:
>>> -  Obj ();
>>> -  Obj (const Obj &);
>>> -  ~Obj ();
>>> -  int var[2];
>>> -};
>>> -
>>> -int foo (Obj arg)
>>> -{
>>> -  return arg.var[0] + arg.var[1];
>>> -}
>>> -
>>> -Obj::Obj ()
>>> -{
>>> -  var[0] = 1;
>>> -  var[1] = 2;
>>> -}
>>> -
>>> -Obj::Obj (const Obj &obj)
>>> -{
>>> -  var[0] = obj.var[0];
>>> -  var[1] = obj.var[1];
>>> -}
>>> -
>>> -Obj::~Obj ()
>>> -{
>>> -
>>> -}
>>> -
>>> -struct Derived : public Obj
>>> -{
>>> -  int other;
>>> -};
>>> -
>>> -int blap (Derived arg)
>>> -{
>>> -  return foo (arg);
>>> -}
>>> -
>>> -struct Container
>>> -{
>>> -  Obj obj;
>>> -};
>>> -
>>> -int blip (Container arg)
>>> -{
>>> -  return foo (arg.obj);
>>> -}
>>> -
>>> -Obj global_obj;
>>> -Derived global_derived;
>>> -Container global_container;
>>> -
>>> -int
>>> -main ()
>>> -{
>>> -  int bar = foo (global_obj);
>>> -  blap (global_derived);
>>> -  blip (global_container);
>>> -  return bar;
>>> -}
>>> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.exp b/gdb/testsuite/gdb.cp/pass-by-ref.exp
>>> index 94dd345..f44be77 100644
>>> --- a/gdb/testsuite/gdb.cp/pass-by-ref.exp
>>> +++ b/gdb/testsuite/gdb.cp/pass-by-ref.exp
>>> @@ -14,20 +14,395 @@
>>>    # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>
>>>    # Check that GDB can call C++ functions whose parameters have
>>> -# object type, but are passed by reference.
>>> +# object type, and are either passed by value or implicitly by reference.
>>> +#
>>> +# Suppose F is a function that has a call-by-value parameter whose
>>> +# type is class C.  When calling F with an argument A, a copy of A should
>>> +# be created and passed to F.  If C is a trivially-copyable type, A can
>>> +# be copied by a straightforward memory copy.  However, roughly speaking,
>>> +# if C has a user-defined copy constructor and/or a user-defined
>>> +# destructor, the copy ctor should be used to initialize the copy of A
>>> +# before calling F, and a reference to that copy is passed to F.  After
>>> +# the function returns, the destructor should be called to destruct the
>>> +# copy.  In this case, C is said to be a 'pass-by-reference' type.
>>> +# Determining whether C is pass-by-ref depends on
>>> +# how the copy ctor, destructor, and the move ctor of C are defined.
>>> +# First of all, C is not copy constructible if its copy constructor is
>>> +# explicitly or implicitly deleted.  In this case, it would be illegal
>>> +# to pass values of type C to a function.  C is pass-by-value, if all of
>>> +# its copy ctor, dtor, and move ctor are trivially defined.
>>> +# Otherwise, it is pass-by-ref.
>>> +#
>>> +# To cover the many possible combinations, this test generates classes
>>> +# that contain three special functions:
>>> +#   (1) a copy constructor,
>>> +#   (2) a destructor, and
>>> +#   (3) a move constructor.
>>> +# A special function is in one of the following states:
>>> +#  * explicit: The function is explicitly defined by the user.
>>> +#  * defaultedIn: The function is defaulted inside the class decl,
>>> +#      using the 'default' keyword.
>>> +#  * defaultedOut: The function is declared inside the class decl,
>>> +#      and defaulted outside using the 'default' keyword.
>>> +#  * deleted: The function is explicitly deleted by the user,
>>> +#      using the 'delete' keyword.
>>> +#  * absent: The function is not declared by the user (i.e. it does not
>>> +#      exist in the source.  The compiler generates (or deletes) the
>>> +#      definition in this case.
>>> +#
>>> +# The C++ ABI decides if a class is pass-by-value or pass-by-ref
>>> +# (i.e.  trivially copyable or not) first at the language level, based
>>> +# on the state of the special functions.  Then, at the target level, a
>>> +# class may be determined to be pass-by-ref because of its size
>>> +# (e.g.  if it is too large to fit on registers).  For this reason, this
>>> +# test generates both a small and a large version for the same
>>> +# combination of special function states.
>>> +#
>>> +# A class is not trivially-copyable if a base class or a field is not
>>> +# trivially-copyable, even though the class definition itself seems
>>> +# trivial.  To test these cases, we also generate derived classes and
>>> +# container classes.
>>> +#
>>> +# The generated code is placed in the test output directory.
>>> +#
>>> +# The companion test file pass-by-ref-2.exp also contains
>>> +# manually-written cases.
>>>
>>> -if { [skip_cplus_tests] } { continue }
>>> +if {[skip_cplus_tests]} {
>>> +    untested "c++ test skipped"
>>> +    continue
>>> +}
>>>
>>> +# The program source is generated in the output directory.
>>> +# We use standard_testfile here to set convenience variables.
>>>    standard_testfile .cc
>>>
>>> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
>>> +# Some constant values used when generating the source
>>> +
>>> +set SMALL    2
>>> +set LARGE    150
>>> +set ORIGINAL 2
>>> +set CUSTOM   3
>>> +set ADDED    4
>>> +set TRACE    5
>>> +
>>> +
>>> +# Return 1 if the class whose special function states are STATES
>>> +# is copyable.  Otherwise return 0.
>>> +
>>> +proc is_copy_constructible { states } {
>>> +    set cctor [lindex $states 0]
>>> +    set dtor  [lindex $states 1]
>>> +    set mctor [lindex $states 2]
>>> +
>>> +    if {$cctor == "deleted" || ($cctor == "absent" && $mctor != "absent")} {
>>> +	return 0
>>> +    }
>>> +    return 1
>>> +}
>>> +
>>> +# Generate a declaration and an out-of-class definition for a function
>>> +# with the provided signature.  The STATE should be one of the following:
>>> +# - explicit, defaultedIn, defaultedOut, deleted, absent
>>> +
>>> +proc generate_member_function { classname signature length state } {
>>> +    set declaration ""
>>> +    set definition ""
>>> +
>>> +    global CUSTOM
>>> +    global TRACE
>>> +
>>> +    switch $state {
>>> +	explicit {
>>> +	    set declaration "$signature;\n"
>>> +	    set definition "$classname\:\:$signature
>>> +                            {
>>> +                              data\[0\] = $CUSTOM;
>>> +                              data\[[expr $length - 1]\] = $CUSTOM;
>>> +                              tracer = $TRACE;
>>> +                            }\n"
>>> +	}
>>> +	defaultedIn {
>>> +	    set declaration "$signature = default;\n"
>>> +	}
>>> +	defaultedOut {
>>> +	    set declaration "$signature;\n"
>>> +	    set definition "$classname\:\:$signature = default;\n"
>>> +	}
>>> +	deleted {
>>> +	    set declaration "$signature = delete;\n"
>>> +	}
>>> +	default {
>>> +	    # function is not user-defined in this case
>>> +	}
>>> +    }
>>> +
>>> +    return [list $declaration $definition]
>>> +}
>>> +
>>> +# Generate a C++ class with the given CLASSNAME and LENGTH-many
>>> +# integer elements.  The STATES is an array of 3 items
>>> +# containing the desired state of the special functions
>>> +# in this order:
>>> +# copy constructor, destructor, move constructor
>>> +
>>> +proc generate_class { classname length states } {
>>> +    set declarations ""
>>> +    set definitions ""
>>> +    set classname "${classname}_[join $states _]"
>>> +
>>> +    for {set i 0} {$i < [llength $states]} {incr i} {
>>> +	set sig ""
>>> +	switch $i {
>>> +	    0 {set sig "$classname (const $classname \&rhs)"}
>>> +	    1 {set sig "\~$classname (void)"}
>>> +	    2 {set sig "$classname ($classname \&\&rhs)"}
>>> +	}
>>> +
>>> +	set state [lindex $states $i]
>>> +	set code [generate_member_function $classname $sig $length $state]
>>> +	append declarations [lindex $code 0]
>>> +	append definitions [lindex $code 1]
>>> +    }
>>> +
>>> +    global ORIGINAL
>>> +
>>> +    return "
>>> +    /*** C++ class $classname ***/
>>> +    class ${classname} {
>>> +    public:
>>> +        $classname (void);
>>> +        $declarations
>>> +
>>> +        int data\[$length\];
>>> +    };
>>> +
>>> +    $classname\:\:$classname (void)
>>> +    {
>>> +        data\[0\] = $ORIGINAL;
>>> +        data\[[expr $length - 1]\] = $ORIGINAL;
>>> +    }
>>> +
>>> +    $definitions
>>> +
>>> +    $classname ${classname}_var; /* global var */
>>> +
>>> +    template int cbv<$classname> ($classname arg);"
>>> +}
>>> +
>>> +# Generate a small C++ class
>>> +
>>> +proc generate_small_class { states } {
>>> +    global SMALL
>>> +    return [generate_class Small $SMALL $states];
>>> +}
>>> +
>>> +# Generate a large C++ class
>>> +
>>> +proc generate_large_class { states } {
>>> +    global LARGE
>>> +    return [generate_class Large $LARGE $states];
>>> +}
>>> +
>>> +# Generate a class that derives from a small class
>>> +
>>> +proc generate_derived_class { states } {
>>> +    set base "Small_[join $states _]"
>>> +    set classname "Derived_[join $states _]"
>>> +
>>> +    return "
>>> +    /*** Class derived from $base ***/
>>> +    class $classname : public $base {
>>> +    public:
>>> +    };
>>> +
>>> +    $classname ${classname}_var; /* global var */
>>> +
>>> +    template int cbv<$classname> ($classname arg);"
>>> +}
>>> +
>>> +# Generate a class that contains a small class item
>>> +
>>> +proc generate_container_class { states } {
>>> +    set contained "Small_[join $states _]"
>>> +    set classname "Container_[join $states _]"
>>> +
>>> +    return "
>>> +    /*** Class that contains $contained ***/
>>> +    class $classname {
>>> +    public:
>>> +        $contained item;
>>> +    };
>>> +
>>> +    $classname ${classname}_var; /* global var */
>>> +
>>> +    template int cbv_container<$classname> ($classname arg);"
>>> +}
>>> +
>>> +# Generate useful statements that use a class in the debugee program
>>> +
>>> +proc generate_stmts { classprefix states {cbvfun "cbv"}} {
>>> +    set classname "${classprefix}_[join $states _]"
>>> +
>>> +    # Having an explicit call to the cbv function in the debugee program
>>> +    # ensures that the compiler will emit necessary function in the binary.
>>> +    if {[is_copy_constructible $states]} {
>>> +	set cbvcall "$cbvfun<$classname> (${classname}_var);\n"
>>> +    } else {
>>> +	set cbvcall ""
>>> +    }
>>> +
>>> +    return "$cbvcall"
>>> +}
>>> +
>>> +# Generate the complete debugee program
>>> +
>>> +proc generate_program { classes stmts } {
>>> +    global ADDED
>>> +
>>> +    return "
>>> +    /*** THIS FILE IS GENERATED BY THE TEST.  ***/
>>> +
>>> +    static int tracer = 0;
>>> +
>>> +    /* The call-by-value function.  */
>>> +    template <class T>
>>> +    int
>>> +    cbv (T arg)
>>> +    {
>>> +      arg.data\[0\] += $ADDED; // intentionally modify the arg
>>> +      return arg.data\[0\];
>>> +    }
>>> +
>>> +    template <class T>
>>> +    int
>>> +    cbv_container (T arg)
>>> +    {
>>> +      arg.item.data\[0\] += $ADDED;  // intentionally modify
>>> +      return arg.item.data\[0\];
>>> +    }
>>> +
>>> +    $classes
>>> +
>>> +    int
>>> +    main (void)
>>> +    {
>>> +      $stmts
>>> +
>>> +      /* stop here */
>>> +
>>> +      return 0;
>>> +    }"
>>> +}
>>> +
>>> +# Compute all the combinations of special function states.
>>> +# We do not contain the 'deleted' state for the destructor,
>>> +# because it is illegal to have stack-allocated objects
>>> +# whose destructor have been deleted.  This case is covered
>>> +# in pass-by-ref-2 via heap-allocated objects.
>>> +
>>> +set options_nodelete [list absent explicit defaultedIn defaultedOut]
>>> +set options [concat $options_nodelete {deleted}]
>>> +set all_combinations {}
>>> +
>>> +foreach cctor $options {
>>> +    foreach dtor $options_nodelete {
>>> +	foreach mctor $options {
>>> +	    lappend all_combinations [list $cctor $dtor $mctor]
>>> +	}
>>> +    }
>>> +}
>>> +
>>> +# Generate the classes.
>>> +
>>> +set classes ""
>>> +set stmts ""
>>> +
>>> +foreach state $all_combinations {
>>> +    append classes [generate_small_class $state]
>>> +    append stmts [generate_stmts "Small" $state]
>>> +
>>> +    append classes [generate_large_class $state]
>>> +    append stmts [generate_stmts "Large" $state]
>>> +
>>> +    append classes [generate_derived_class $state]
>>> +    append stmts [generate_stmts "Derived" $state]
>>> +
>>> +    append classes [generate_container_class $state]
>>> +    append stmts [generate_stmts "Container" $state "cbv_container"]
>>> +}
>>> +
>>> +# Generate the program code and compile
>>> +set program [generate_program $classes $stmts]
>>> +set srcfile [standard_output_file ${srcfile}]
>>> +gdb_produce_source $srcfile $program
>>> +
>>> +set options {debug c++ additional_flags=-std=c++11}
>>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
>>>        return -1
>>>    }
>>>
>>> -if ![runto_main] then {
>>> +if {![runto_main]} {
>>> +    untested "failed to run to main"
>>>        return -1
>>>    }
>>>
>>> -gdb_test "print foo (global_obj)" " = 3" "call function in obj"
>>> -gdb_test "print blap (global_derived)" " = 3" "call function in derived"
>>> -gdb_test "print blip (global_container)" " = 3" "call function in container"
>>> +set bp_location [gdb_get_line_number "stop here"]
>>> +gdb_breakpoint $bp_location
>>> +gdb_continue_to_breakpoint "end of main" ".*return .*;"
>>> +
>>> +# Do the checks for a given class whose name is prefixed with PREFIX,
>>> +# and whose special functions have the states given in STATES.
>>> +# The name of the call-by-value function and the expression to access
>>> +# the data field can be specified explicitly if the default values
>>> +# do not work.
>>> +
>>> +proc test_for_class { prefix states cbvfun data_field length} {
>>> +    set name "${prefix}_[join $states _]"
>>> +
>>> +    set cctor [lindex $states 0]
>>> +    set dtor  [lindex $states 1]
>>> +    set mctor [lindex $states 2]
>>> +
>>> +    global ORIGINAL
>>> +    global CUSTOM
>>> +    global ADDED
>>> +    global TRACE
>>> +
>>> +    with_test_prefix $name {
>>> +	if {[is_copy_constructible $states]} {
>>> +	    set expected [expr {$ORIGINAL + $ADDED}]
>>> +	    if {$cctor == "explicit"} {
>>> +		set expected [expr {$CUSTOM + $ADDED}]
>>> +	    }
>>> +	    if {$dtor == "explicit"} {
>>> +		gdb_test "print tracer = 0" " = 0" "reset the tracer"
>>> +	    }
>>> +	    gdb_test "print ${cbvfun}<$name> (${name}_var)" " = $expected" \
>>> +		"call '$cbvfun'"
>>> +	    gdb_test "print ${name}_var.${data_field}\[0\]" " = $ORIGINAL" \
>>> +		"cbv argument should not change (item 0)"
>>> +	    if {$length > 1} {
>>> +		set last_index [expr $length - 1]
>>> +		gdb_test "print ${name}_var.${data_field}\[$last_index\]" \
>>> +		    " = $ORIGINAL" \
>>> +		    "cbv argument should not change (item $last_index)"
>>> +	    }
>>> +	    if {$dtor == "explicit"} {
>>> +		gdb_test "print tracer" " = $TRACE" \
>>> +		    "destructor should be called"
>>> +	    }
>>> +	} else {
>>> +	    gdb_test "print ${cbvfun}<$name> (${name}_var)" \
>>> +		".* cannot be evaluated .* '${name}' is not copy constructible" \
>>> +		"calling '$cbvfun' should be refused"
>>> +	}
>>> +    }
>>> +}
>>> +
>>> +foreach state $all_combinations {
>>> +    test_for_class "Small"     $state "cbv"           "data"      $SMALL
>>> +    test_for_class "Large"     $state "cbv"           "data"      $LARGE
>>> +    test_for_class "Derived"   $state "cbv"           "data"      1
>>> +    test_for_class "Container" $state "cbv_container" "item.data" 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
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2020-01-13 19:40       ` Luis Machado
@ 2020-01-13 20:21         ` Aktemur, Tankut Baris
  2020-01-14 13:13           ` Aktemur, Tankut Baris
  0 siblings, 1 reply; 25+ messages in thread
From: Aktemur, Tankut Baris @ 2020-01-13 20:21 UTC (permalink / raw)
  To: Luis Machado, tromey, gdb-patches, Tankut Baris Aktemur (Code Review)

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

> 
> >
> > Thanks for the suggestion.
> >
> > -Baris
> >
> >> On 12/14/19 6:53 AM, Tankut Baris Aktemur (Code Review) wrote:
> >>> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/142
> >>> ......................................................................
> >>>
> >>> testsuite, cp: increase the coverage of testing pass-by-ref arguments
> >>>
> >>> Extend testcases for GDB's infcall of call-by-value functions that
> >>> take aggregate values as parameters.  In particular, existing test has
> >>> been substantially extended with class definitions whose definitions
> >>> of copy constructor, destructor, and move constructor functions are a
> >>> combination of
> >>>
> >>> (1) explicitly defined by the user,
> >>> (2) defaulted inside the class declaration,
> >>> (3) defaulted outside the class declaration,
> >>> (4) deleted
> >>> (5) not defined in the source.
> >>>
> >>> For each combination, a small and a large class is generated as well
> >>> as a derived class and a container class.  Additionally, the following
> >>> manually-written cases are provided:
> >>>
> >>> - a dynamic class (i.e. class with a virtual method)
> >>> - classes that contain an array field
> >>> - a class whose copy ctor is inlined
> >>> - a class whose destructor is deleted
> >>> - classes with multiple copy and/or move ctors
> >>>
> >>> Test cases check whether GDB makes the right decision to pass an
> >>> object by value or implicitly by reference, whether really a copy of
> >>> the argument is passed, and whether the copy constructor and
> >>> destructor of the clone of the argument are invoked properly.
> >>>
> >>> The input program pass-by-ref.cc is generated in the test's output
> >>> directory.  The input program pass-by-ref-2.cc is manually-written.
> >>>
> >>> Tests have been verified on the X86_64 architecture with
> >>> GCC 7.4.0, 8.2.0, and 9.2.1.
> >>>
> >>> gdb/testsuite/ChangeLog:
> >>> 2019-11-07  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> >>>
> >>> 	* gdb.cp/pass-by-ref.cc: Delete.  Generated in the output
> >>> 	directory instead.
> >>> 	* gdb.cp/pass-by-ref.exp: Extend with more cases.
> >>> 	* gdb.cp/pass-by-ref-2.cc: New file.
> >>> 	* gdb.cp/pass-by-ref-2.exp: New file.
> >>>
> >>> Change-Id: Ie8ab1f260c6ad5ee4eb34b2c1597ce24af04abb6
> >>> ---
> >>> A gdb/testsuite/gdb.cp/pass-by-ref-2.cc
> >>> A gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> >>> D gdb/testsuite/gdb.cp/pass-by-ref.cc
> >>> M gdb/testsuite/gdb.cp/pass-by-ref.exp
> >>> 4 files changed, 791 insertions(+), 86 deletions(-)
> >>>
> >>>
> >>>
> >>> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.cc b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
> >>> new file mode 100644
> >>> index 0000000..1cd5a16
> >>> --- /dev/null
> >>> +++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.cc
> >>> @@ -0,0 +1,295 @@
> >>> +/* This testcase is part of GDB, the GNU debugger.
> >>> +
> >>> +   Copyright 2019 Free Software Foundation, Inc.
> >>> +
> >>> +   This program is free software; you can redistribute it and/or modify
> >>> +   it under the terms of the GNU General Public License as published by
> >>> +   the Free Software Foundation; either version 3 of the License, or
> >>> +   (at your option) any later version.
> >>> +
> >>> +   This program is distributed in the hope that it will be useful,
> >>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>> +   GNU General Public License for more details.
> >>> +
> >>> +   You should have received a copy of the GNU General Public License
> >>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >>> +
> >>> +class ByVal {
> >>> +public:
> >>> +  ByVal (void);
> >>> +
> >>> +  int x;
> >>> +};
> >>> +
> >>> +ByVal::ByVal (void)
> >>> +{
> >>> +  x = 2;
> >>> +}
> >>> +
> >>> +class ByRef {
> >>> +public:
> >>> +  ByRef (void);
> >>> +
> >>> +  ByRef (const ByRef &rhs);
> >>> +
> >>> +  int x;
> >>> +};
> >>> +
> >>> +ByRef::ByRef (void)
> >>> +{
> >>> +  x = 2;
> >>> +}
> >>> +
> >>> +ByRef::ByRef (const ByRef &rhs)
> >>> +{
> >>> +  x = 3; /* ByRef-cctor */
> >>> +}
> >>> +
> >>> +class ArrayContainerByVal {
> >>> +public:
> >>> +  ByVal items[2];
> >>> +};
> >>> +
> >>> +int
> >>> +cbvArrayContainerByVal (ArrayContainerByVal arg)
> >>> +{
> >>> +  arg.items[0].x += 4;  // intentionally modify
> >>> +  return arg.items[0].x;
> >>> +}
> >>> +
> >>> +class ArrayContainerByRef {
> >>> +public:
> >>> +  ByRef items[2];
> >>> +};
> >>> +
> >>> +int
> >>> +cbvArrayContainerByRef (ArrayContainerByRef arg)
> >>> +{
> >>> +  arg.items[0].x += 4;  // intentionally modify
> >>> +  return arg.items[0].x;
> >>> +}
> >>> +
> >>> +class DynamicBase {
> >>> +public:
> >>> +  DynamicBase (void);
> >>> +
> >>> +  virtual int get (void);
> >>> +
> >>> +  int x;
> >>> +};
> >>> +
> >>> +DynamicBase::DynamicBase (void)
> >>> +{
> >>> +  x = 2;
> >>> +}
> >>> +
> >>> +int
> >>> +DynamicBase::get (void)
> >>> +{
> >>> +  return 42;
> >>> +}
> >>> +
> >>> +class Dynamic : public DynamicBase {
> >>> +public:
> >>> +  virtual int get (void);
> >>> +};
> >>> +
> >>> +int
> >>> +Dynamic::get (void)
> >>> +{
> >>> +  return 9999;
> >>> +}
> >>> +
> >>> +int
> >>> +cbvDynamic (DynamicBase arg)
> >>> +{
> >>> +  arg.x += 4;  // intentionally modify
> >>> +  return arg.x + arg.get ();
> >>> +}
> >>> +
> >>> +class Inlined {
> >>> +public:
> >>> +  Inlined (void);
> >>> +
> >>> +  __attribute__((always_inline))
> >>> +  Inlined (const Inlined &rhs)
> >>> +  {
> >>> +    x = 3;
> >>> +  }
> >>> +
> >>> +  int x;
> >>> +};
> >>> +
> >>> +Inlined::Inlined (void)
> >>> +{
> >>> +  x = 2;
> >>> +}
> >>> +
> >>> +int
> >>> +cbvInlined (Inlined arg)
> >>> +{
> >>> +  arg.x += 4;  // intentionally modify
> >>> +  return arg.x;
> >>> +}
> >>> +
> >>> +class DtorDel {
> >>> +public:
> >>> +  DtorDel (void);
> >>> +
> >>> +  ~DtorDel (void) = delete;
> >>> +
> >>> +  int x;
> >>> +};
> >>> +
> >>> +DtorDel::DtorDel (void)
> >>> +{
> >>> +  x = 2;
> >>> +}
> >>> +
> >>> +int
> >>> +cbvDtorDel (DtorDel arg)
> >>> +{
> >>> +  // Calling this method should be rejected
> >>> +  return arg.x;
> >>> +}
> >>> +
> >>> +class FourCCtor {
> >>> +public:
> >>> +  FourCCtor (void);
> >>> +
> >>> +  FourCCtor (FourCCtor &rhs);
> >>> +  FourCCtor (const FourCCtor &rhs);
> >>> +  FourCCtor (volatile FourCCtor &rhs);
> >>> +  FourCCtor (const volatile FourCCtor &rhs);
> >>> +
> >>> +  int x;
> >>> +};
> >>> +
> >>> +FourCCtor::FourCCtor (void)
> >>> +{
> >>> +  x = 2;
> >>> +}
> >>> +
> >>> +FourCCtor::FourCCtor (FourCCtor &rhs)
> >>> +{
> >>> +  x = 3;
> >>> +}
> >>> +
> >>> +FourCCtor::FourCCtor (const FourCCtor &rhs)
> >>> +{
> >>> +  x = 4;
> >>> +}
> >>> +
> >>> +FourCCtor::FourCCtor (volatile FourCCtor &rhs)
> >>> +{
> >>> +  x = 5;
> >>> +}
> >>> +
> >>> +FourCCtor::FourCCtor (const volatile FourCCtor &rhs)
> >>> +{
> >>> +  x = 6;
> >>> +}
> >>> +
> >>> +int
> >>> +cbvFourCCtor (FourCCtor arg)
> >>> +{
> >>> +  arg.x += 10;  // intentionally modify
> >>> +  return arg.x;
> >>> +}
> >>> +
> >>> +class TwoMCtor {
> >>> +public:
> >>> +  TwoMCtor (void);
> >>> +
> >>> +  /* Even though one move ctor is defaulted, the other
> >>> +     is explicit.  */
> >>> +  TwoMCtor (const TwoMCtor &&rhs);
> >>> +  TwoMCtor (TwoMCtor &&rhs) = default;
> >>> +
> >>> +  int x;
> >>> +};
> >>> +
> >>> +TwoMCtor::TwoMCtor (void)
> >>> +{
> >>> +  x = 2;
> >>> +}
> >>> +
> >>> +TwoMCtor::TwoMCtor (const TwoMCtor &&rhs)
> >>> +{
> >>> +  x = 3;
> >>> +}
> >>> +
> >>> +int
> >>> +cbvTwoMCtor (TwoMCtor arg)
> >>> +{
> >>> +  arg.x += 10;  // intentionally modify
> >>> +  return arg.x;
> >>> +}
> >>> +
> >>> +class TwoMCtorAndCCtor {
> >>> +public:
> >>> +  TwoMCtorAndCCtor (void);
> >>> +
> >>> +  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &rhs) = default;
> >>> +
> >>> +  /* Even though one move ctor is defaulted, the other
> >>> +     is explicit.  This makes the type pass-by-ref.  */
> >>> +  TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs);
> >>> +  TwoMCtorAndCCtor (TwoMCtorAndCCtor &&rhs) = default;
> >>> +
> >>> +  int x;
> >>> +};
> >>> +
> >>> +TwoMCtorAndCCtor::TwoMCtorAndCCtor (void)
> >>> +{
> >>> +  x = 2;
> >>> +}
> >>> +
> >>> +TwoMCtorAndCCtor::TwoMCtorAndCCtor (const TwoMCtorAndCCtor &&rhs)
> >>> +{
> >>> +  x = 4;
> >>> +}
> >>> +
> >>> +int
> >>> +cbvTwoMCtorAndCCtor (TwoMCtorAndCCtor arg)
> >>> +{
> >>> +  arg.x += 10;  // intentionally modify
> >>> +  return arg.x;
> >>> +}
> >>> +
> >>> +ArrayContainerByVal arrayContainerByVal;
> >>> +ArrayContainerByRef arrayContainerByRef;
> >>> +Dynamic dynamic;
> >>> +Inlined inlined;
> >>> +// Cannot stack-allocate DtorDel
> >>> +DtorDel *dtorDel;
> >>> +FourCCtor fourCctor_c0v0;
> >>> +const FourCCtor fourCctor_c1v0;
> >>> +volatile FourCCtor fourCctor_c0v1;
> >>> +const volatile FourCCtor fourCctor_c1v1;
> >>> +TwoMCtor twoMctor;
> >>> +TwoMCtorAndCCtor twoMctorAndCctor;
> >>> +
> >>> +int
> >>> +main (void)
> >>> +{
> >>> +  int v;
> >>> +  dtorDel = new DtorDel;
> >>> +  /* Explicitly call the cbv function to make sure the compiler
> >>> +     will not omit any code in the binary.  */
> >>> +  v = cbvArrayContainerByVal (arrayContainerByVal);
> >>> +  v = cbvArrayContainerByRef (arrayContainerByRef);
> >>> +  v = cbvDynamic (dynamic);
> >>> +  v = cbvInlined (inlined);
> >>> +  v = cbvFourCCtor (fourCctor_c0v0);
> >>> +  v = cbvFourCCtor (fourCctor_c1v0);
> >>> +  v = cbvFourCCtor (fourCctor_c0v1);
> >>> +  v = cbvFourCCtor (fourCctor_c1v1);
> >>> +  /* v = cbvTwoMCtor (twoMctor); */ // This is illegal, cctor is deleted
> >>> +  v = cbvTwoMCtorAndCCtor (twoMctorAndCctor);
> >>> +
> >>> +  /* stop here */
> >>> +
> >>> +  return 0;
> >>> +}
> >>> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref-2.exp b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> >>> new file mode 100644
> >>> index 0000000..7cce886
> >>> --- /dev/null
> >>> +++ b/gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> >>> @@ -0,0 +1,114 @@
> >>> +# Copyright 2019 Free Software Foundation, Inc.
> >>> +
> >>> +# This program is free software; you can redistribute it and/or modify
> >>> +# it under the terms of the GNU General Public License as published by
> >>> +# the Free Software Foundation; either version 3 of the License, or
> >>> +# (at your option) any later version.
> >>> +#
> >>> +# This program is distributed in the hope that it will be useful,
> >>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>> +# GNU General Public License for more details.
> >>> +#
> >>> +# You should have received a copy of the GNU General Public License
> >>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >>> +
> >>> +# Check that GDB can call C++ functions whose parameters have
> >>> +# object type, and are either passed by value or implicitly by reference.
> >>> +#
> >>> +# This is a companion test to pass-by-ref.exp.  In this test, the input
> >>> +# is manually-written.  In pass-by-ref.exp, the test input is generated.
> >>> +#
> >>> +# We include tests for classes that
> >>> +# - contain arrays as fields,
> >>> +# - are dynamic (i.e. have virtual methods)
> >>> +# - have inlined copy ctor
> >>> +# - have deleted destructor
> >>> +
> >>> +if {[skip_cplus_tests]} {
> >>> +    untested "c++ test skipped"
> >>> +    continue
> >>> +}
> >>> +
> >>> +standard_testfile .cc
> >>> +
> >>> +set options {debug c++ additional_flags=-std=c++11}
> >>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
> >>> +    return -1
> >>> +}
> >>> +
> >>> +if {![runto_main]} {
> >>> +    untested "failed to run to main"
> >>> +    return -1
> >>> +}
> >>> +
> >>> +set bp_location [gdb_get_line_number "stop here"]
> >>> +gdb_breakpoint $bp_location
> >>> +gdb_continue_to_breakpoint "end of main" ".*return .*;"
> >>> +
> >>> +gdb_test "print cbvArrayContainerByVal (arrayContainerByVal)" "6" \
> >>> +    "call cbvArrayContainerByVal"
> >>> +gdb_test "print arrayContainerByVal.items\[0\].x" "2" \
> >>> +    "cbv argument 'arrayContainerByVal' should not change"
> >>> +
> >>> +gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" "7" \
> >>> +    "call cbvArrayContainerByRef"
> >>> +gdb_test "print arrayContainerByRef.items\[0\].x" "2" \
> >>> +    "cbv argument 'arrayContainerByRef' should not change"
> >>> +
> >>> +gdb_test "print cbvDynamic (dynamic)" "48" \
> >>> +    "call cbvDynamic"
> >>> +gdb_test "print dynamic.x" "2" \
> >>> +    "cbv argument 'dynamic' should not change"
> >>> +
> >>> +set sig "\"Inlined\:\:Inlined\\(.*Inlined const\&\\)\""
> >>> +gdb_test "print cbvInlined (inlined)" \
> >>> +    "expression cannot be evaluated .* \\(maybe inlined\\?\\)"
> >>> +
> >>> +gdb_test "print cbvDtorDel (*dtorDel)" \
> >>> +    ".* cannot be evaluated .* 'DtorDel' is not destructible" \
> >>> +    "type not destructible"
> >>> +
> >>> +# Test that GDB calls the correct copy ctor
> >>> +gdb_test "print cbvFourCCtor (fourCctor_c0v0)" "13" \
> >>> +    "call cbvFourCCtor (c0v0)"
> >>> +gdb_test "print fourCctor_c0v0.x" "2" \
> >>> +    "cbv argument 'twoCctor_c0v0' should not change"
> >>> +
> >>> +gdb_test "print cbvFourCCtor (fourCctor_c1v0)" "14" \
> >>> +    "call cbvFourCCtor (c1v0)"
> >>> +gdb_test "print fourCctor_c1v0.x" "2" \
> >>> +    "cbv argument 'twoCctor_c1v0' should not change"
> >>> +
> >>> +gdb_test "print cbvFourCCtor (fourCctor_c0v1)" "15" \
> >>> +    "call cbvFourCCtor (c0v1)"
> >>> +gdb_test "print fourCctor_c0v1.x" "2" \
> >>> +    "cbv argument 'twoCctor_c0v1' should not change"
> >>> +
> >>> +gdb_test "print cbvFourCCtor (fourCctor_c1v1)" "16" \
> >>> +    "call cbvFourCCtor (c1v1)"
> >>> +gdb_test "print fourCctor_c1v1.x" "2" \
> >>> +    "cbv argument 'twoCctor_c1v1' should not change"
> >>> +
> >>> +gdb_test "print cbvTwoMCtor (twoMctor)" \
> >>> +    ".* cannot be evaluated .* 'TwoMCtor' is not copy constructible" \
> >>> +    "copy ctor is implicitly deleted"
> >>> +
> >>> +gdb_test "print cbvTwoMCtorAndCCtor (twoMctorAndCctor)" "12" \
> >>> +    "call cbvTwoMCtorAndCCtor"
> >>> +gdb_test "print twoMctorAndCctor.x" "2" \
> >>> +    "cbv argument 'twoMctorAndCtor' should not change"
> >>> +
> >>> +# Test that we get a breakpoint from the cctor during infcall and
> >>> +# we can examine arguments.  This is a test that the dummy frame
> >>> +# of the copy constructor is set up correctly by the infcall mechanism.
> >>> +set bp_location [gdb_get_line_number "ByRef-cctor"]
> >>> +gdb_breakpoint $bp_location
> >>> +gdb_test "print cbvArrayContainerByRef (arrayContainerByRef)" \
> >>> +    ".*The program being debugged stopped.*" \
> >>> +    "call cbvArrayContainerByRef with BP"
> >>> +gdb_test "backtrace" [multi_line \
> >>> +    "#0  ByRef\:\:ByRef .* at .*$srcfile:$bp_location" \
> >>> +    "#1  .* ArrayContainerByRef::ArrayContainerByRef .*" \
> >>> +    "#2  <function called from gdb>" \
> >>> +    "#3  main.*"]
> >>> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.cc b/gdb/testsuite/gdb.cp/pass-by-ref.cc
> >>> deleted file mode 100644
> >>> index bbe450a..0000000
> >>> --- a/gdb/testsuite/gdb.cp/pass-by-ref.cc
> >>> +++ /dev/null
> >>> @@ -1,79 +0,0 @@
> >>> -/* This testcase is part of GDB, the GNU debugger.
> >>> -
> >>> -   Copyright 2007-2019 Free Software Foundation, Inc.
> >>> -
> >>> -   This program is free software; you can redistribute it and/or modify
> >>> -   it under the terms of the GNU General Public License as published by
> >>> -   the Free Software Foundation; either version 3 of the License, or
> >>> -   (at your option) any later version.
> >>> -
> >>> -   This program is distributed in the hope that it will be useful,
> >>> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>> -   GNU General Public License for more details.
> >>> -
> >>> -   You should have received a copy of the GNU General Public License
> >>> -   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >>> -
> >>> -class Obj {
> >>> -public:
> >>> -  Obj ();
> >>> -  Obj (const Obj &);
> >>> -  ~Obj ();
> >>> -  int var[2];
> >>> -};
> >>> -
> >>> -int foo (Obj arg)
> >>> -{
> >>> -  return arg.var[0] + arg.var[1];
> >>> -}
> >>> -
> >>> -Obj::Obj ()
> >>> -{
> >>> -  var[0] = 1;
> >>> -  var[1] = 2;
> >>> -}
> >>> -
> >>> -Obj::Obj (const Obj &obj)
> >>> -{
> >>> -  var[0] = obj.var[0];
> >>> -  var[1] = obj.var[1];
> >>> -}
> >>> -
> >>> -Obj::~Obj ()
> >>> -{
> >>> -
> >>> -}
> >>> -
> >>> -struct Derived : public Obj
> >>> -{
> >>> -  int other;
> >>> -};
> >>> -
> >>> -int blap (Derived arg)
> >>> -{
> >>> -  return foo (arg);
> >>> -}
> >>> -
> >>> -struct Container
> >>> -{
> >>> -  Obj obj;
> >>> -};
> >>> -
> >>> -int blip (Container arg)
> >>> -{
> >>> -  return foo (arg.obj);
> >>> -}
> >>> -
> >>> -Obj global_obj;
> >>> -Derived global_derived;
> >>> -Container global_container;
> >>> -
> >>> -int
> >>> -main ()
> >>> -{
> >>> -  int bar = foo (global_obj);
> >>> -  blap (global_derived);
> >>> -  blip (global_container);
> >>> -  return bar;
> >>> -}
> >>> diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.exp b/gdb/testsuite/gdb.cp/pass-by-ref.exp
> >>> index 94dd345..f44be77 100644
> >>> --- a/gdb/testsuite/gdb.cp/pass-by-ref.exp
> >>> +++ b/gdb/testsuite/gdb.cp/pass-by-ref.exp
> >>> @@ -14,20 +14,395 @@
> >>>    # along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >>>
> >>>    # Check that GDB can call C++ functions whose parameters have
> >>> -# object type, but are passed by reference.
> >>> +# object type, and are either passed by value or implicitly by reference.
> >>> +#
> >>> +# Suppose F is a function that has a call-by-value parameter whose
> >>> +# type is class C.  When calling F with an argument A, a copy of A should
> >>> +# be created and passed to F.  If C is a trivially-copyable type, A can
> >>> +# be copied by a straightforward memory copy.  However, roughly speaking,
> >>> +# if C has a user-defined copy constructor and/or a user-defined
> >>> +# destructor, the copy ctor should be used to initialize the copy of A
> >>> +# before calling F, and a reference to that copy is passed to F.  After
> >>> +# the function returns, the destructor should be called to destruct the
> >>> +# copy.  In this case, C is said to be a 'pass-by-reference' type.
> >>> +# Determining whether C is pass-by-ref depends on
> >>> +# how the copy ctor, destructor, and the move ctor of C are defined.
> >>> +# First of all, C is not copy constructible if its copy constructor is
> >>> +# explicitly or implicitly deleted.  In this case, it would be illegal
> >>> +# to pass values of type C to a function.  C is pass-by-value, if all of
> >>> +# its copy ctor, dtor, and move ctor are trivially defined.
> >>> +# Otherwise, it is pass-by-ref.
> >>> +#
> >>> +# To cover the many possible combinations, this test generates classes
> >>> +# that contain three special functions:
> >>> +#   (1) a copy constructor,
> >>> +#   (2) a destructor, and
> >>> +#   (3) a move constructor.
> >>> +# A special function is in one of the following states:
> >>> +#  * explicit: The function is explicitly defined by the user.
> >>> +#  * defaultedIn: The function is defaulted inside the class decl,
> >>> +#      using the 'default' keyword.
> >>> +#  * defaultedOut: The function is declared inside the class decl,
> >>> +#      and defaulted outside using the 'default' keyword.
> >>> +#  * deleted: The function is explicitly deleted by the user,
> >>> +#      using the 'delete' keyword.
> >>> +#  * absent: The function is not declared by the user (i.e. it does not
> >>> +#      exist in the source.  The compiler generates (or deletes) the
> >>> +#      definition in this case.
> >>> +#
> >>> +# The C++ ABI decides if a class is pass-by-value or pass-by-ref
> >>> +# (i.e.  trivially copyable or not) first at the language level, based
> >>> +# on the state of the special functions.  Then, at the target level, a
> >>> +# class may be determined to be pass-by-ref because of its size
> >>> +# (e.g.  if it is too large to fit on registers).  For this reason, this
> >>> +# test generates both a small and a large version for the same
> >>> +# combination of special function states.
> >>> +#
> >>> +# A class is not trivially-copyable if a base class or a field is not
> >>> +# trivially-copyable, even though the class definition itself seems
> >>> +# trivial.  To test these cases, we also generate derived classes and
> >>> +# container classes.
> >>> +#
> >>> +# The generated code is placed in the test output directory.
> >>> +#
> >>> +# The companion test file pass-by-ref-2.exp also contains
> >>> +# manually-written cases.
> >>>
> >>> -if { [skip_cplus_tests] } { continue }
> >>> +if {[skip_cplus_tests]} {
> >>> +    untested "c++ test skipped"
> >>> +    continue
> >>> +}
> >>>
> >>> +# The program source is generated in the output directory.
> >>> +# We use standard_testfile here to set convenience variables.
> >>>    standard_testfile .cc
> >>>
> >>> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
> >>> +# Some constant values used when generating the source
> >>> +
> >>> +set SMALL    2
> >>> +set LARGE    150
> >>> +set ORIGINAL 2
> >>> +set CUSTOM   3
> >>> +set ADDED    4
> >>> +set TRACE    5
> >>> +
> >>> +
> >>> +# Return 1 if the class whose special function states are STATES
> >>> +# is copyable.  Otherwise return 0.
> >>> +
> >>> +proc is_copy_constructible { states } {
> >>> +    set cctor [lindex $states 0]
> >>> +    set dtor  [lindex $states 1]
> >>> +    set mctor [lindex $states 2]
> >>> +
> >>> +    if {$cctor == "deleted" || ($cctor == "absent" && $mctor != "absent")} {
> >>> +	return 0
> >>> +    }
> >>> +    return 1
> >>> +}
> >>> +
> >>> +# Generate a declaration and an out-of-class definition for a function
> >>> +# with the provided signature.  The STATE should be one of the following:
> >>> +# - explicit, defaultedIn, defaultedOut, deleted, absent
> >>> +
> >>> +proc generate_member_function { classname signature length state } {
> >>> +    set declaration ""
> >>> +    set definition ""
> >>> +
> >>> +    global CUSTOM
> >>> +    global TRACE
> >>> +
> >>> +    switch $state {
> >>> +	explicit {
> >>> +	    set declaration "$signature;\n"
> >>> +	    set definition "$classname\:\:$signature
> >>> +                            {
> >>> +                              data\[0\] = $CUSTOM;
> >>> +                              data\[[expr $length - 1]\] = $CUSTOM;
> >>> +                              tracer = $TRACE;
> >>> +                            }\n"
> >>> +	}
> >>> +	defaultedIn {
> >>> +	    set declaration "$signature = default;\n"
> >>> +	}
> >>> +	defaultedOut {
> >>> +	    set declaration "$signature;\n"
> >>> +	    set definition "$classname\:\:$signature = default;\n"
> >>> +	}
> >>> +	deleted {
> >>> +	    set declaration "$signature = delete;\n"
> >>> +	}
> >>> +	default {
> >>> +	    # function is not user-defined in this case
> >>> +	}
> >>> +    }
> >>> +
> >>> +    return [list $declaration $definition]
> >>> +}
> >>> +
> >>> +# Generate a C++ class with the given CLASSNAME and LENGTH-many
> >>> +# integer elements.  The STATES is an array of 3 items
> >>> +# containing the desired state of the special functions
> >>> +# in this order:
> >>> +# copy constructor, destructor, move constructor
> >>> +
> >>> +proc generate_class { classname length states } {
> >>> +    set declarations ""
> >>> +    set definitions ""
> >>> +    set classname "${classname}_[join $states _]"
> >>> +
> >>> +    for {set i 0} {$i < [llength $states]} {incr i} {
> >>> +	set sig ""
> >>> +	switch $i {
> >>> +	    0 {set sig "$classname (const $classname \&rhs)"}
> >>> +	    1 {set sig "\~$classname (void)"}
> >>> +	    2 {set sig "$classname ($classname \&\&rhs)"}
> >>> +	}
> >>> +
> >>> +	set state [lindex $states $i]
> >>> +	set code [generate_member_function $classname $sig $length $state]
> >>> +	append declarations [lindex $code 0]
> >>> +	append definitions [lindex $code 1]
> >>> +    }
> >>> +
> >>> +    global ORIGINAL
> >>> +
> >>> +    return "
> >>> +    /*** C++ class $classname ***/
> >>> +    class ${classname} {
> >>> +    public:
> >>> +        $classname (void);
> >>> +        $declarations
> >>> +
> >>> +        int data\[$length\];
> >>> +    };
> >>> +
> >>> +    $classname\:\:$classname (void)
> >>> +    {
> >>> +        data\[0\] = $ORIGINAL;
> >>> +        data\[[expr $length - 1]\] = $ORIGINAL;
> >>> +    }
> >>> +
> >>> +    $definitions
> >>> +
> >>> +    $classname ${classname}_var; /* global var */
> >>> +
> >>> +    template int cbv<$classname> ($classname arg);"
> >>> +}
> >>> +
> >>> +# Generate a small C++ class
> >>> +
> >>> +proc generate_small_class { states } {
> >>> +    global SMALL
> >>> +    return [generate_class Small $SMALL $states];
> >>> +}
> >>> +
> >>> +# Generate a large C++ class
> >>> +
> >>> +proc generate_large_class { states } {
> >>> +    global LARGE
> >>> +    return [generate_class Large $LARGE $states];
> >>> +}
> >>> +
> >>> +# Generate a class that derives from a small class
> >>> +
> >>> +proc generate_derived_class { states } {
> >>> +    set base "Small_[join $states _]"
> >>> +    set classname "Derived_[join $states _]"
> >>> +
> >>> +    return "
> >>> +    /*** Class derived from $base ***/
> >>> +    class $classname : public $base {
> >>> +    public:
> >>> +    };
> >>> +
> >>> +    $classname ${classname}_var; /* global var */
> >>> +
> >>> +    template int cbv<$classname> ($classname arg);"
> >>> +}
> >>> +
> >>> +# Generate a class that contains a small class item
> >>> +
> >>> +proc generate_container_class { states } {
> >>> +    set contained "Small_[join $states _]"
> >>> +    set classname "Container_[join $states _]"
> >>> +
> >>> +    return "
> >>> +    /*** Class that contains $contained ***/
> >>> +    class $classname {
> >>> +    public:
> >>> +        $contained item;
> >>> +    };
> >>> +
> >>> +    $classname ${classname}_var; /* global var */
> >>> +
> >>> +    template int cbv_container<$classname> ($classname arg);"
> >>> +}
> >>> +
> >>> +# Generate useful statements that use a class in the debugee program
> >>> +
> >>> +proc generate_stmts { classprefix states {cbvfun "cbv"}} {
> >>> +    set classname "${classprefix}_[join $states _]"
> >>> +
> >>> +    # Having an explicit call to the cbv function in the debugee program
> >>> +    # ensures that the compiler will emit necessary function in the binary.
> >>> +    if {[is_copy_constructible $states]} {
> >>> +	set cbvcall "$cbvfun<$classname> (${classname}_var);\n"
> >>> +    } else {
> >>> +	set cbvcall ""
> >>> +    }
> >>> +
> >>> +    return "$cbvcall"
> >>> +}
> >>> +
> >>> +# Generate the complete debugee program
> >>> +
> >>> +proc generate_program { classes stmts } {
> >>> +    global ADDED
> >>> +
> >>> +    return "
> >>> +    /*** THIS FILE IS GENERATED BY THE TEST.  ***/
> >>> +
> >>> +    static int tracer = 0;
> >>> +
> >>> +    /* The call-by-value function.  */
> >>> +    template <class T>
> >>> +    int
> >>> +    cbv (T arg)
> >>> +    {
> >>> +      arg.data\[0\] += $ADDED; // intentionally modify the arg
> >>> +      return arg.data\[0\];
> >>> +    }
> >>> +
> >>> +    template <class T>
> >>> +    int
> >>> +    cbv_container (T arg)
> >>> +    {
> >>> +      arg.item.data\[0\] += $ADDED;  // intentionally modify
> >>> +      return arg.item.data\[0\];
> >>> +    }
> >>> +
> >>> +    $classes
> >>> +
> >>> +    int
> >>> +    main (void)
> >>> +    {
> >>> +      $stmts
> >>> +
> >>> +      /* stop here */
> >>> +
> >>> +      return 0;
> >>> +    }"
> >>> +}
> >>> +
> >>> +# Compute all the combinations of special function states.
> >>> +# We do not contain the 'deleted' state for the destructor,
> >>> +# because it is illegal to have stack-allocated objects
> >>> +# whose destructor have been deleted.  This case is covered
> >>> +# in pass-by-ref-2 via heap-allocated objects.
> >>> +
> >>> +set options_nodelete [list absent explicit defaultedIn defaultedOut]
> >>> +set options [concat $options_nodelete {deleted}]
> >>> +set all_combinations {}
> >>> +
> >>> +foreach cctor $options {
> >>> +    foreach dtor $options_nodelete {
> >>> +	foreach mctor $options {
> >>> +	    lappend all_combinations [list $cctor $dtor $mctor]
> >>> +	}
> >>> +    }
> >>> +}
> >>> +
> >>> +# Generate the classes.
> >>> +
> >>> +set classes ""
> >>> +set stmts ""
> >>> +
> >>> +foreach state $all_combinations {
> >>> +    append classes [generate_small_class $state]
> >>> +    append stmts [generate_stmts "Small" $state]
> >>> +
> >>> +    append classes [generate_large_class $state]
> >>> +    append stmts [generate_stmts "Large" $state]
> >>> +
> >>> +    append classes [generate_derived_class $state]
> >>> +    append stmts [generate_stmts "Derived" $state]
> >>> +
> >>> +    append classes [generate_container_class $state]
> >>> +    append stmts [generate_stmts "Container" $state "cbv_container"]
> >>> +}
> >>> +
> >>> +# Generate the program code and compile
> >>> +set program [generate_program $classes $stmts]
> >>> +set srcfile [standard_output_file ${srcfile}]
> >>> +gdb_produce_source $srcfile $program
> >>> +
> >>> +set options {debug c++ additional_flags=-std=c++11}
> >>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
> >>>        return -1
> >>>    }
> >>>
> >>> -if ![runto_main] then {
> >>> +if {![runto_main]} {
> >>> +    untested "failed to run to main"
> >>>        return -1
> >>>    }
> >>>
> >>> -gdb_test "print foo (global_obj)" " = 3" "call function in obj"
> >>> -gdb_test "print blap (global_derived)" " = 3" "call function in derived"
> >>> -gdb_test "print blip (global_container)" " = 3" "call function in container"
> >>> +set bp_location [gdb_get_line_number "stop here"]
> >>> +gdb_breakpoint $bp_location
> >>> +gdb_continue_to_breakpoint "end of main" ".*return .*;"
> >>> +
> >>> +# Do the checks for a given class whose name is prefixed with PREFIX,
> >>> +# and whose special functions have the states given in STATES.
> >>> +# The name of the call-by-value function and the expression to access
> >>> +# the data field can be specified explicitly if the default values
> >>> +# do not work.
> >>> +
> >>> +proc test_for_class { prefix states cbvfun data_field length} {
> >>> +    set name "${prefix}_[join $states _]"
> >>> +
> >>> +    set cctor [lindex $states 0]
> >>> +    set dtor  [lindex $states 1]
> >>> +    set mctor [lindex $states 2]
> >>> +
> >>> +    global ORIGINAL
> >>> +    global CUSTOM
> >>> +    global ADDED
> >>> +    global TRACE
> >>> +
> >>> +    with_test_prefix $name {
> >>> +	if {[is_copy_constructible $states]} {
> >>> +	    set expected [expr {$ORIGINAL + $ADDED}]
> >>> +	    if {$cctor == "explicit"} {
> >>> +		set expected [expr {$CUSTOM + $ADDED}]
> >>> +	    }
> >>> +	    if {$dtor == "explicit"} {
> >>> +		gdb_test "print tracer = 0" " = 0" "reset the tracer"
> >>> +	    }
> >>> +	    gdb_test "print ${cbvfun}<$name> (${name}_var)" " = $expected" \
> >>> +		"call '$cbvfun'"
> >>> +	    gdb_test "print ${name}_var.${data_field}\[0\]" " = $ORIGINAL" \
> >>> +		"cbv argument should not change (item 0)"
> >>> +	    if {$length > 1} {
> >>> +		set last_index [expr $length - 1]
> >>> +		gdb_test "print ${name}_var.${data_field}\[$last_index\]" \
> >>> +		    " = $ORIGINAL" \
> >>> +		    "cbv argument should not change (item $last_index)"
> >>> +	    }
> >>> +	    if {$dtor == "explicit"} {
> >>> +		gdb_test "print tracer" " = $TRACE" \
> >>> +		    "destructor should be called"
> >>> +	    }
> >>> +	} else {
> >>> +	    gdb_test "print ${cbvfun}<$name> (${name}_var)" \
> >>> +		".* cannot be evaluated .* '${name}' is not copy constructible" \
> >>> +		"calling '$cbvfun' should be refused"
> >>> +	}
> >>> +    }
> >>> +}
> >>> +
> >>> +foreach state $all_combinations {
> >>> +    test_for_class "Small"     $state "cbv"           "data"      $SMALL
> >>> +    test_for_class "Large"     $state "cbv"           "data"      $LARGE
> >>> +    test_for_class "Derived"   $state "cbv"           "data"      1
> >>> +    test_for_class "Container" $state "cbv_container" "item.data" 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
> >
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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2020-01-13 20:21         ` Aktemur, Tankut Baris
@ 2020-01-14 13:13           ` Aktemur, Tankut Baris
  2020-01-14 13:31             ` Luis Machado
  0 siblings, 1 reply; 25+ messages in thread
From: Aktemur, Tankut Baris @ 2020-01-14 13:13 UTC (permalink / raw)
  To: Luis Machado, tromey, gdb-patches

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



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 .*;"
@@ -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"
-- 
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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2020-01-14 13:13           ` Aktemur, Tankut Baris
@ 2020-01-14 13:31             ` Luis Machado
  2020-01-15 14:41               ` Aktemur, Tankut Baris
  0 siblings, 1 reply; 25+ messages in thread
From: Luis Machado @ 2020-01-14 13:31 UTC (permalink / raw)
  To: Aktemur, Tankut Baris, tromey, gdb-patches

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2020-01-14 13:31             ` Luis Machado
@ 2020-01-15 14:41               ` Aktemur, Tankut Baris
  2020-01-28 12:30                 ` Aktemur, Tankut Baris
  0 siblings, 1 reply; 25+ messages in thread
From: Aktemur, Tankut Baris @ 2020-01-15 14:41 UTC (permalink / raw)
  To: Luis Machado, tromey, gdb-patches

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

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2020-01-15 14:41               ` Aktemur, Tankut Baris
@ 2020-01-28 12:30                 ` Aktemur, Tankut Baris
  2020-01-29  3:18                   ` Simon Marchi
  0 siblings, 1 reply; 25+ messages in thread
From: Aktemur, Tankut Baris @ 2020-01-28 12:30 UTC (permalink / raw)
  To: Luis Machado, tromey, gdb-patches

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(+)

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2020-01-28 12:30                 ` Aktemur, Tankut Baris
@ 2020-01-29  3:18                   ` Simon Marchi
  2020-01-29 10:54                     ` Aktemur, Tankut Baris
  2020-01-29 11:08                     ` Aktemur, Tankut Baris
  0 siblings, 2 replies; 25+ messages in thread
From: Simon Marchi @ 2020-01-29  3:18 UTC (permalink / raw)
  To: Aktemur, Tankut Baris, Luis Machado, tromey, gdb-patches

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2020-01-29  3:18                   ` Simon Marchi
@ 2020-01-29 10:54                     ` Aktemur, Tankut Baris
  2020-01-29 11:08                     ` Aktemur, Tankut Baris
  1 sibling, 0 replies; 25+ messages in thread
From: Aktemur, Tankut Baris @ 2020-01-29 10:54 UTC (permalink / raw)
  To: Simon Marchi, Luis Machado, tromey, gdb-patches

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [review v3] testsuite, cp: increase the coverage of testing pass-by-ref arguments
  2020-01-29  3:18                   ` Simon Marchi
  2020-01-29 10:54                     ` Aktemur, Tankut Baris
@ 2020-01-29 11:08                     ` Aktemur, Tankut Baris
  1 sibling, 0 replies; 25+ messages in thread
From: Aktemur, Tankut Baris @ 2020-01-29 11:08 UTC (permalink / raw)
  To: Simon Marchi, Luis Machado, tromey, gdb-patches

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2020-01-29 10:54 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-18 13:53 [review] testsuite, cp: increase the coverage of testing pass-by-ref arguments Tankut Baris Aktemur (Code Review)
2019-10-28 22:06 ` Tom Tromey (Code Review)
2019-10-31 13:19 ` Tankut Baris Aktemur (Code Review)
2019-11-01 15:18   ` Tom Tromey
2019-11-08  8:08     ` Aktemur, Tankut Baris
2019-12-09 17:45 ` [review v2] " Tankut Baris Aktemur (Code Review)
2019-12-09 17:56 ` Tankut Baris Aktemur (Code Review)
2019-12-13 21:38 ` Tom Tromey (Code Review)
2019-12-14  9:53 ` [review v3] " Tankut Baris Aktemur (Code Review)
2020-01-13 18:58   ` Luis Machado
2020-01-13 19:38     ` Aktemur, Tankut Baris
2020-01-13 19:40       ` Luis Machado
2020-01-13 20:21         ` Aktemur, Tankut Baris
2020-01-14 13:13           ` Aktemur, Tankut Baris
2020-01-14 13:31             ` Luis Machado
2020-01-15 14:41               ` Aktemur, Tankut Baris
2020-01-28 12:30                 ` Aktemur, Tankut Baris
2020-01-29  3:18                   ` Simon Marchi
2020-01-29 10:54                     ` Aktemur, Tankut Baris
2020-01-29 11:08                     ` Aktemur, Tankut Baris
2019-12-14  9:56 ` Tankut Baris Aktemur (Code Review)
2019-12-20  7:29 ` Tankut Baris Aktemur (Code Review)
2019-12-20 15:54 ` Tom Tromey (Code Review)
2019-12-20 16:47 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-12-20 16:47 ` Sourceware to Gerrit sync (Code Review)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).