Patchwork Force array coercion in c_get_string

login
register
mail settings
Submitter Tom Tromey
Date April 25, 2019, 6:17 p.m.
Message ID <20190425181740.18062-1-tromey@adacore.com>
Download mbox | patch
Permalink /patch/32417/
State New
Headers show

Comments

Tom Tromey - April 25, 2019, 6:17 p.m.
A user here noticed that the Python Value.string method did not work
for Ada arrays.  I tracked this down to an oddity in value_as_address
-- namely, it calls coerce_array, but that function will not force
array coercion when the language has c_style_arrays=false, as Ada
does.

This patch fixes the problem by forcing array coercion in
c_get_string.  Changing either value_as_address or coerce_array seemed
iffy to me.

Initially here I tried making arrays take the "in GDB's memory" branch
of the if near the top of c_get_string; but this fails the C struct
hack test case added for PR 16196.

Tested on x86-64 Fedora 29.

gdb/ChangeLog
2019-04-25  Tom Tromey  <tromey@adacore.com>

	* c-lang.c (c_get_string): Call value_coerce_array.

gdb/testsuite/ChangeLog
2019-04-25  Tom Tromey  <tromey@adacore.com>

	* gdb.python/py-value.exp (test_value_in_inferior): Add Ada test.
---
 gdb/ChangeLog                         | 4 ++++
 gdb/c-lang.c                          | 7 +++++++
 gdb/testsuite/ChangeLog               | 4 ++++
 gdb/testsuite/gdb.python/py-value.exp | 7 +++++++
 4 files changed, 22 insertions(+)
Pedro Alves - April 25, 2019, 6:32 p.m.
On 4/25/19 7:17 PM, Tom Tromey wrote:
> A user here noticed that the Python Value.string method did not work
> for Ada arrays.  I tracked this down to an oddity in value_as_address
> -- namely, it calls coerce_array, but that function will not force
> array coercion when the language has c_style_arrays=false, as Ada
> does.
> 
> This patch fixes the problem by forcing array coercion in
> c_get_string.  Changing either value_as_address or coerce_array seemed
> iffy to me.

So with the patch, we coerce the array to inferior memory, in order
to read its contents back from the inferior?  Is that right?
Why not skip the detour and save the memory allocation in the inferior?

If this patch is reacheable with core or exec file debugging, your patch
makes gdb error out, because you can't allocate inferior memory in those
cases, of course.  Not sure whether that's relevant here, but it might be.

Thanks,
Pedro Alves
Tom Tromey - April 25, 2019, 6:52 p.m.
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> So with the patch, we coerce the array to inferior memory, in order
Pedro> to read its contents back from the inferior?  Is that right?
Pedro> Why not skip the detour and save the memory allocation in the inferior?

Yeah, that's weird, isn't it.

I think I was confused and thinking that value_coerce_to_target would
not bother for things already in memory.

I'll look at a different approach.

Tom

Patch

diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index 33506f1d1ed..2084134c36d 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -306,6 +306,13 @@  c_get_string (struct value *value, gdb::unique_xmalloc_ptr<gdb_byte> *buffer,
     }
   else
     {
+      /* value_as_address calls coerce_array, but that won't actually
+	 coerce the array if c_style_arrays is false.  However, in
+	 this particular case, we do want to always force array
+	 coercion.  So, check this case here.  */
+      if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+	value = value_coerce_array (value);
+
       CORE_ADDR addr = value_as_address (value);
 
       /* Prior to the fix for PR 16196 read_string would ignore fetchlimit
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index b3d90b52272..7f42d31572a 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -315,6 +315,13 @@  proc test_value_in_inferior {} {
   gdb_test "python print (\"---\"+st.string (length = 0)+\"---\")" "------" "test string (length = 0) is empty"
   gdb_test "python print (len(st.string (length = 0)))" "0" "test length is 0"
 
+  # We choose Ada here to test a language where c_style_arrays is
+  # false.
+  gdb_test "set lang ada" \
+      "Warning: the current language does not match this frame."
+  gdb_test "python print (st.string ())"  "divide et impera"  \
+      "Test string with no length in ada"
+  gdb_test_no_output "set lang auto"
 
   # Fetch a string that has embedded nulls.
   gdb_test "print nullst" "\"divide\\\\000et\\\\000impera\".*"