public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Tankut Baris Aktemur (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: gdb-patches@sourceware.org
Subject: [review] infcall: handle pass-by-reference arguments appropriately
Date: Fri, 18 Oct 2019 13:53:00 -0000	[thread overview]
Message-ID: <gerrit.1571406803000.I18fa5d0df814dfa0defe9e862a88a6dbf1d99d01@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1571406803000.I18fa5d0df814dfa0defe9e862a88a6dbf1d99d01@gnutoolchain-gerrit.osci.io>

Tankut Baris Aktemur has uploaded a new change for review.

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

infcall: handle pass-by-reference arguments appropriately

If an aggregate argument is implicitly pass-by-reference, allocate a
temporary object on the stack, initialize it via the copy constructor
(if exists) or trivially by memcpy'ing.  Pass the reference of the
temporary to the callee function.  After the callee returns, invoke
the destructor of the temporary.

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

	* infcall.c (call_function_by_hand_dummy): Update the argument-
	passing section for call-by-value parameters.
	(struct destructor_info): New struct.
	(call_destructors): New auxiliary function.

Change-Id: I18fa5d0df814dfa0defe9e862a88a6dbf1d99d01
---
M gdb/infcall.c
1 file changed, 100 insertions(+), 3 deletions(-)



diff --git a/gdb/infcall.c b/gdb/infcall.c
index 288b9e0..701df4d 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -42,6 +42,7 @@
 #include "thread-fsm.h"
 #include <algorithm>
 #include "gdbsupport/scope-exit.h"
+#include <list>
 
 /* If we can't find a function's name from its address,
    we print this instead.  */
@@ -704,6 +705,33 @@
   return addr;
 }
 
+/* The data structure which keeps a destructor function and
+   its implicit 'this' parameter.  */
+
+struct destructor_info
+{
+  destructor_info (struct value *function, struct value *self)
+    : function (function), self (self) { }
+
+  struct value *function;
+  struct value *self;
+};
+
+
+/* Auxiliary function that takes a list of destructor functions
+   with their 'this' parameters, and invokes the functions.  */
+
+static void
+call_destructors (const std::list<destructor_info> &dtors_to_invoke,
+		  struct type *default_return_type)
+{
+  for (auto vals : dtors_to_invoke)
+    {
+      call_function_by_hand (vals.function, default_return_type,
+			     gdb::make_array_view (&(vals.self), 1));
+    }
+}
+
 /* See infcall.h.  */
 
 struct value *
@@ -983,6 +1011,12 @@
       internal_error (__FILE__, __LINE__, _("bad switch"));
     }
 
+  /* Coerce the arguments and handle pass-by-reference.
+     We want to remember the destruction required for pass-by-ref values.
+     For these, store the dtor function and the 'this' argument
+     in DTORS_TO_INVOKE.  */
+  std::list<destructor_info> dtors_to_invoke;
+
   for (int i = args.size () - 1; i >= 0; i--)
     {
       int prototyped;
@@ -1020,9 +1054,68 @@
       args[i] = value_arg_coerce (gdbarch, args[i],
 				  param_type, prototyped);
 
-      if (param_type != NULL
-	  && !(language_pass_by_reference (param_type).trivially_copyable))
-	args[i] = value_addr (args[i]);
+      if (param_type == NULL)
+	continue;
+
+      auto info = language_pass_by_reference (param_type);
+      if (!info.copy_constructible)
+	error (_("expression cannot be evaluated because the type '%s' "
+		 "is not copy constructible"), TYPE_NAME (param_type));
+
+      if (!info.destructible)
+	error (_("expression cannot be evaluated because the type '%s' "
+		 "is not destructible"), TYPE_NAME (param_type));
+
+      if (info.trivially_copyable)
+	continue;
+
+      /* This is a pass-by-ref value.  Check for error cases before
+	 pushing the temporary to the stack.  */
+
+      if (!info.trivially_copy_constructible && info.cctor_name == NULL)
+	error (_("evaluation of this expression requires a copy constructor"
+		 " for the type '%s'."), TYPE_NAME (param_type));
+
+      if (!info.trivially_destructible && info.dtor_name == NULL)
+	error (_("evaluation of this expression requires a destructor"
+		 " for the type '%s'."), TYPE_NAME (param_type));
+
+      /* Make a copy of the argument on the stack.  If the argument is
+	 trivially copy ctor'able, copy bit by bit.  Otherwise, call
+	 the copy ctor to initialize the clone.  */
+      CORE_ADDR addr = reserve_stack_space (param_type, sp);
+      struct value *clone
+	= value_from_contents_and_address (param_type, NULL, addr);
+      push_thread_stack_temporary (call_thread.get (), clone);
+      struct value *clone_ptr
+	= value_from_pointer (lookup_pointer_type (param_type), addr);
+
+      if (info.trivially_copy_constructible)
+	{
+	  int length = TYPE_LENGTH (param_type);
+	  write_memory (addr, value_contents (args[i]), length);
+	}
+      else
+	{
+	  struct value *copy_ctor
+	    = find_function_in_inferior (info.cctor_name, 0);
+	  struct value *cctor_args[2] = { clone_ptr, args[i] };
+	  call_function_by_hand (copy_ctor, default_return_type,
+				 gdb::make_array_view (cctor_args, 2));
+	}
+
+      /* If the argument has a destructor, remember it so that we
+	 invoke it after the infcall is complete.  */
+      if (!info.trivially_destructible)
+	{
+	  struct value *dtor
+	    = find_function_in_inferior (info.dtor_name, 0);
+	  /* Insert the dtor to the front of the list to call them
+	     in reverse order later.  */
+	  dtors_to_invoke.emplace_front (dtor, clone_ptr);
+	}
+
+      args[i] = clone_ptr;
     }
 
   /* Reserve space for the return structure to be written on the
@@ -1189,6 +1282,10 @@
 	    maybe_remove_breakpoints ();
 
 	    gdb_assert (retval != NULL);
+
+	    /* Destruct the pass-by-ref argument clones.  */
+	    call_destructors (dtors_to_invoke, default_return_type);
+
 	    return retval;
 	  }
 

       reply	other threads:[~2019-10-18 13:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-18 13:53 Tankut Baris Aktemur (Code Review) [this message]
2019-10-29 19:54 ` Tom Tromey (Code Review)
2019-12-09 17:45 ` [review v2] " Tankut Baris Aktemur (Code Review)
2019-12-09 17:59 ` Tankut Baris Aktemur (Code Review)
2019-12-13 21:35 ` 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)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=gerrit.1571406803000.I18fa5d0df814dfa0defe9e862a88a6dbf1d99d01@gnutoolchain-gerrit.osci.io \
    --to=gerrit@gnutoolchain-gerrit.osci.io \
    --cc=gdb-patches@sourceware.org \
    --cc=tankut.baris.aktemur@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).