[PATCH] Use read_memory in ada_exception_message_1

[PATCH] Use read_memory in ada_exception_message_1

[Tom Tromey]

Testing using the internal AdaCore test suite showed a regression from
the target string reading changes.  In particular, now
ada_exception_message_1 can get the wrong answer in some cases.

I was not able to reproduce this failure with the Fedora gnat.
Perhaps it is related to some local change to gnat; I do not know.

Meanwhile, because ada_exception_message_1 knows the length of the
string to read, we can use read_memory here.  This fixes the bug.

I've updated the test suite to at least exercise this code path.
However, as mentioned above, the new test does not actually provoke
the failure.

gdb/ChangeLog
2020-07-02  Tom Tromey  <tromey@adacore.com>

	* ada-lang.c (ada_exception_message_1): Use read_memory.

gdb/testsuite/ChangeLog
2020-07-02  Tom Tromey  <tromey@adacore.com>

	* gdb.ada/catch_ex/foo.adb: Pass string to raise.
	* gdb.ada/catch_ex.exp: Examine catchpoint text.s
---
 gdb/ChangeLog                          | 4 ++++
 gdb/ada-lang.c                         | 7 ++++++-
 gdb/testsuite/ChangeLog                | 5 +++++
 gdb/testsuite/gdb.ada/catch_ex.exp     | 2 +-
 gdb/testsuite/gdb.ada/catch_ex/foo.adb | 2 +-
 5 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 98508c168bc..cbcceba838d 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -11894,7 +11894,12 @@ ada_exception_message_1 (void)
   if (e_msg_len <= 0)
     return NULL;
 
-  return target_read_string (value_address (e_msg_val), INT_MAX);
+  gdb::unique_xmalloc_ptr<char> e_msg ((char *) xmalloc (e_msg_len + 1));
+  read_memory (value_address (e_msg_val), (gdb_byte *) e_msg.get (),
+	       e_msg_len);
+  e_msg.get ()[e_msg_len] = '\0';
+
+  return e_msg;
 }
 
 /* Same as ada_exception_message_1, except that all exceptions are
diff --git a/gdb/testsuite/gdb.ada/catch_ex.exp b/gdb/testsuite/gdb.ada/catch_ex.exp
index 7bb1c06f544..68914139830 100644
--- a/gdb/testsuite/gdb.ada/catch_ex.exp
+++ b/gdb/testsuite/gdb.ada/catch_ex.exp
@@ -62,7 +62,7 @@ gdb_test "info break" \
          "info break, catch all Ada exceptions"
 
 set catchpoint_msg \
-  "Catchpoint $any_nb, CONSTRAINT_ERROR (\\\(foo\\.adb:$decimal explicit raise\\\) )?at $any_addr in foo \\\(\\\).*at .*foo.adb:$any_nb"
+  "Catchpoint $any_nb, CONSTRAINT_ERROR (\\\(ignore C_E\\\) )?at $any_addr in foo \\\(\\\).*at .*foo.adb:$any_nb"
 gdb_test "continue" \
          "Continuing\.$eol$catchpoint_msg$eol.*SPOT1" \
          "continuing to first exception"
diff --git a/gdb/testsuite/gdb.ada/catch_ex/foo.adb b/gdb/testsuite/gdb.ada/catch_ex/foo.adb
index bee394fc02f..cdf0286beee 100644
--- a/gdb/testsuite/gdb.ada/catch_ex/foo.adb
+++ b/gdb/testsuite/gdb.ada/catch_ex/foo.adb
@@ -17,7 +17,7 @@ procedure Foo is
 begin
 
    begin
-      raise Constraint_Error;  -- SPOT1
+      raise Constraint_Error with "ignore C_E";  -- SPOT1
    exception
       when others =>
          null;
-- 
2.21.3

Re: [PATCH] Use read_memory in ada_exception_message_1

[Joel Brobecker]

Hi Tom,

On Thu, Jul 02, 2020 at 03:13:46PM -0600, Tom Tromey wrote:
> Testing using the internal AdaCore test suite showed a regression from
> the target string reading changes.  In particular, now
> ada_exception_message_1 can get the wrong answer in some cases.
> 
> I was not able to reproduce this failure with the Fedora gnat.
> Perhaps it is related to some local change to gnat; I do not know.

It's probably a change that we made internally at AdaCore which
hasn't made its way to either the GCC repository or into the version
that Fedora uses. Considering the change that you are making,
I'm guessing this might have something to do with a change we made
that might have removed the terminating NUL character at the end
(Ada strings don't have a terminating NUL).

It would be useful to provide some information that shows what
we were supposed to get and what we actually do get, at the very
least for archeology purposes. This may also show help confirm
or refute my guess above.

> Meanwhile, because ada_exception_message_1 knows the length of the
> string to read, we can use read_memory here.  This fixes the bug.
> 
> I've updated the test suite to at least exercise this code path.
> However, as mentioned above, the new test does not actually provoke
> the failure.
> 
> gdb/ChangeLog
> 2020-07-02  Tom Tromey  <tromey@adacore.com>
> 
> 	* ada-lang.c (ada_exception_message_1): Use read_memory.
> 
> gdb/testsuite/ChangeLog
> 2020-07-02  Tom Tromey  <tromey@adacore.com>
> 
> 	* gdb.ada/catch_ex/foo.adb: Pass string to raise.
> 	* gdb.ada/catch_ex.exp: Examine catchpoint text.s

OK for me. Since we do know the string's length thanks to the fact
that Ada arrays have bounds built in, it seems just as well to read
the buffer without having to worry about reading things piecemeal
until we find a terminating NUL character.

Just one nit: In the last line of the ChangeLog entry above, there is
an unwanted trailing 's'.

-- 
Joel

Re: [PATCH] Use read_memory in ada_exception_message_1

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> It would be useful to provide some information that shows what
Joel> we were supposed to get and what we actually do get, at the very
Joel> least for archeology purposes. This may also show help confirm
Joel> or refute my guess above.

I added a bit of text to the commit message.

>> * gdb.ada/catch_ex.exp: Examine catchpoint text.s

Joel> Just one nit: In the last line of the ChangeLog entry above, there is
Joel> an unwanted trailing 's'.

Oops, I fixed this.  I'm checking it in now.

Tom