public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/9] Fix "finish" with variably-sized types
@ 2022-10-07 18:01 Tom Tromey
  2022-10-07 18:01 ` [PATCH 1/9] Fix crash in amd64-tdep.c Tom Tromey
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Tom Tromey @ 2022-10-07 18:01 UTC (permalink / raw)
  To: gdb-patches

The "finish" command does not correctly work with variably-sized
types.  This affects some Ada programs; I'm not sure whether it can
affect programs written in other languages.

This series fixes the problem, for the architectures I could test.
Patch #2 explains this.

Regression tested on x86-64 Fedora 34.  I also ran this through the
AdaCore internal test suite, on all the architectures that are touched
by this series.

Tom



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

* [PATCH 1/9] Fix crash in amd64-tdep.c
  2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
@ 2022-10-07 18:01 ` Tom Tromey
  2022-10-07 18:01 ` [PATCH 2/9] Add new overload of gdbarch_return_value Tom Tromey
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-10-07 18:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

amd64-tdep.c could crash when 'finish'ing from a function whose return
type had variable length.  In this situation, the value will be passed
by reference, and this patch avoids the crash.

(Note that this does not fully fix the bug reported, but it does fix
the crash, so it seems worthwhile to land independently.)
---
 gdb/amd64-tdep.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index ea2b3b1ecc3..22d69c85387 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -666,7 +666,8 @@ amd64_classify_aggregate (struct type *type, enum amd64_reg_class theclass[2])
 	loc_bitpos attributes, which will cause an assert to trigger within
 	the unaligned field check.  As classes with virtual bases are not
 	trivially copyable, checking that first avoids this problem.  */
-  if (type->length () > 16
+  if (TYPE_HAS_DYNAMIC_LENGTH (type)
+      || type->length () > 16
       || !language_pass_by_reference (type).trivially_copyable
       || amd64_has_unaligned_fields (type))
     {
-- 
2.34.3


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

* [PATCH 2/9] Add new overload of gdbarch_return_value
  2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
  2022-10-07 18:01 ` [PATCH 1/9] Fix crash in amd64-tdep.c Tom Tromey
@ 2022-10-07 18:01 ` Tom Tromey
  2022-10-07 18:01 ` [PATCH 3/9] Don't emit gdbarch_return_value Tom Tromey
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-10-07 18:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The gdbarch "return_value" can't correctly handle variably-sized
types.  The problem here is that the TYPE_LENGTH of such a type is 0,
until the type is resolved, which requires reading memory.  However,
gdbarch_return_value only accepts a buffer as an out parameter.

Fixing this requires letting the implementation of the gdbarch method
resolve the type and return a value -- that is, both the contents and
the new type.

After an attempt at this, I realized I wouldn't be able to correctly
update all implementations (there are ~80) of this method.  So,
instead, this patch adds a new method that falls back to the current
method, and it updates gdb to only call the new method.  This way it's
possible to incrementally convert the architectures that I am able to
test.
---
 gdb/arch-utils.c          | 18 ++++++++++++++++++
 gdb/arch-utils.h          |  6 ++++++
 gdb/elfread.c             |  4 ++--
 gdb/gdbarch-components.py | 36 ++++++++++++++++++++++++++++++++++--
 gdb/gdbarch-gen.h         | 21 +++++++++++++++++++--
 gdb/gdbarch.c             | 36 +++++++++++++++++++++++++-----------
 gdb/infcall.c             |  7 +++----
 gdb/infcmd.c              |  9 ++++-----
 gdb/stack.c               |  7 ++++---
 gdb/value.c               |  4 ++--
 10 files changed, 117 insertions(+), 31 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 77269425ddc..677e405fd6c 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1092,6 +1092,24 @@ default_read_core_file_mappings
 {
 }
 
+enum return_value_convention
+default_gdbarch_return_value
+     (struct gdbarch *gdbarch, struct value *function, struct type *valtype,
+      struct regcache *regcache, struct value **read_value,
+      const gdb_byte *writebuf)
+{
+  gdb_byte *readbuf = nullptr;
+
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (valtype);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
+
+  return gdbarch_return_value (gdbarch, function, valtype, regcache,
+			       readbuf, writebuf);
+}
+
 /* Non-zero if we want to trace architecture code.  */
 
 #ifndef GDBARCH_DEBUG
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index f850e5fd6e7..2e1f5a561f6 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -300,4 +300,10 @@ extern void default_read_core_file_mappings
    struct bfd *cbfd,
    read_core_file_mappings_pre_loop_ftype pre_loop_cb,
    read_core_file_mappings_loop_ftype loop_cb);
+
+extern enum return_value_convention default_gdbarch_return_value
+     (struct gdbarch *gdbarch, struct value *function, struct type *valtype,
+      struct regcache *regcache, struct value **read_value,
+      const gdb_byte *writebuf);
+
 #endif /* ARCH_UTILS_H */
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 8aee634b44b..a6c7ead9558 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1018,8 +1018,8 @@ elf_gnu_ifunc_resolver_return_stop (code_breakpoint *b)
   set_value_address (func_func, b->loc->related_address);
 
   value = allocate_value (value_type);
-  gdbarch_return_value (gdbarch, func_func, value_type, regcache,
-			value_contents_raw (value).data (), NULL);
+  gdbarch_return_value_as_value (gdbarch, func_func, value_type, regcache,
+				 &value, NULL);
   resolved_address = value_as_address (value);
   resolved_pc = gdbarch_convert_from_func_ptr_addr
     (gdbarch, resolved_address, current_inferior ()->top_target ());
diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
index b8d7648d5ec..844eb4f161d 100644
--- a/gdb/gdbarch-components.py
+++ b/gdb/gdbarch-components.py
@@ -854,6 +854,9 @@ If WRITEBUF is not NULL, it contains a return value which will be
 stored into the appropriate register.  This can be used when we want
 to force the value returned by a function (see the "return" command
 for instance).
