RE: [PATCH 0/8] Fix inferior call for C++ pass-by-reference arguments

["Aktemur, Tankut Baris" <tankut dot baris dot aktemur at intel dot com>]

> Subject: [PATCH 0/8] Fix inferior call for C++ pass-by-reference arguments
> 
> Dear All,
> 
> This is a set of patches to fix GDB's conformance to C++ ABI when
> invoking functions that have call-by-value arguments of aggregate
> types that should be implicitly passed by reference.


Hello,

I'd like to ping the review request for this patch, available also at
https://sourceware.org/ml/gdb-patches/2019-04/msg00461.html

So far, Patch 1/8 received a review (thank you, Andrew Burgess).

Regards,
-Baris

 
> To give some background information first, suppose we have a class 'K'
> and a function 'cbv' that has a call-by-value parameter of type 'K'.
> 
> 	class K {
> 	public:
> 		K (const K &other) { ... }
> 		~K (void) { ... }
> 		...
> 	};
> 
> 	int
> 	cbv (K k)
> 	{
> 		... use k ...
> 	}
> 
> 	int
> 	main (void)
> 	{
> 		K q1;
> 		...
> 		cbv (q1);
> 		...
> 	}
> 
> When making a function call such as 'cbv(q1)', the caller needs to
> create a (temporary) copy of the argument 'q1', say 'q2', and pass
> 'q2' to 'cbv' as the argument.  After the function call returns, the
> caller can discard 'q2'.  However, because the type 'K' has a
> user-defined constructor and destructor, the caller cannot simply copy
> the contents of 'q1' to 'q2'; it has to initialize 'q2' via the copy
> constructor.  Similarly, 'q2' has to be destroyed by invoking the
> user-defined destructor.  For this reason, according to the C++ ABI,
> the caller first allocates a space for 'q2', initializes it using the
> copy constructor, and then passes the *address* of 'q2' to 'cbv'.  The
> caller uses the same address to destruct 'q2' after the function call
> is complete.  Although 'k' is declared as a call-by-value parameter,
> it is implicitly *pass-by-reference*.
> 
> Had the type 'K' not have a user-defined copy constructor or a
> destructor, the caller could have cloned 'q1' to 'q2' by a
> straightforward memory copying operation.  Such types are trivially
> copyable.  In that case, 'K' would be a *pass-by-value* type.
> 
> Present-day GDB has problems regarding pass-by-reference arguments
> when making inferior calls.  In particular:
> 
>   - Relatively new DWARF attributes are not used for inferring whether
>     a type is pass-by-reference or not.  This leads to incorrect
>     deductions and, in return, crashing inferior calls.
> 
>   - For a pass-by-reference argument, a copy of the argument is not
>     made; the address of the argument itself is passed to the callee.
>     Hence, no copy-constructor call or a destructor call takes place,
>     as well.
> 
> The proposed patch aims to fix these problems.  Tested on the X86_64
> architecture with GCC 7.4.0 and 8.2.0.  ChangeLog entries will be
> included in the patch when/if the patch is accepted.
> 
> Best regards.
> 
> 
> Tankut Baris Aktemur (8):
>   gdb: recognize new DWARF attributes: defaulted, deleted, calling conv.
>   infcall, c++: allow more info to be computed for pass-by-reference
>     values
>   infcall, c++: collect more pass-by-reference information
>   infcall: refactor 'call_function_by_hand_dummy'
>   infcall: move assertions in 'call_function_by_hand_dummy' to an
>     earlier spot
>   infcall: remove unused parameter in 'value_arg_coerce'
>   infcall: handle pass-by-reference arguments appropriately
>   testsuite, cp: increase the coverage of testing pass-by-ref arguments
> 
>  gdb/arch-utils.c                       |    2 +-
>  gdb/cp-abi.c                           |    6 +-
>  gdb/cp-abi.h                           |   10 +-
>  gdb/dwarf2read.c                       |   20 +
>  gdb/gdbtypes.c                         |    7 +
>  gdb/gdbtypes.h                         |   19 +-
>  gdb/gnu-v3-abi.c                       |  268 +-
>  gdb/infcall.c                          |  213 +-
>  gdb/language.c                         |   27 +-
>  gdb/language.h                         |   55 +-
>  gdb/testsuite/gdb.cp/pass-by-ref-2.cc  |  179 +
>  gdb/testsuite/gdb.cp/pass-by-ref-2.exp |   85 +
>  gdb/testsuite/gdb.cp/pass-by-ref.cc    | 7368 +++++++++++++++++++++++-
>  gdb/testsuite/gdb.cp/pass-by-ref.exp   |  412 +-
>  gdb/tic6x-tdep.c                       |    2 +-
>  15 files changed, 8514 insertions(+), 159 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.cp/pass-by-ref-2.cc
>  create mode 100644 gdb/testsuite/gdb.cp/pass-by-ref-2.exp
> 
> --
> 2.21.0

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