+
+NOTE: it is better to implement return_value_as_value instead, as that
+method can properly handle variably-sized types.
 """,
     type="enum return_value_convention",
     name="return_value",
@@ -864,8 +867,37 @@ for instance).
         ("gdb_byte *", "readbuf"),
         ("const gdb_byte *", "writebuf"),
     ],
-    predicate=True,
-    invalid=True,
+    invalid=False,
+)
+
+Method(
+    comment="""
+Return the return-value convention that will be used by FUNCTION
+to return a value of type VALTYPE.  FUNCTION may be NULL in which
+case the return convention is computed based only on VALTYPE.
+
+If READ_VALUE is not NULL, extract the return value and save it in
+this pointer.
+
+If WRITEBUF is not NULL, it contains a return value which will be
+stored into the appropriate register.  This can be used when we want
+to force the value returned by a function (see the "return" command
+for instance).
+""",
+    type="enum return_value_convention",
+    name="return_value_as_value",
+    params=[
+        ("struct value *", "function"),
+        ("struct type *", "valtype"),
+        ("struct regcache *", "regcache"),
+        ("struct value **", "read_value"),
+        ("const gdb_byte *", "writebuf"),
+    ],
+    predefault="default_gdbarch_return_value",
+    # If we're using the default, then the other method must be set;
+    # but if we aren't using the default here then the other method
+    # must not be set.
+    invalid="(gdbarch->return_value_as_value == default_gdbarch_return_value) == (gdbarch->return_value == nullptr)",
 )
 
 Method(
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index 383d84f8142..4ec11fd4eee 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -433,14 +433,31 @@ extern void set_gdbarch_integer_to_address (struct gdbarch *gdbarch, gdbarch_int
    If WRITEBUF is not NULL, it contains a return value which will be
    stored into the appropriate register.  This can be used when we want
    to force the value returned by a function (see the "return" command
-   for instance). */
+   for instance).
 
-extern bool gdbarch_return_value_p (struct gdbarch *gdbarch);
+   NOTE: it is better to implement return_value_as_value instead, as that
+   method can properly handle variably-sized types. */
 
 typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf);
 extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf);
 extern void set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch_return_value_ftype *return_value);
 
+/* Return the return-value convention that will be used by FUNCTION
+   to return a value of type VALTYPE.  FUNCTION may be NULL in which
+   case the return convention is computed based only on VALTYPE.
+
+   If READ_VALUE is not NULL, extract the return value and save it in
+   this pointer.
+
+   If WRITEBUF is not NULL, it contains a return value which will be
+   stored into the appropriate register.  This can be used when we want
+   to force the value returned by a function (see the "return" command
+   for instance). */
+
+typedef enum return_value_convention (gdbarch_return_value_as_value_ftype) (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, struct value **read_value, const gdb_byte *writebuf);
+extern enum return_value_convention gdbarch_return_value_as_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, struct value **read_value, const gdb_byte *writebuf);
+extern void set_gdbarch_return_value_as_value (struct gdbarch *gdbarch, gdbarch_return_value_as_value_ftype *return_value_as_value);
+
 /* Return true if the return value of function is stored in the first hidden
    parameter.  In theory, this feature should be language-dependent, specified
    by language and its ABI, such as C++.  Unfortunately, compiler may
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 4b0c10be402..911aa0ec277 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -116,6 +116,7 @@ struct gdbarch
   gdbarch_address_to_pointer_ftype *address_to_pointer = nullptr;
   gdbarch_integer_to_address_ftype *integer_to_address = nullptr;
   gdbarch_return_value_ftype *return_value = nullptr;
+  gdbarch_return_value_as_value_ftype *return_value_as_value = nullptr;
   gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = nullptr;
   gdbarch_skip_prologue_ftype *skip_prologue = nullptr;
   gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr;
@@ -313,6 +314,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->value_from_register = default_value_from_register;
   gdbarch->pointer_to_address = unsigned_pointer_to_address;
   gdbarch->address_to_pointer = unsigned_address_to_pointer;
+  gdbarch->return_value_as_value = default_gdbarch_return_value;
   gdbarch->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p;
   gdbarch->breakpoint_from_pc = default_breakpoint_from_pc;
   gdbarch->sw_breakpoint_from_kind = NULL;
@@ -464,7 +466,9 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of pointer_to_address, invalid_p == 0 */
   /* Skip verify of address_to_pointer, invalid_p == 0 */
   /* Skip verify of integer_to_address, has predicate.  */
-  /* Skip verify of return_value, has predicate.  */
+  /* Skip verify of return_value, invalid_p == 0 */
+  if ((gdbarch->return_value_as_value == default_gdbarch_return_value) == (gdbarch->return_value == nullptr))
+    log.puts ("\n\treturn_value_as_value");
   /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */
   if (gdbarch->skip_prologue == 0)
     log.puts ("\n\tskip_prologue");
@@ -871,12 +875,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   gdb_printf (file,
                       "gdbarch_dump: integer_to_address = <%s>\n",
                       host_address_to_string (gdbarch->integer_to_address));
-  gdb_printf (file,
-                      "gdbarch_dump: gdbarch_return_value_p() = %d\n",
-                      gdbarch_return_value_p (gdbarch));
   gdb_printf (file,
                       "gdbarch_dump: return_value = <%s>\n",
                       host_address_to_string (gdbarch->return_value));
+  gdb_printf (file,
+                      "gdbarch_dump: return_value_as_value = <%s>\n",
+                      host_address_to_string (gdbarch->return_value_as_value));
   gdb_printf (file,
                       "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n",
                       host_address_to_string (gdbarch->return_in_first_hidden_param_p));
@@ -2660,13 +2664,6 @@ set_gdbarch_integer_to_address (struct gdbarch *gdbarch,
   gdbarch->integer_to_address = integer_to_address;
 }
 
-bool
-gdbarch_return_value_p (struct gdbarch *gdbarch)
-{
-  gdb_assert (gdbarch != NULL);
-  return gdbarch->return_value != NULL;
-}
-
 enum return_value_convention
 gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf)
 {
@@ -2684,6 +2681,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch,
   gdbarch->return_value = return_value;
 }
 
+enum return_value_convention
+gdbarch_return_value_as_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, struct value **read_value, const gdb_byte *writebuf)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->return_value_as_value != NULL);
+  if (gdbarch_debug >= 2)
+    gdb_printf (gdb_stdlog, "gdbarch_return_value_as_value called\n");
+  return gdbarch->return_value_as_value (gdbarch, function, valtype, regcache, read_value, writebuf);
+}
+
+void
+set_gdbarch_return_value_as_value (struct gdbarch *gdbarch,
+                                   gdbarch_return_value_as_value_ftype return_value_as_value)
+{
+  gdbarch->return_value_as_value = return_value_as_value;
+}
+
 int
 gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type)
 {
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 34852191043..21270ef48d3 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -457,10 +457,9 @@ get_call_return_value (struct call_return_meta_info *ri)
     }
   else
     {
-      retval = allocate_value (ri->value_type);
-      gdbarch_return_value (ri->gdbarch, ri->function, ri->value_type,
-			    get_current_regcache (),
-			    value_contents_raw (retval).data (), NULL);
+      gdbarch_return_value_as_value (ri->gdbarch, ri->function, ri->value_type,
+				     get_current_regcache (),
+				     &retval, NULL);
       if (stack_temporaries && class_or_union_p (ri->value_type))
 	{
 	  /* Values of class type returned in registers are copied onto
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index decd61111b7..b66d5e73784 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1485,15 +1485,14 @@ get_return_value (struct symbol *func_symbol, struct value *function)
      inferior function call code.  In fact, when inferior function
      calls are made async, this will likely be made the norm.  */
 
-  switch (gdbarch_return_value (gdbarch, function, value_type,
-				NULL, NULL, NULL))
+  switch (gdbarch_return_value_as_value (gdbarch, function, value_type,
+					 NULL, NULL, NULL))
     {
     case RETURN_VALUE_REGISTER_CONVENTION:
     case RETURN_VALUE_ABI_RETURNS_ADDRESS:
     case RETURN_VALUE_ABI_PRESERVES_ADDRESS:
-      value = allocate_value (value_type);
-      gdbarch_return_value (gdbarch, function, value_type, stop_regs,
-			    value_contents_raw (value).data (), NULL);
+      gdbarch_return_value_as_value (gdbarch, function, value_type, stop_regs,
+				     &value, NULL);
       break;
     case RETURN_VALUE_STRUCT_CONVENTION:
       value = NULL;
diff --git a/gdb/stack.c b/gdb/stack.c
index 379635e409e..f2b70d20f4d 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2835,9 +2835,10 @@ return_command (const char *retval_exp, int from_tty)
 
       gdb_assert (rv_conv != RETURN_VALUE_STRUCT_CONVENTION
 		  && rv_conv != RETURN_VALUE_ABI_RETURNS_ADDRESS);
-      gdbarch_return_value (cache_arch, function, return_type,
-			    get_current_regcache (), NULL /*read*/,
-			    value_contents (return_value).data () /*write*/);
+      gdbarch_return_value_as_value
+	(cache_arch, function, return_type,
+	 get_current_regcache (), NULL /*read*/,
+	 value_contents (return_value).data () /*write*/);
     }
 
   /* If we are at the end of a call dummy now, pop the dummy frame
diff --git a/gdb/value.c b/gdb/value.c
index 8ed941f3749..e79022e740c 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3878,8 +3878,8 @@ struct_return_convention (struct gdbarch *gdbarch,
     error (_("Function return type unknown."));
 
   /* Probe the architecture for the return-value convention.  */
-  return gdbarch_return_value (gdbarch, function, value_type,
-			       NULL, NULL, NULL);
+  return gdbarch_return_value_as_value (gdbarch, function, value_type,
+					NULL, NULL, NULL);
 }
 
 /* Return true if the function returning the specified type is using
-- 
2.34.3


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

* [PATCH 3/9] Don't emit gdbarch_return_value
  2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
  2022-10-07 18:01 ` [PATCH 1/9] Fix crash in amd64-tdep.c Tom Tromey
  2022-10-07 18:01 ` [PATCH 2/9] Add new overload of gdbarch_return_value Tom Tromey
@ 2022-10-07 18:01 ` Tom Tromey
  2022-10-07 18:01 ` [PATCH 4/9] Introduce value_at_non_lval Tom Tromey
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-10-07 18:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The previous patch introduced a new overload of gdbarch_return_value.
The intent here is that this new overload always be called by the core
of gdb -- the previous implementation is effectively deprecated,
because a call to the old-style method will not work with any
converted architectures (whereas calling the new-style method is will
delegate when needed).

This patch changes gdbarch.py so that the old gdbarch_return_value
wrapper function can be omitted.  This will prevent any errors from
creeping in.
---
 gdb/arch-utils.c          | 36 ++++++++++----------
 gdb/gdbarch-components.py |  6 ++++
 gdb/gdbarch-gen.h         |  1 -
 gdb/gdbarch.c             | 10 ------
 gdb/gdbarch.py            | 70 +++++++++++++++++++++------------------
 5 files changed, 61 insertions(+), 62 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 677e405fd6c..cce0bf809f5 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1092,24 +1092,6 @@ default_read_core_file_mappings
 {
 }
 
-enum return_value_convention
-default_gdbarch_return_value
-     (struct gdbarch *gdbarch, struct value *function, struct type *valtype,
-      struct regcache *regcache, struct value **read_value,
-      const gdb_byte *writebuf)
-{
-  gdb_byte *readbuf = nullptr;
-
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (valtype);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
-  return gdbarch_return_value (gdbarch, function, valtype, regcache,
-			       readbuf, writebuf);
-}
-
 /* Non-zero if we want to trace architecture code.  */
 
 #ifndef GDBARCH_DEBUG
@@ -1180,6 +1162,24 @@ pstring_list (const char *const *list)
 
 #include "gdbarch.c"
 
+enum return_value_convention
+default_gdbarch_return_value
+     (struct gdbarch *gdbarch, struct value *function, struct type *valtype,
+      struct regcache *regcache, struct value **read_value,
+      const gdb_byte *writebuf)
+{
+  gdb_byte *readbuf = nullptr;
+
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (valtype);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
+
+  return gdbarch->return_value (gdbarch, function, valtype, regcache,
+				readbuf, writebuf);
+}
+
 obstack *gdbarch_obstack (gdbarch *arch)
 {
   return &arch->obstack;
diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
index 844eb4f161d..3b0f62e334d 100644
--- a/gdb/gdbarch-components.py
+++ b/gdb/gdbarch-components.py
@@ -113,6 +113,9 @@
 # 'result' can be used to reference the result of the function/method
 # implementation.  The 'result_checks' can only be used if the 'type'
 # of this Function/Method is not 'void'.
+#
+# * "implement" - optional, a boolean.  If True (the default), a
+# wrapper function for this function will be emitted.
 
 Info(
     type="const struct bfd_arch_info *",
@@ -868,6 +871,9 @@ method can properly handle variably-sized types.
         ("const gdb_byte *", "writebuf"),
     ],
     invalid=False,
+    # We don't want to accidentally introduce calls to this, as gdb
+    # should only ever call return_value_new (see below).
+    implement=False,
 )
 
 Method(
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index 4ec11fd4eee..059865e86f7 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -439,7 +439,6 @@ extern void set_gdbarch_integer_to_address (struct gdbarch *gdbarch, gdbarch_int
    method can properly handle variably-sized types. */
 
 typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf);
-extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf);
 extern void set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch_return_value_ftype *return_value);
 
 /* Return the return-value convention that will be used by FUNCTION
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 911aa0ec277..3c7e99e0874 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -2664,16 +2664,6 @@ set_gdbarch_integer_to_address (struct gdbarch *gdbarch,
   gdbarch->integer_to_address = integer_to_address;
 }
 
-enum return_value_convention
-gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf)
-{
-  gdb_assert (gdbarch != NULL);
-  gdb_assert (gdbarch->return_value != NULL);
-  if (gdbarch_debug >= 2)
-    gdb_printf (gdb_stdlog, "gdbarch_return_value called\n");
-  return gdbarch->return_value (gdbarch, function, valtype, regcache, readbuf, writebuf);
-}
-
 void
 set_gdbarch_return_value (struct gdbarch *gdbarch,
                           gdbarch_return_value_ftype return_value)
diff --git a/gdb/gdbarch.py b/gdb/gdbarch.py
index da848feae84..df01dd11c05 100755
--- a/gdb/gdbarch.py
+++ b/gdb/gdbarch.py
@@ -119,6 +119,7 @@ class Function(_Component):
         printer=None,
         param_checks=None,
         result_checks=None,
+        implement=True,
     ):
         super().__init__(
             comment=comment,
@@ -132,6 +133,7 @@ class Function(_Component):
             params=params,
             param_checks=param_checks,
             result_checks=result_checks,
+            implement=implement,
         )
 
     def ftype(self):
@@ -246,10 +248,11 @@ with open("gdbarch-gen.h", "w") as f:
                 f"typedef {c.type} ({c.ftype()}) ({c.param_list()});",
                 file=f,
             )
-            print(
-                f"extern {c.type} gdbarch_{c.name} ({c.set_list()});",
-                file=f,
-            )
+            if c.implement:
+                print(
+                    f"extern {c.type} gdbarch_{c.name} ({c.set_list()});",
+                    file=f,
+                )
             print(
                 f"extern void set_gdbarch_{c.name} (struct gdbarch *gdbarch, {c.ftype()} *{c.name});",
                 file=f,
@@ -441,38 +444,39 @@ with open("gdbarch.c", "w") as f:
             print(f"  return {c.get_predicate()};", file=f)
             print("}", file=f)
         if isinstance(c, Function):
-            print(file=f)
-            print(f"{c.type}", file=f)
-            print(f"gdbarch_{c.name} ({c.set_list()})", file=f)
-            print("{", file=f)
-            print("  gdb_assert (gdbarch != NULL);", file=f)
-            print(f"  gdb_assert (gdbarch->{c.name} != NULL);", file=f)
-            if c.predicate and c.predefault:
-                # Allow a call to a function with a predicate.
+            if c.implement:
+                print(file=f)
+                print(f"{c.type}", file=f)
+                print(f"gdbarch_{c.name} ({c.set_list()})", file=f)
+                print("{", file=f)
+                print("  gdb_assert (gdbarch != NULL);", file=f)
+                print(f"  gdb_assert (gdbarch->{c.name} != NULL);", file=f)
+                if c.predicate and c.predefault:
+                    # Allow a call to a function with a predicate.
+                    print(
+                        f"  /* Do not check predicate: {c.get_predicate()}, allow call.  */",
+                        file=f,
+                    )
+                if c.param_checks:
+                    for rule in c.param_checks:
+                        print(f"  gdb_assert ({rule});", file=f)
+                print("  if (gdbarch_debug >= 2)", file=f)
                 print(
-                    f"  /* Do not check predicate: {c.get_predicate()}, allow call.  */",
+                    f"""    gdb_printf (gdb_stdlog, "gdbarch_{c.name} called\\n");""",
                     file=f,
                 )
-            if c.param_checks:
-                for rule in c.param_checks:
-                    print(f"  gdb_assert ({rule});", file=f)
-            print("  if (gdbarch_debug >= 2)", file=f)
-            print(
-                f"""    gdb_printf (gdb_stdlog, "gdbarch_{c.name} called\\n");""",
-                file=f,
-            )
-            print("  ", file=f, end="")
-            if c.type != "void":
-                if c.result_checks:
-                    print("auto result = ", file=f, end="")
-                else:
-                    print("return ", file=f, end="")
-            print(f"gdbarch->{c.name} ({c.actuals()});", file=f)
-            if c.type != "void" and c.result_checks:
-                for rule in c.result_checks:
-                    print(f"  gdb_assert ({rule});", file=f)
-                print("  return result;", file=f)
-            print("}", file=f)
+                print("  ", file=f, end="")
+                if c.type != "void":
+                    if c.result_checks:
+                        print("auto result = ", file=f, end="")
+                    else:
+                        print("return ", file=f, end="")
+                print(f"gdbarch->{c.name} ({c.actuals()});", file=f)
+                if c.type != "void" and c.result_checks:
+                    for rule in c.result_checks:
+                        print(f"  gdb_assert ({rule});", file=f)
+                    print("  return result;", file=f)
+                print("}", file=f)
             print(file=f)
             print("void", file=f)
             print(f"set_gdbarch_{c.name} (struct gdbarch *gdbarch,", file=f)
-- 
2.34.3


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

* [PATCH 4/9] Introduce value_at_non_lval
  2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
                   ` (2 preceding siblings ...)
  2022-10-07 18:01 ` [PATCH 3/9] Don't emit gdbarch_return_value Tom Tromey
@ 2022-10-07 18:01 ` Tom Tromey
  2022-10-07 18:01 ` [PATCH 5/9] Don't let property evaluation affect the current language Tom Tromey
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-10-07 18:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

In some cases, while a value might be read from memory, gdb should not
record the value as being equivalent to that memory.

In Ada, the inferior call code will call ada_convert_actual -- and
here, if the argument is already in memory, that address will simply
be reused.  However, for a call like "f(g())", the result of "g" might
be on the stack and thus overwritten by the call to "f".

This patch introduces a new function that is like value_at but that
ensures that the result is non-lvalue.
---
 gdb/valops.c | 10 ++++++++++
 gdb/value.h  |  4 ++++
 2 files changed, 14 insertions(+)

diff --git a/gdb/valops.c b/gdb/valops.c
index de8a68888e4..be4ce025c89 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1018,6 +1018,16 @@ value_at (struct type *type, CORE_ADDR addr)
   return get_value_at (type, addr, 0);
 }
 
+/* See value.h.  */
+
+struct value *
+value_at_non_lval (struct type *type, CORE_ADDR addr)
+{
+  struct value *result = value_at (type, addr);
+  VALUE_LVAL (result) = not_lval;
+  return result;
+}
+
 /* Return a lazy value with type TYPE located at ADDR (cf. value_at).
    The type of the created value may differ from the passed type TYPE.
    Make sure to retrieve the returned values's new type after this call
diff --git a/gdb/value.h b/gdb/value.h
index 52752df1f4c..86e06a54d7d 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -690,6 +690,10 @@ extern struct value *value_from_component (struct value *, struct type *,
 extern struct value *value_at (struct type *type, CORE_ADDR addr);
 extern struct value *value_at_lazy (struct type *type, CORE_ADDR addr);
 
+/* Like value_at, but ensures that the result is marked not_lval.
+   This can be important if the memory is "volatile".  */
+extern struct value *value_at_non_lval (struct type *type, CORE_ADDR addr);
+
 extern struct value *value_from_contents_and_address_unresolved
      (struct type *, const gdb_byte *, CORE_ADDR);
 extern struct value *value_from_contents_and_address (struct type *,
-- 
2.34.3


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

* [PATCH 5/9] Don't let property evaluation affect the current language
  2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
                   ` (3 preceding siblings ...)
  2022-10-07 18:01 ` [PATCH 4/9] Introduce value_at_non_lval Tom Tromey
@ 2022-10-07 18:01 ` Tom Tromey
  2022-10-07 18:01 ` [PATCH 6/9] Convert selected architectures to gdbarch_return_value_as_value Tom Tromey
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-10-07 18:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

On PPC, we saw that calling an inferior function could sometimes
change the current language, because gdb would select the call dummy
frame -- associated with _start.

This patch changes gdb so that the current language is never affected
by DWARF property evaluation.
---
 gdb/dwarf2/loc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index ad45d57a654..3333bf438ff 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -1647,6 +1647,11 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
   if (prop == NULL)
     return false;
 
+  /* Evaluating a property should not change the current language.
+     Without this here this could happen if the code below selects a
+     frame.  */
+  scoped_restore_current_language save_language;
+
   if (frame == NULL && has_stack_frames ())
     frame = get_selected_frame (NULL);
 
-- 
2.34.3


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

* [PATCH 6/9] Convert selected architectures to gdbarch_return_value_as_value
  2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
                   ` (4 preceding siblings ...)
  2022-10-07 18:01 ` [PATCH 5/9] Don't let property evaluation affect the current language Tom Tromey
@ 2022-10-07 18:01 ` Tom Tromey
  2022-10-07 18:01 ` [PATCH 7/9] Fix inferior calls with variably-sized return type Tom Tromey
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-10-07 18:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This converts a few selected architectures to use
gdbarch_return_value_as_value rather than gdbarch_return_value.  The
architectures are just the ones that I am able to test.  This patch
should not introduce any behavior changes.
---
 gdb/aarch64-tdep.c       | 10 ++++++++--
 gdb/amd64-tdep.c         | 13 ++++++++++---
 gdb/amd64-windows-tdep.c | 11 +++++++++--
 gdb/arm-tdep.c           | 11 +++++++++--
 gdb/i386-tdep.c          | 21 ++++++++++++++++-----
 gdb/ppc-linux-tdep.c     | 12 ++++++++++--
 gdb/riscv-tdep.c         | 11 +++++++++--
 gdb/sparc-tdep.c         | 11 +++++++++--
 gdb/sparc64-tdep.c       |  1 +
 9 files changed, 81 insertions(+), 20 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 78bf1225e97..6d24f5ab495 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2447,8 +2447,14 @@ aarch64_store_return_value (struct type *type, struct regcache *regs,
 static enum return_value_convention
 aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
 		      struct type *valtype, struct regcache *regcache,
-		      gdb_byte *readbuf, const gdb_byte *writebuf)
+		      struct value **read_value, const gdb_byte *writebuf)
 {
+  gdb_byte *readbuf = nullptr;
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (valtype);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
 
   if (valtype->code () == TYPE_CODE_STRUCT
       || valtype->code () == TYPE_CODE_UNION
@@ -3704,7 +3710,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, aarch64_dwarf_reg_to_regnum);
 
   /* Returning results.  */
-  set_gdbarch_return_value (gdbarch, aarch64_return_value);
+  set_gdbarch_return_value_as_value (gdbarch, aarch64_return_value);
 
   /* Disassembly.  */
   set_gdbarch_print_insn (gdbarch, aarch64_gdb_print_insn);
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 22d69c85387..c250c07e4ce 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -789,7 +789,7 @@ amd64_classify (struct type *type, enum amd64_reg_class theclass[2])
 static enum return_value_convention
 amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 		    struct type *type, struct regcache *regcache,
-		    gdb_byte *readbuf, const gdb_byte *writebuf)
+		    struct value **read_value, const gdb_byte *writebuf)
 {
   enum amd64_reg_class theclass[2];
   int len = type->length ();
@@ -799,7 +799,14 @@ amd64_return_value (struct gdbarch *gdbarch, struct value *function,
   int sse_reg = 0;
   int i;
 
-  gdb_assert (!(readbuf && writebuf));
+  gdb_assert (!(read_value && writebuf));
+
+  gdb_byte *readbuf = nullptr;
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
 
   /* 1. Classify the return type with the classification algorithm.  */
   amd64_classify (type, theclass);
@@ -3236,7 +3243,7 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
   set_gdbarch_register_to_value (gdbarch, i387_register_to_value);
   set_gdbarch_value_to_register (gdbarch, i387_value_to_register);
 
-  set_gdbarch_return_value (gdbarch, amd64_return_value);
+  set_gdbarch_return_value_as_value (gdbarch, amd64_return_value);
 
   set_gdbarch_skip_prologue (gdbarch, amd64_skip_prologue);
 
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 8573e4c0676..05a5990f1e0 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -355,11 +355,18 @@ amd64_windows_push_dummy_call
 static enum return_value_convention
 amd64_windows_return_value (struct gdbarch *gdbarch, struct value *function,
 			    struct type *type, struct regcache *regcache,
-			    gdb_byte *readbuf, const gdb_byte *writebuf)
+			    struct value **read_value, const gdb_byte *writebuf)
 {
   int len = type->length ();
   int regnum = -1;
 
+  gdb_byte *readbuf = nullptr;
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
+
   /* See if our value is returned through a register.  If it is, then
      store the associated register number in REGNUM.  */
   switch (type->code ())
@@ -1297,7 +1304,7 @@ amd64_windows_init_abi_common (gdbarch_info info, struct gdbarch *gdbarch)
 
   /* Function calls.  */
   set_gdbarch_push_dummy_call (gdbarch, amd64_windows_push_dummy_call);
-  set_gdbarch_return_value (gdbarch, amd64_windows_return_value);
+  set_gdbarch_return_value_as_value (gdbarch, amd64_windows_return_value);
   set_gdbarch_skip_main_prologue (gdbarch, amd64_skip_main_prologue);
   set_gdbarch_skip_trampoline_code (gdbarch,
 				    amd64_windows_skip_trampoline_code);
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index d357066653b..132c10b1ca6 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9008,8 +9008,15 @@ arm_store_return_value (struct type *type, struct regcache *regs,
 static enum return_value_convention
 arm_return_value (struct gdbarch *gdbarch, struct value *function,
 		  struct type *valtype, struct regcache *regcache,
-		  gdb_byte *readbuf, const gdb_byte *writebuf)
+		  struct value **read_value, const gdb_byte *writebuf)
 {
+  gdb_byte *readbuf = nullptr;
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (valtype);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
+
   arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
   struct type *func_type = function ? value_type (function) : NULL;
   enum arm_vfp_cprc_base_type vfp_base_type;
@@ -10546,7 +10553,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_register_name (gdbarch, arm_register_name);
 
   /* Returning results.  */
-  set_gdbarch_return_value (gdbarch, arm_return_value);
+  set_gdbarch_return_value_as_value (gdbarch, arm_return_value);
 
   /* Disassembly.  */
   set_gdbarch_print_insn (gdbarch, gdb_print_insn_arm);
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a2f0017612b..e609b7c833e 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3032,10 +3032,17 @@ i386_reg_struct_return_p (struct gdbarch *gdbarch, struct type *type)
 static enum return_value_convention
 i386_return_value (struct gdbarch *gdbarch, struct value *function,
 		   struct type *type, struct regcache *regcache,
-		   gdb_byte *readbuf, const gdb_byte *writebuf)
+		   struct value **read_value, const gdb_byte *writebuf)
 {
   enum type_code code = type->code ();
 
+  gdb_byte *readbuf = nullptr;
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
+
   if (((code == TYPE_CODE_STRUCT
 	|| code == TYPE_CODE_UNION
 	|| code == TYPE_CODE_ARRAY)
@@ -3083,9 +3090,13 @@ i386_return_value (struct gdbarch *gdbarch, struct value *function,
      here.  */
   if (code == TYPE_CODE_STRUCT && type->num_fields () == 1)
     {
-      type = check_typedef (type->field (0).type ());
-      return i386_return_value (gdbarch, function, type, regcache,
-				readbuf, writebuf);
+      struct type *inner_type = check_typedef (type->field (0).type ());
+      enum return_value_convention result
+	= i386_return_value (gdbarch, function, inner_type, regcache,
+			     read_value, writebuf);
+      if (read_value != nullptr)
+	deprecated_set_value_type (*read_value, type);
+      return result;
     }
 
   if (readbuf)
@@ -8574,7 +8585,7 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_register_to_value (gdbarch,  i386_register_to_value);
   set_gdbarch_value_to_register (gdbarch, i386_value_to_register);
 
-  set_gdbarch_return_value (gdbarch, i386_return_value);
+  set_gdbarch_return_value_as_value (gdbarch, i386_return_value);
 
   set_gdbarch_skip_prologue (gdbarch, i386_skip_prologue);
 
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 96eb931743f..2ea838acef1 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -249,8 +249,15 @@ ppc_linux_memory_remove_breakpoint (struct gdbarch *gdbarch,
 static enum return_value_convention
 ppc_linux_return_value (struct gdbarch *gdbarch, struct value *function,
 			struct type *valtype, struct regcache *regcache,
-			gdb_byte *readbuf, const gdb_byte *writebuf)
+			struct value **read_value, const gdb_byte *writebuf)
 {  
+  gdb_byte *readbuf = nullptr;
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (valtype);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
+
   if ((valtype->code () == TYPE_CODE_STRUCT
        || valtype->code () == TYPE_CODE_UNION)
       && !((valtype->length () == 16 || valtype->length () == 8)
@@ -2094,7 +2101,8 @@ ppc_linux_init_abi (struct gdbarch_info info,
 	 (well ignoring vectors that is).  When this was corrected, it
 	 wasn't fixed for GNU/Linux native platform.  Use the
 	 PowerOpen struct convention.  */
-      set_gdbarch_return_value (gdbarch, ppc_linux_return_value);
+      set_gdbarch_return_value_as_value (gdbarch, ppc_linux_return_value);
+      set_gdbarch_return_value (gdbarch, nullptr);
 
       set_gdbarch_memory_remove_breakpoint (gdbarch,
 					    ppc_linux_memory_remove_breakpoint);
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index feca17d9141..ed92ff67c7b 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -3221,13 +3221,20 @@ riscv_return_value (struct gdbarch  *gdbarch,
 		    struct value *function,
 		    struct type *type,
 		    struct regcache *regcache,
-		    gdb_byte *readbuf,
+		    struct value **read_value,
 		    const gdb_byte *writebuf)
 {
   struct riscv_call_info call_info (gdbarch);
   struct riscv_arg_info info;
   struct type *arg_type;
 
+  gdb_byte *readbuf = nullptr;
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
+
   arg_type = check_typedef (type);
   riscv_arg_location (gdbarch, &info, &call_info, arg_type, false);
 
@@ -3889,7 +3896,7 @@ riscv_gdbarch_init (struct gdbarch_info info,
   set_gdbarch_type_align (gdbarch, riscv_type_align);
 
   /* Information about the target architecture.  */
-  set_gdbarch_return_value (gdbarch, riscv_return_value);
+  set_gdbarch_return_value_as_value (gdbarch, riscv_return_value);
   set_gdbarch_breakpoint_kind_from_pc (gdbarch, riscv_breakpoint_kind_from_pc);
   set_gdbarch_sw_breakpoint_from_kind (gdbarch, riscv_sw_breakpoint_from_kind);
   set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index 320f898eb98..bbb98ef5ed7 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1497,10 +1497,17 @@ sparc32_store_return_value (struct type *type, struct regcache *regcache,
 static enum return_value_convention
 sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
 		      struct type *type, struct regcache *regcache,
-		      gdb_byte *readbuf, const gdb_byte *writebuf)
+		      struct value **read_value, const gdb_byte *writebuf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
+  gdb_byte *readbuf = nullptr;
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
+
   /* The psABI says that "...every stack frame reserves the word at
      %fp+64.  If a function returns a structure, union, or
      quad-precision value, this word should hold the address of the
@@ -1854,7 +1861,7 @@ sparc32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_push_dummy_code (gdbarch, sparc32_push_dummy_code);
   set_gdbarch_push_dummy_call (gdbarch, sparc32_push_dummy_call);
 
-  set_gdbarch_return_value (gdbarch, sparc32_return_value);
+  set_gdbarch_return_value_as_value (gdbarch, sparc32_return_value);
   set_gdbarch_stabs_argument_has_addr
     (gdbarch, sparc32_stabs_argument_has_addr);
 
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 6b9d9eaa957..8cd0b2df82b 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -1838,6 +1838,7 @@ sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_push_dummy_call (gdbarch, sparc64_push_dummy_call);
 
   set_gdbarch_return_value (gdbarch, sparc64_return_value);
+  set_gdbarch_return_value_as_value (gdbarch, default_gdbarch_return_value);
   set_gdbarch_stabs_argument_has_addr
     (gdbarch, default_stabs_argument_has_addr);
 
-- 
2.34.3


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

* [PATCH 7/9] Fix inferior calls with variably-sized return type
  2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
                   ` (5 preceding siblings ...)
  2022-10-07 18:01 ` [PATCH 6/9] Convert selected architectures to gdbarch_return_value_as_value Tom Tromey
@ 2022-10-07 18:01 ` Tom Tromey
  2022-10-07 18:01 ` [PATCH 8/9] Use value_at_non_lval in get_call_return_value Tom Tromey
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-10-07 18:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This patch updates the gdbarch_return_value_as_value implementations
to work correctly with variably-sized return types.
---
 gdb/aarch64-tdep.c       | 22 +++++++++---------
 gdb/amd64-tdep.c         | 18 +++++++--------
 gdb/amd64-windows-tdep.c | 19 +++++++--------
 gdb/arm-tdep.c           | 30 +++++++++++++++---------
 gdb/i386-tdep.c          | 22 ++++++++----------
 gdb/riscv-tdep.c         | 50 +++++++++++++++++++++-------------------
 gdb/sparc-tdep.c         | 19 +++++++--------
 7 files changed, 91 insertions(+), 89 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 6d24f5ab495..1ba3228a037 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2335,6 +2335,9 @@ aarch64_return_in_memory (struct gdbarch *gdbarch, struct type *type)
   int elements;
   struct type *fundamental_type;
 
+  if (TYPE_HAS_DYNAMIC_LENGTH (type))
+    return 1;
+
   if (aapcs_is_vfp_call_or_return_candidate (type, &elements,
 					     &fundamental_type))
     {
@@ -2449,13 +2452,6 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
 		      struct type *valtype, struct regcache *regcache,
 		      struct value **read_value, const gdb_byte *writebuf)
 {
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (valtype);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   if (valtype->code () == TYPE_CODE_STRUCT
       || valtype->code () == TYPE_CODE_UNION
       || valtype->code () == TYPE_CODE_ARRAY)
@@ -2471,12 +2467,12 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
 
 	  aarch64_debug_printf ("return value in memory");
 
-	  if (readbuf)
+	  if (read_value != nullptr)
 	    {
 	      CORE_ADDR addr;
 
 	      regcache->cooked_read (AARCH64_STRUCT_RETURN_REGNUM, &addr);
-	      read_memory (addr, readbuf, valtype->length ());
+	      *read_value = value_at_non_lval (valtype, addr);
 	    }
 
 	  return RETURN_VALUE_ABI_RETURNS_ADDRESS;
@@ -2486,8 +2482,12 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
   if (writebuf)
     aarch64_store_return_value (valtype, regcache, writebuf);
 
-  if (readbuf)
-    aarch64_extract_return_value (valtype, regcache, readbuf);
+  if (read_value)
+    {
+      *read_value = allocate_value (valtype);
+      aarch64_extract_return_value (valtype, regcache,
+				    value_contents_raw (*read_value).data ());
+    }
 
   aarch64_debug_printf ("return value in registers");
 
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index c250c07e4ce..e6f9f3eca32 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -801,13 +801,6 @@ amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 
   gdb_assert (!(read_value && writebuf));
 
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (type);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   /* 1. Classify the return type with the classification algorithm.  */
   amd64_classify (type, theclass);
 
@@ -824,17 +817,24 @@ amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 	 can always find the return value just after the function has
 	 returned.  */
 
-      if (readbuf)
+      if (read_value != nullptr)
 	{
 	  ULONGEST addr;
 
 	  regcache_raw_read_unsigned (regcache, AMD64_RAX_REGNUM, &addr);
-	  read_memory (addr, readbuf, type->length ());
+	  *read_value = value_at_non_lval (type, addr);
 	}
 
       return RETURN_VALUE_ABI_RETURNS_ADDRESS;
     }
 
+  gdb_byte *readbuf = nullptr;
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
+
   /* 8. If the class is COMPLEX_X87, the real part of the value is
 	returned in %st0 and the imaginary part in %st1.  */
   if (theclass[0] == AMD64_COMPLEX_X87)
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 05a5990f1e0..1d366417075 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -360,13 +360,6 @@ amd64_windows_return_value (struct gdbarch *gdbarch, struct value *function,
   int len = type->length ();
   int regnum = -1;
 
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (type);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   /* See if our value is returned through a register.  If it is, then
      store the associated register number in REGNUM.  */
   switch (type->code ())
@@ -401,20 +394,24 @@ amd64_windows_return_value (struct gdbarch *gdbarch, struct value *function,
   if (regnum < 0)
     {
       /* RAX contains the address where the return value has been stored.  */
-      if (readbuf)
+      if (read_value != nullptr)
 	{
 	  ULONGEST addr;
 
 	  regcache_raw_read_unsigned (regcache, AMD64_RAX_REGNUM, &addr);
-	  read_memory (addr, readbuf, type->length ());
+	  *read_value = value_at_non_lval (type, addr);
 	}
       return RETURN_VALUE_ABI_RETURNS_ADDRESS;
     }
   else
     {
       /* Extract the return value from the register where it was stored.  */
-      if (readbuf)
-	regcache->raw_read_part (regnum, 0, len, readbuf);
+      if (read_value != nullptr)
+	{
+	  *read_value = allocate_value (type);
+	  regcache->raw_read_part (regnum, 0, len,
+				   value_contents_raw (*read_value).data ());
+	}
       if (writebuf)
 	regcache->raw_write_part (regnum, 0, len, writebuf);
       return RETURN_VALUE_REGISTER_CONVENTION;
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 132c10b1ca6..edc11253627 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8808,6 +8808,9 @@ arm_return_in_memory (struct gdbarch *gdbarch, struct type *type)
       && TYPE_CODE_ARRAY != code && TYPE_CODE_COMPLEX != code)
     return 0;
 
+  if (TYPE_HAS_DYNAMIC_LENGTH (type))
+    return 1;
+
   if (TYPE_CODE_ARRAY == code && type->is_vector ())
     {
       /* Vector values should be returned using ARM registers if they
@@ -9010,13 +9013,6 @@ arm_return_value (struct gdbarch *gdbarch, struct value *function,
 		  struct type *valtype, struct regcache *regcache,
 		  struct value **read_value, const gdb_byte *writebuf)
 {
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (valtype);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
   struct type *func_type = function ? value_type (function) : NULL;
   enum arm_vfp_cprc_base_type vfp_base_type;
@@ -9028,6 +9024,14 @@ arm_return_value (struct gdbarch *gdbarch, struct value *function,
       int reg_char = arm_vfp_cprc_reg_char (vfp_base_type);
       int unit_length = arm_vfp_cprc_unit_length (vfp_base_type);
       int i;
+
+      gdb_byte *readbuf = nullptr;
+      if (read_value != nullptr)
+	{
+	  *read_value = allocate_value (valtype);
+	  readbuf = value_contents_raw (*read_value).data ();
+	}
+
       for (i = 0; i < vfp_base_count; i++)
 	{
 	  if (reg_char == 'q')
@@ -9079,12 +9083,12 @@ arm_return_value (struct gdbarch *gdbarch, struct value *function,
       if (tdep->struct_return == pcc_struct_return
 	  || arm_return_in_memory (gdbarch, valtype))
 	{
-	  if (readbuf)
+	  if (read_value != nullptr)
 	    {
 	      CORE_ADDR addr;
 
 	      regcache->cooked_read (ARM_A1_REGNUM, &addr);
-	      read_memory (addr, readbuf, valtype->length ());
+	      *read_value = value_at_non_lval (valtype, addr);
 	    }
 	  return RETURN_VALUE_ABI_RETURNS_ADDRESS;
 	}
@@ -9098,8 +9102,12 @@ arm_return_value (struct gdbarch *gdbarch, struct value *function,
   if (writebuf)
     arm_store_return_value (valtype, regcache, writebuf);
 
-  if (readbuf)
-    arm_extract_return_value (valtype, regcache, readbuf);
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (valtype);
+      gdb_byte *readbuf = value_contents_raw (*read_value).data ();
+      arm_extract_return_value (valtype, regcache, readbuf);
+    }
 
   return RETURN_VALUE_REGISTER_CONVENTION;
 }
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index e609b7c833e..ebc6ebd7d8b 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3008,7 +3008,8 @@ i386_reg_struct_return_p (struct gdbarch *gdbarch, struct type *type)
 
   if (struct_convention == pcc_struct_convention
       || (struct_convention == default_struct_convention
-	  && tdep->struct_return == pcc_struct_return))
+	  && tdep->struct_return == pcc_struct_return)
+      || TYPE_HAS_DYNAMIC_LENGTH (type))
     return 0;
 
   /* Structures consisting of a single `float', `double' or 'long
@@ -3036,13 +3037,6 @@ i386_return_value (struct gdbarch *gdbarch, struct value *function,
 {
   enum type_code code = type->code ();
 
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (type);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   if (((code == TYPE_CODE_STRUCT
 	|| code == TYPE_CODE_UNION
 	|| code == TYPE_CODE_ARRAY)
@@ -3070,12 +3064,12 @@ i386_return_value (struct gdbarch *gdbarch, struct value *function,
 	 a record, so the convention applied to records also applies
 	 to arrays.  */
 
-      if (readbuf)
+      if (read_value != nullptr)
 	{
 	  ULONGEST addr;
 
 	  regcache_raw_read_unsigned (regcache, I386_EAX_REGNUM, &addr);
-	  read_memory (addr, readbuf, type->length ());
+	  *read_value = value_at_non_lval (type, addr);
 	}
 
       return RETURN_VALUE_ABI_RETURNS_ADDRESS;
@@ -3099,8 +3093,12 @@ i386_return_value (struct gdbarch *gdbarch, struct value *function,
       return result;
     }
 
-  if (readbuf)
-    i386_extract_return_value (gdbarch, type, regcache, readbuf);
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      i386_extract_return_value (gdbarch, type, regcache,
+				 value_contents_raw (*read_value).data ());
+    }
   if (writebuf)
     i386_store_return_value (gdbarch, type, regcache, writebuf);
 
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index ed92ff67c7b..26a5e91c74f 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -2492,7 +2492,8 @@ static void
 riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo,
 			   struct riscv_call_info *cinfo)
 {
-  if (ainfo->length > (2 * cinfo->xlen))
+  if (TYPE_HAS_DYNAMIC_LENGTH (ainfo->type)
+      || ainfo->length > (2 * cinfo->xlen))
     {
       /* Argument is going to be passed by reference.  */
       ainfo->argloc[0].loc_type
@@ -2910,8 +2911,12 @@ riscv_arg_location (struct gdbarch *gdbarch,
       break;
 
     case TYPE_CODE_STRUCT:
-      riscv_call_arg_struct (ainfo, cinfo);
-      break;
+      if (!TYPE_HAS_DYNAMIC_LENGTH (ainfo->type))
+	{
+	  riscv_call_arg_struct (ainfo, cinfo);
+	  break;
+	}
+      /* FALLTHROUGH */
 
     default:
       riscv_call_arg_scalar_int (ainfo, cinfo);
@@ -3228,13 +3233,6 @@ riscv_return_value (struct gdbarch  *gdbarch,
   struct riscv_arg_info info;
   struct type *arg_type;
 
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (type);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   arg_type = check_typedef (type);
   riscv_arg_location (gdbarch, &info, &call_info, arg_type, false);
 
@@ -3246,15 +3244,15 @@ riscv_return_value (struct gdbarch  *gdbarch,
       gdb_printf (gdb_stdlog, "\n");
     }
 
-  if (readbuf != nullptr || writebuf != nullptr)
+  if (read_value != nullptr || writebuf != nullptr)
     {
       unsigned int arg_len;
       struct value *abi_val;
-      gdb_byte *old_readbuf = nullptr;
+      gdb_byte *readbuf = nullptr;
       int regnum;
 
       /* We only do one thing at a time.  */
-      gdb_assert (readbuf == nullptr || writebuf == nullptr);
+      gdb_assert (read_value == nullptr || writebuf == nullptr);
 
       /* In some cases the argument is not returned as the declared type,
 	 and we need to cast to or from the ABI type in order to
@@ -3295,7 +3293,6 @@ riscv_return_value (struct gdbarch  *gdbarch,
       else
 	{
 	  abi_val = allocate_value (info.type);
-	  old_readbuf = readbuf;
 	  readbuf = value_contents_raw (abi_val).data ();
 	}
       arg_len = info.type->length ();
@@ -3375,8 +3372,17 @@ riscv_return_value (struct gdbarch  *gdbarch,
 
 	    regcache_cooked_read_unsigned (regcache, RISCV_A0_REGNUM,
 					   &addr);
-	    if (readbuf != nullptr)
-	      read_memory (addr, readbuf, info.length);
+	    if (read_value != nullptr)
+	      {
+		abi_val = value_at_non_lval (type, addr);
+		/* Also reset the expected type, so that the cast
+		   later on is a no-op.  If the cast is not a no-op,
+		   and if the return type is variably-sized, then the
+		   type of ABI_VAL will differ from ARG_TYPE due to
+		   dynamic type resolution, and so will most likely
+		   fail.  */
+		arg_type = value_type (abi_val);
+	      }
 	    if (writebuf != nullptr)
 	      write_memory (addr, writebuf, info.length);
 	  }
@@ -3391,10 +3397,8 @@ riscv_return_value (struct gdbarch  *gdbarch,
       /* This completes the cast from abi type back to the declared type
 	 in the case that we are reading from the machine.  See the
 	 comment at the head of this block for more details.  */
-      if (readbuf != nullptr)
+      if (read_value != nullptr)
 	{
-	  struct value *arg_val;
-
 	  if (is_fixed_point_type (arg_type))
 	    {
 	      /* Convert abi_val to the actual return type, but
@@ -3405,15 +3409,13 @@ riscv_return_value (struct gdbarch  *gdbarch,
 	      unscaled.read (value_contents (abi_val),
 			     type_byte_order (info.type),
 			     info.type->is_unsigned ());
-	      arg_val = allocate_value (arg_type);
-	      unscaled.write (value_contents_raw (arg_val),
+	      *read_value = allocate_value (arg_type);
+	      unscaled.write (value_contents_raw (*read_value),
 			      type_byte_order (arg_type),
 			      arg_type->is_unsigned ());
 	    }
 	  else
-	    arg_val = value_cast (arg_type, abi_val);
-	  memcpy (old_readbuf, value_contents_raw (arg_val).data (),
-		  arg_type->length ());
+	    *read_value = value_cast (arg_type, abi_val);
 	}
     }
 
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index bbb98ef5ed7..c2d1ed15273 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1501,13 +1501,6 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (type);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   /* The psABI says that "...every stack frame reserves the word at
      %fp+64.  If a function returns a structure, union, or
      quad-precision value, this word should hold the address of the
@@ -1520,11 +1513,11 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
       ULONGEST sp;
       CORE_ADDR addr;
 
-      if (readbuf)
+      if (read_value != nullptr)
 	{
 	  regcache_cooked_read_unsigned (regcache, SPARC_SP_REGNUM, &sp);
 	  addr = read_memory_unsigned_integer (sp + 64, 4, byte_order);
-	  read_memory (addr, readbuf, type->length ());
+	  *read_value = value_at_non_lval (type, addr);
 	}
       if (writebuf)
 	{
@@ -1536,8 +1529,12 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
       return RETURN_VALUE_ABI_PRESERVES_ADDRESS;
     }
 
-  if (readbuf)
-    sparc32_extract_return_value (type, regcache, readbuf);
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      gdb_byte *readbuf = value_contents_raw (*read_value).data ();
+      sparc32_extract_return_value (type, regcache, readbuf);
+    }
   if (writebuf)
     sparc32_store_return_value (type, regcache, writebuf);
 
-- 
2.34.3


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

* [PATCH 8/9] Use value_at_non_lval in get_call_return_value
  2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
                   ` (6 preceding siblings ...)
  2022-10-07 18:01 ` [PATCH 7/9] Fix inferior calls with variably-sized return type Tom Tromey
@ 2022-10-07 18:01 ` Tom Tromey
  2022-10-07 18:01 ` [PATCH 9/9] Add test case for "finish" with variably-sized types Tom Tromey
  2023-01-03 16:44 ` [PATCH 0/9] Fix " Tom Tromey
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-10-07 18:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

get_call_return_value can handle RETURN_VALUE_STRUCT_CONVENTION,
because the call is completely managed by gdb.  However, it does not
handle variably-sized types correctly.  The simplest way to fix this
is to use value_at_non_lval, which does type resolution.
---
 gdb/infcall.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/gdb/infcall.c b/gdb/infcall.c
index 21270ef48d3..9b679d930e9 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -448,12 +448,7 @@ get_call_return_value (struct call_return_meta_info *ri)
 	  push_thread_stack_temporary (thr, retval);
 	}
       else
-	{
-	  retval = allocate_value (ri->value_type);
-	  read_value_memory (retval, 0, 1, ri->struct_addr,
-			     value_contents_raw (retval).data (),
-			     ri->value_type->length ());
-	}
+	retval = value_at_non_lval (ri->value_type, ri->struct_addr);
     }
   else
     {
-- 
2.34.3


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

* [PATCH 9/9] Add test case for "finish" with variably-sized types
  2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
                   ` (7 preceding siblings ...)
  2022-10-07 18:01 ` [PATCH 8/9] Use value_at_non_lval in get_call_return_value Tom Tromey
@ 2022-10-07 18:01 ` Tom Tromey
  2023-01-03 16:44 ` [PATCH 0/9] Fix " Tom Tromey
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-10-07 18:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This adds a test case for "finish" with variably-sized types, and for
inferior calls as well.  This also extends the "runto" proc to handle
temporary breakpoints.
---
 gdb/testsuite/gdb.ada/finish-var-size.exp     | 43 +++++++++++++++++++
 gdb/testsuite/gdb.ada/finish-var-size/p.adb   | 21 +++++++++
 gdb/testsuite/gdb.ada/finish-var-size/pck.adb | 21 +++++++++
 gdb/testsuite/gdb.ada/finish-var-size/pck.ads | 36 ++++++++++++++++
 gdb/testsuite/lib/gdb.exp                     |  4 +-
 5 files changed, 123 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/finish-var-size.exp
 create mode 100644 gdb/testsuite/gdb.ada/finish-var-size/p.adb
 create mode 100644 gdb/testsuite/gdb.ada/finish-var-size/pck.adb
 create mode 100644 gdb/testsuite/gdb.ada/finish-var-size/pck.ads

diff --git a/gdb/testsuite/gdb.ada/finish-var-size.exp b/gdb/testsuite/gdb.ada/finish-var-size.exp
new file mode 100644
index 00000000000..aab3fd6474e
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/finish-var-size.exp
@@ -0,0 +1,43 @@
+# Copyright 2022 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/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+# GCC 12 has the needed fix.
+if {![test_compiler_info {gcc-1[2-9]-*}]} {
+    untested "GCC too told for this test"
+    return -1
+}
+
+standard_ada_testfile p
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable debug] != ""} {
+  return -1
+}
+
+clean_restart ${testfile}
+runto "pck.get" temporary
+
+set value [string_to_regexp "= (defined => true, payload => true)"]
+
+# With some ABIs the return value cannot be determined.  Accept this,
+# or the correct result.
+gdb_test "finish" \
+         "Value returned .*($value|Cannot determine contents)"
+
+# Test that an inferior call yields the correct result.
+gdb_test "print pck.get(True)" $value
diff --git a/gdb/testsuite/gdb.ada/finish-var-size/p.adb b/gdb/testsuite/gdb.ada/finish-var-size/p.adb
new file mode 100644
index 00000000000..33d7b4895b4
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/finish-var-size/p.adb
@@ -0,0 +1,21 @@
+--  Copyright 2022 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/>.
+
+with Pck; use Pck;
+procedure P is
+  V : Result_T := Get (True);
+begin
+  null;
+end P;
diff --git a/gdb/testsuite/gdb.ada/finish-var-size/pck.adb b/gdb/testsuite/gdb.ada/finish-var-size/pck.adb
new file mode 100644
index 00000000000..884cf9160fd
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/finish-var-size/pck.adb
@@ -0,0 +1,21 @@
+--  Copyright 2022 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/>.
+
+package body Pck is
+  function Get (Value: Boolean) return Result_T is
+  begin
+    return (Defined => True, Payload => Value);
+  end Get;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/finish-var-size/pck.ads b/gdb/testsuite/gdb.ada/finish-var-size/pck.ads
new file mode 100644
index 00000000000..d6fcbfa1f1d
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/finish-var-size/pck.ads
@@ -0,0 +1,36 @@
+--  Copyright 2022 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/>.
+
+package Pck is
+  type Array_Type is array (1 .. 64) of Integer;
+
+  type Maybe_Array (Defined : Boolean := False) is
+    record
+      Arr : Array_Type;
+      Arr2 : Array_Type;
+    end record;
+
+  type Result_T (Defined : Boolean := False) is
+    record
+      case Defined is
+        when False =>
+	  Arr : Maybe_Array;
+        when True =>
+          Payload : Boolean;
+      end case;
+    end record;
+
+  function Get (Value: Boolean) return Result_T;
+end Pck;
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 44cc28b3005..aa71cfd5e42 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -719,13 +719,13 @@ proc runto { linespec args } {
     # the "at foo.c:36" output we get with -g.
     # the "in func" output we get without -g.
     gdb_expect 30 {
-	-re "Break.* at .*:$decimal.*$gdb_prompt $" {
+	-re "(?:Break|Temporary break).* at .*:$decimal.*$gdb_prompt $" {
 	    if { $print_pass } {
 		pass $test_name
 	    }
 	    return 1
 	}
-	-re "Breakpoint \[0-9\]*, \[0-9xa-f\]* in .*$gdb_prompt $" { 
+	-re "(?:Breakpoint|Temporary breakpoint) \[0-9\]*, \[0-9xa-f\]* in .*$gdb_prompt $" { 
 	    if { $print_pass } {
 		pass $test_name
 	    }
-- 
2.34.3


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

* Re: [PATCH 0/9] Fix "finish" with variably-sized types
  2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
                   ` (8 preceding siblings ...)
  2022-10-07 18:01 ` [PATCH 9/9] Add test case for "finish" with variably-sized types Tom Tromey
@ 2023-01-03 16:44 ` Tom Tromey
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2023-01-03 16:44 UTC (permalink / raw)
  To: Tom Tromey via Gdb-patches; +Cc: Tom Tromey

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> The "finish" command does not correctly work with variably-sized
Tom> types.  This affects some Ada programs; I'm not sure whether it can
Tom> affect programs written in other languages.

Tom> This series fixes the problem, for the architectures I could test.
Tom> Patch #2 explains this.

Tom> Regression tested on x86-64 Fedora 34.  I also ran this through the
Tom> AdaCore internal test suite, on all the architectures that are touched
Tom> by this series.

I've rebased this series.  It required one minor change to adapt to some
other work in this area.

I've re-regression tested it on x86-64 Fedora 36.  We've also been using
this internally at AdaCore since October or so, regularly testing on all
the platforms that are touched by the series.

I'm checking it in now.

thanks,
Tom

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

end of thread, other threads:[~2023-01-03 16:44 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
2022-10-07 18:01 ` [PATCH 1/9] Fix crash in amd64-tdep.c Tom Tromey
2022-10-07 18:01 ` [PATCH 2/9] Add new overload of gdbarch_return_value Tom Tromey
2022-10-07 18:01 ` [PATCH 3/9] Don't emit gdbarch_return_value Tom Tromey
2022-10-07 18:01 ` [PATCH 4/9] Introduce value_at_non_lval Tom Tromey
2022-10-07 18:01 ` [PATCH 5/9] Don't let property evaluation affect the current language Tom Tromey
2022-10-07 18:01 ` [PATCH 6/9] Convert selected architectures to gdbarch_return_value_as_value Tom Tromey
2022-10-07 18:01 ` [PATCH 7/9] Fix inferior calls with variably-sized return type Tom Tromey
2022-10-07 18:01 ` [PATCH 8/9] Use value_at_non_lval in get_call_return_value Tom Tromey
2022-10-07 18:01 ` [PATCH 9/9] Add test case for "finish" with variably-sized types Tom Tromey
2023-01-03 16:44 ` [PATCH 0/9] Fix " Tom Tromey

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