public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
@ 2011-12-06 17:33 Pedro Alves
  2011-12-07 10:07 ` Joel Brobecker
  0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2011-12-06 17:33 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker

Hi!

I get a bunch of cascading failures on gdb.ada/catch_ex.exp and gdb.ada/mi_catch_ex.exp.
These annoy me because the set of FAILs and timeouts is different in
sync vs async.  :-)  But that's actually irrelevant.  The relevant bit of gdb.sum:

Running ../../../gdb/gdb/testsuite/gdb.ada/catch_ex.exp ...
FAIL: gdb.ada/catch_ex.exp: insert catchpoint on all Ada exceptions
FAIL: gdb.ada/catch_ex.exp: info break, catch all Ada exceptions
FAIL: gdb.ada/catch_ex.exp: continuing to first exception (the program exited)
FAIL: gdb.ada/catch_ex.exp: continuing to second exception (the program is no longer running)
FAIL: gdb.ada/catch_ex.exp: insert catchpoint on Program_Error
FAIL: gdb.ada/catch_ex.exp: insert catchpoint on failed assertions
FAIL: gdb.ada/catch_ex.exp: insert catchpoint on unhandled exceptions
FAIL: gdb.ada/catch_ex.exp: info break, second run
FAIL: gdb.ada/catch_ex.exp: continuing to Program_Error exception (the program exited)
FAIL: gdb.ada/catch_ex.exp: continuing to failed assertion (the program is no longer running)
FAIL: gdb.ada/catch_ex.exp: continuing to unhandled exception (the program is no longer running)
FAIL: gdb.ada/catch_ex.exp: continuing to program completion (the program is no longer running)
FAIL: gdb.ada/catch_ex.exp: tcatch exception
FAIL: gdb.ada/catch_ex.exp: continuing to temporary catchpoint (the program exited)
FAIL: gdb.ada/catch_ex.exp: continuing to program completion (the program is no longer running)
Running ../../../gdb/gdb/testsuite/gdb.ada/mi_catch_ex.exp ...
FAIL: gdb.ada/mi_catch_ex.exp: insert catchpoint on all Ada exceptions
FAIL: gdb.ada/mi_catch_ex.exp: continue to exception catchpoint hit (unknown output after running)
FAIL: gdb.ada/mi_catch_ex.exp: continue to exception catchpoint hit (MI error)
FAIL: gdb.ada/mi_catch_ex.exp: continue to exception catchpoint hit (timeout)
FAIL: gdb.ada/mi_catch_ex.exp: continue to exception catchpoint hit (unknown output after running)
FAIL: gdb.ada/mi_catch_ex.exp: continue to exception catchpoint hit (MI error)
FAIL: gdb.ada/mi_catch_ex.exp: continue to exception catchpoint hit (timeout)
FAIL: gdb.ada/mi_catch_ex.exp: continue to exception catchpoint hit (MI error)
FAIL: gdb.ada/mi_catch_ex.exp: continue to exception catchpoint hit (timeout)

And the gdb.log shows:

(gdb) catch exception
Cannot insert catchpoints in this configuration.
(gdb) FAIL: gdb.ada/catch_ex.exp: insert catchpoint on all Ada exceptions
...

So it looks like I just don't have debug info for the runtime on this
machine.  The tests in question already try to detect the case:

    -re "Cannot break on __gnat_raise_nodefer_with_msg in this configuration\.$eol$gdb_prompt $" {
	# If the runtime was not built with enough debug information,
	# or if it was stripped, we can not test exception
	# catchpoints.
	unsupported $msg
	return -1
    }

but they don't match this specific error message.  I can't seem to find the
"Cannot break on" string anywhere, is it escaping me?  The patch below
makes the test accept both variants.  Should we keep the new form
only instead?

(BTW, the "cannot insert catchpoints" error may be a bit confusing,
considering there are other non-Ada catchpoints that will work just
fine.)

gdb/testsuite/
2011-12-06  Pedro Alves  <pedro@codesourcery.com>

	* gdb.ada/catch_ex.exp: Skip as unsupported if "catch exception"
	throws "Cannot insert catchpoints in this configuration".
	* gdb.ada/mi_catch_ex.exp: Likewise.

With the patch I now get:

Running ../../../gdb/gdb/testsuite/gdb.ada/catch_ex.exp ...
Running ../../../gdb/gdb/testsuite/gdb.ada/mi_catch_ex.exp ...

                === gdb Summary ===

# of expected passes            2
# of unsupported tests          2

Okay?

---
 gdb/testsuite/gdb.ada/catch_ex.exp    |    2 +-
 gdb/testsuite/gdb.ada/mi_catch_ex.exp |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.ada/catch_ex.exp b/gdb/testsuite/gdb.ada/catch_ex.exp
index fa458d8..0ea3256 100644
--- a/gdb/testsuite/gdb.ada/catch_ex.exp
+++ b/gdb/testsuite/gdb.ada/catch_ex.exp
@@ -56,7 +56,7 @@ gdb_test_multiple "catch exception" $msg {
     -re "Catchpoint $any_nb: all Ada exceptions$eol$gdb_prompt $" {
 	pass $msg
     }
-    -re "Cannot break on __gnat_raise_nodefer_with_msg in this configuration\.$eol$gdb_prompt $" {
+    -re "Cannot (insert catchpoints|break on __gnat_raise_nodefer_with_msg) in this configuration\.$eol$gdb_prompt $" {
 	# If the runtime was not built with enough debug information,
 	# or if it was stripped, we can not test exception
 	# catchpoints.
diff --git a/gdb/testsuite/gdb.ada/mi_catch_ex.exp b/gdb/testsuite/gdb.ada/mi_catch_ex.exp
index dadc574..4be1cad 100644
--- a/gdb/testsuite/gdb.ada/mi_catch_ex.exp
+++ b/gdb/testsuite/gdb.ada/mi_catch_ex.exp
@@ -46,7 +46,7 @@ gdb_test_multiple "catch exception" $msg {
     -re "Catchpoint $any_nb: all Ada exceptions$eol$gdb_prompt $" {
 	pass $msg
     }
-    -re "Cannot break on __gnat_raise_nodefer_with_msg in this configuration\.\[\r\n\]+$gdb_prompt $" {
+    -re "Cannot (insert catchpoints|break on __gnat_raise_nodefer_with_msg) in this configuration\.\[\r\n\]+$gdb_prompt $" {
 	# If the runtime was not built with enough debug information,
 	# or if it was stripped, we can not test exception
 	# catchpoints.

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-06 17:33 [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints Pedro Alves
@ 2011-12-07 10:07 ` Joel Brobecker
  2011-12-07 15:29   ` Pedro Alves
                     ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Joel Brobecker @ 2011-12-07 10:07 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> (gdb) catch exception
> Cannot insert catchpoints in this configuration.

We're going to have to talk to the guys who decided to build the Ada
runtime without debugging info. The normal way to build the runtime
is no debug info *except* for a few files. This is really doing a
disservice to the users!

Grumble, grumble, grumble. Sorry.

> but they don't match this specific error message.  I can't seem to find the
> "Cannot break on" string anywhere, is it escaping me?

It's actuall there, see ada-lang.c:ada_exception_sal. But upon more
careful inspection, I think it might have become dead code:
The error is only raised if we failed to look that symbol up, but
the call to ada_exception_support_info_sniffer to have already errored
out if that was the case. So I think we can replace that by an assertion
and simplify the testcase.

That's going to be on me, but I won't have much time this week.

In the meantime...

> (BTW, the "cannot insert catchpoints" error may be a bit confusing,
> considering there are other non-Ada catchpoints that will work just
> fine.)

I'll try to think of a better error message - it's not my forte...

> gdb/testsuite/
> 2011-12-06  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* gdb.ada/catch_ex.exp: Skip as unsupported if "catch exception"
> 	throws "Cannot insert catchpoints in this configuration".
> 	* gdb.ada/mi_catch_ex.exp: Likewise.

... I think that this is a good stop-gap measure. It helps you, so
it's OK to go in. I'll cleanup afterwards anyways.  Thanks for doing
that.

-- 
Joel

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-07 10:07 ` Joel Brobecker
@ 2011-12-07 15:29   ` Pedro Alves
  2011-12-07 16:10     ` Pedro Alves
  2011-12-07 22:01   ` Tom Tromey
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2011-12-07 15:29 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Wednesday 07 December 2011 10:01:01, Joel Brobecker wrote:
> > (gdb) catch exception
> > Cannot insert catchpoints in this configuration.
> 
> We're going to have to talk to the guys who decided to build the Ada
> runtime without debugging info. The normal way to build the runtime
> is no debug info *except* for a few files. This is really doing a
> disservice to the users!

This is plain gcc/gnat as shipped on Ubuntu 11.04.  It may be
that the idea is that since this is not necessary for backtracing,
it's not crucial, and then one is expected to install some -dev
or -dbg package to be able to fully debug Ada?  But then again,
I've installed all gnat related -dev and -dbg packagets I could
find now, and I still get catchpoints unsupported.  What library
is supposed to have these symbols?

> Grumble, grumble, grumble. Sorry.

That's fine.  :-)

> > gdb/testsuite/
> > 2011-12-06  Pedro Alves  <pedro@codesourcery.com>
> > 
> > 	* gdb.ada/catch_ex.exp: Skip as unsupported if "catch exception"
> > 	throws "Cannot insert catchpoints in this configuration".
> > 	* gdb.ada/mi_catch_ex.exp: Likewise.
> 
> ... I think that this is a good stop-gap measure. It helps you, so
> it's OK to go in. I'll cleanup afterwards anyways.  Thanks for doing
> that.

Alright, I've checked it in now.  Thanks!

-- 
Pedro Alves

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-07 15:29   ` Pedro Alves
@ 2011-12-07 16:10     ` Pedro Alves
  0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2011-12-07 16:10 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker

On Wednesday 07 December 2011 15:14:11, Pedro Alves wrote:
> On Wednesday 07 December 2011 10:01:01, Joel Brobecker wrote:
> > > (gdb) catch exception
> > > Cannot insert catchpoints in this configuration.
> > 
> > We're going to have to talk to the guys who decided to build the Ada
> > runtime without debugging info. The normal way to build the runtime
> > is no debug info *except* for a few files. This is really doing a
> > disservice to the users!
> 
> This is plain gcc/gnat as shipped on Ubuntu 11.04.  It may be
> that the idea is that since this is not necessary for backtracing,
> it's not crucial, and then one is expected to install some -dev
> or -dbg package to be able to fully debug Ada?  But then again,
> I've installed all gnat related -dev and -dbg packagets I could
> find now, and I still get catchpoints unsupported.  What library
> is supposed to have these symbols?

Oh silly me.  I forgot that we default to prefix=/usr/local, and
then we end up not picking the /usr/debug debug dir by default.
Building with --prefix=/usr made catchpoints now work.  And I had
stumbled on this before when I hacked on Ada catchpoints a
while back, and just forgot about it.  The good
news is that I still get all passes, sync and async.  :-)

Now I don't know if I needed any extra package compared to what
I had already installed, or if I did, which . :-P  In any case, it's
clear the necessary symbols aren't in any library's minimal symbols.

-- 
Pedro Alves

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-07 10:07 ` Joel Brobecker
  2011-12-07 15:29   ` Pedro Alves
@ 2011-12-07 22:01   ` Tom Tromey
  2011-12-07 23:18     ` Joel Brobecker
  2011-12-11 17:42   ` [commit] Ada exception catchpoint support cleanup Joel Brobecker
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2011-12-07 22:01 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Pedro Alves, gdb-patches

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

>> (gdb) catch exception
>> Cannot insert catchpoints in this configuration.

Joel> We're going to have to talk to the guys who decided to build the Ada
Joel> runtime without debugging info. The normal way to build the runtime
Joel> is no debug info *except* for a few files. This is really doing a
Joel> disservice to the users!

Fedora strips everything to a separate debuginfo file in an automated
way.  There might be an exception for one library, but last time I asked
about this, I was told that exceptions are a pain and could we please do
something else.  Also, the granularity of debuginfo packages is quite
large.

This is one reason we ended up using sdt.h probes in preference to a
special unwind hook function for the C++ "next-over-throw" work.

If you can make it so this can work without debuginfo somehow, that
would be much better for users.  I think the sdt.h approach is quite
awesome; though it is ELF-specific (still -- better to do excellently on
some platforms if possible) and maybe hard to port to Ada.

Tom

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-07 22:01   ` Tom Tromey
@ 2011-12-07 23:18     ` Joel Brobecker
  2011-12-09  3:50       ` Tom Tromey
  2011-12-09 17:20       ` Pedro Alves
  0 siblings, 2 replies; 18+ messages in thread
From: Joel Brobecker @ 2011-12-07 23:18 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Pedro Alves, gdb-patches

(to answer Pedro's questions, the files in question are libgnat.a and
libgnat.so)

> Fedora strips everything to a separate debuginfo file in an automated
> way.  There might be an exception for one library, but last time I asked
> about this, I was told that exceptions are a pain and could we please do
> something else.  Also, the granularity of debuginfo packages is quite
> large.

I can see the reason for this, but it is kind of a pain for Ada.
I believe the makefiles already build the runtime without debugging
info except for the few files that we need (I think it's basically
a-tags.adb and a-except.adb)...

> If you can make it so this can work without debuginfo somehow, that
> would be much better for users.

The problem is that we need to get to the argument of function we are
breaking on in order to determine which exception has been raised.
That's where we really need the debug info. I think I even added
a comment about that in the code...

-- 
Joel

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-07 23:18     ` Joel Brobecker
@ 2011-12-09  3:50       ` Tom Tromey
  2011-12-09 17:20       ` Pedro Alves
  1 sibling, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2011-12-09  3:50 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Pedro Alves, gdb-patches

Tom> If you can make it so this can work without debuginfo somehow, that
Tom> would be much better for users.

Joel> The problem is that we need to get to the argument of function we are
Joel> breaking on in order to determine which exception has been raised.
Joel> That's where we really need the debug info. I think I even added
Joel> a comment about that in the code...

Another nice feature of sdt.h probes is that you can extract arguments
without debuginfo :-)

Tom

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-07 23:18     ` Joel Brobecker
  2011-12-09  3:50       ` Tom Tromey
@ 2011-12-09 17:20       ` Pedro Alves
  2011-12-09 18:00         ` Tom Tromey
  1 sibling, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2011-12-09 17:20 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches

On Wednesday 07 December 2011 22:01:19, Joel Brobecker wrote:
> (to answer Pedro's questions, the files in question are libgnat.a and
> libgnat.so)
> 
> > Fedora strips everything to a separate debuginfo file in an automated
> > way.  There might be an exception for one library, but last time I asked
> > about this, I was told that exceptions are a pain and could we please do
> > something else.  Also, the granularity of debuginfo packages is quite
> > large.
> 
> I can see the reason for this, but it is kind of a pain for Ada.
> I believe the makefiles already build the runtime without debugging
> info except for the few files that we need (I think it's basically
> a-tags.adb and a-except.adb)...
> 
> > If you can make it so this can work without debuginfo somehow, that
> > would be much better for users.
> 
> The problem is that we need to get to the argument of function we are
> breaking on in order to determine which exception has been raised.
> That's where we really need the debug info. I think I even added
> a comment about that in the code...

GDB knows the target's function call ABI (for infcalls), and supposedly
this function's prototype is cast in stone as part of the ABI too.
GDB could just know where to get the arguments from?  You'd still need
minimal symbols in libgnat, but that doesn't look much different
from needing minimal symbols for libpthread, from the packager's
perspective -- it should be simpler than caring about stripping the library
except a couple objects, due to size considerations.

-- 
Pedro Alves

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-09 17:20       ` Pedro Alves
@ 2011-12-09 18:00         ` Tom Tromey
  2011-12-09 18:13           ` Pedro Alves
  0 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2011-12-09 18:00 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Joel Brobecker, gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Joel> The problem is that we need to get to the argument of function we are
Joel> breaking on in order to determine which exception has been raised.
Joel> That's where we really need the debug info. I think I even added
Joel> a comment about that in the code...

Pedro> GDB knows the target's function call ABI (for infcalls), and supposedly
Pedro> this function's prototype is cast in stone as part of the ABI too.
Pedro> GDB could just know where to get the arguments from?

Yeah, it could be done.  Right now though the knowledge in GDB is in a
form that is specific to making infcalls, not decoding them.  So, adding
this functionality would be a significant amount of work, and also
non-trivial to test.  I count 51 calls to set_gdbarch_push_dummy_call in
the tree...

Tom

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-09 18:00         ` Tom Tromey
@ 2011-12-09 18:13           ` Pedro Alves
  2011-12-09 18:40             ` Tom Tromey
  0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2011-12-09 18:13 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Joel Brobecker, gdb-patches

On Friday 09 December 2011 17:43:40, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Joel> The problem is that we need to get to the argument of function we are
> Joel> breaking on in order to determine which exception has been raised.
> Joel> That's where we really need the debug info. I think I even added
> Joel> a comment about that in the code...
> 
> Pedro> GDB knows the target's function call ABI (for infcalls), and supposedly
> Pedro> this function's prototype is cast in stone as part of the ABI too.
> Pedro> GDB could just know where to get the arguments from?
> 
> Yeah, it could be done.  Right now though the knowledge in GDB is in a
> form that is specific to making infcalls, not decoding them.  So, adding
> this functionality would be a significant amount of work, and also
> non-trivial to test.  I count 51 calls to set_gdbarch_push_dummy_call in
> the tree...
> 

But _only_ 45 unique callbacks.

$ grep set_gdbarch_push_dummy_call *-tdep.c | sed 's/.*arch, //g; s/);//g' | sort | uniq | wc -l
45

:-D

Kidding aside, obviously we wouldn't need to convert
everything at once.  We could even get away with only handling
the simpler prototypes we care about at first.

-- 
Pedro Alves

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-09 18:13           ` Pedro Alves
@ 2011-12-09 18:40             ` Tom Tromey
  2011-12-10 22:53               ` Pedro Alves
  0 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2011-12-09 18:40 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Joel Brobecker, gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> Kidding aside, obviously we wouldn't need to convert
Pedro> everything at once.  We could even get away with only handling
Pedro> the simpler prototypes we care about at first.

Yeah.  And to be totally clear, if someone wants to do this, I am in
favor of it.

Tom

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-09 18:40             ` Tom Tromey
@ 2011-12-10 22:53               ` Pedro Alves
  2011-12-11 20:33                 ` Joel Brobecker
  2011-12-20 14:53                 ` Tom Tromey
  0 siblings, 2 replies; 18+ messages in thread
From: Pedro Alves @ 2011-12-10 22:53 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Joel Brobecker, gdb-patches

On Friday 09 December 2011 18:12:13, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Pedro> Kidding aside, obviously we wouldn't need to convert
> Pedro> everything at once.  We could even get away with only handling
> Pedro> the simpler prototypes we care about at first.
> 
> Yeah.  And to be totally clear, if someone wants to do this, I am in
> favor of it.

Here's a start.  I didn't really try to confirm if I was extracting the
arguments correctly for any other function bug the Ada runtime's
function in question, but at least it works in the case we're
interested in.  :-)

Unfortunately, libgnat is fully stripped (at least on ubuntu), symbol
table and all, so there'd be a little repackaging work to do to get
rid of the debug info dependency -- otherwise, we still need debug
info to be able to insert the hook breakpoints (but no longer to
be able to extract the hook's argument).

2011-12-10  Pedro Alves  <pedro@codesourcery.com>

	* ada-lang.c (ada_unhandled_exception_name_addr): Extract the
	exception name without using debug info if the gdbarch supports
	it.
	(ada_exception_name_addr_1): Use ada_unhandled_exception_name_addr
	for ex_catch_exception.
	* amd64-tdep.c (amd64_extract_arguments_1)
	(amd64_extract_arguments): New.
	(amd64_init_abi): Install amd64_extract_arguments as
	extract_arguments gdbarch method.
	* i386-tdep.c (i386_extract_arguments): New.
	(i386_gdbarch_init): Install i386_extract_arguments as
	extract_arguments gdbarch method.
	* gdbarch.sh (extract_arguments): New.
	* gdbarch.h, gdbarch.c: Regenerate.
---

 gdb/ada-lang.c   |   61 ++++++++++++++++++++-
 gdb/amd64-tdep.c |  157 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbarch.c    |   33 +++++++++++
 gdb/gdbarch.h    |    6 ++
 gdb/gdbarch.sh   |    7 ++
 gdb/i386-tdep.c  |   50 +++++++++++++++++
 6 files changed, 311 insertions(+), 3 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 3843539..1aecfdb 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -10804,7 +10804,63 @@ ada_find_printable_frame (struct frame_info *fi)
 static CORE_ADDR
 ada_unhandled_exception_name_addr (void)
 {
-  return parse_and_eval_address ("e.full_name");
+  struct frame_info *frame = get_current_frame ();
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+
+  /* Try to extract the arguments even if debug info for libgnat isn't
+     present.  */
+  if (gdbarch_extract_arguments_p (gdbarch))
+    {
+      struct type **args_in = alloca (1 * sizeof (struct type *));
+      struct value **args_out = alloca (1 * sizeof (struct value *));
+      struct type *void_ptr_type
+	= lookup_pointer_type (builtin_type (gdbarch)->builtin_void);
+      CORE_ADDR e;
+      struct value *full_name_val;
+      CORE_ADDR full_name;
+
+      /* Argument is a pointer to a system.standard_library.exception_data
+	 object.  E.g.,
+
+	 <__gnat_debug_raise_exception> (e=0x7ffff7dcc2e0) at s-except.adb:46
+	 (gdb) p e
+	 $1 = (access system.standard_library.exception_data) 0x7ffff7dcc2e0
+	 (gdb) p *e
+	 $2 = (
+	   not_handled_by_others => false,
+	   lang => 65 'A',
+	   name_length => 17,
+	   full_name => (system.address) 0x7ffff7b624a0,
+	   htable_ptr => 0x7ffff7dcc320,
+	   import_code => 0,
+	   raise_hook => 0
+	 )
+
+	 We're interested in FULL_NAME, which is at offset 8 of E in
+	 all ABIs we care about.
+      */
+
+      /* Extract the sole function's argument.  This is a pointer to a
+	 system.standard_library.exception_data object.  */
+      args_in[0] = void_ptr_type;
+      gdbarch_extract_arguments (gdbarch, frame, 1, args_in, args_out,
+				 NULL, NULL);
+
+      /* Get the argument pointer.  */
+      e = value_as_address (args_out[0]);
+
+      /* The FULL_NAME pointer is at offset 8 of the object pointed by
+	 E.  */
+      full_name_val = value_at (void_ptr_type, e + 8);
+      full_name = value_as_address (full_name_val);
+      return full_name;
+    }
+  else
+    {
+      /* No support for extracting arguments.  Will need to rely on
+	 debug info being present.  */
+      return parse_and_eval_address ("e.full_name");
+    }
 }
 
 /* Same as ada_unhandled_exception_name_addr, except that this function
@@ -10859,8 +10915,7 @@ ada_exception_name_addr_1 (enum exception_catchpoint_kind ex,
   switch (ex)
     {
       case ex_catch_exception:
-        return (parse_and_eval_address ("e.full_name"));
-        break;
+	return ada_unhandled_exception_name_addr ();
 
       case ex_catch_exception_unhandled:
         return exception_info->unhandled_exception_name_addr ();
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 803a20f..4f506ed 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -905,6 +905,162 @@ amd64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   return sp + 16;
 }
+
+static void
+amd64_extract_arguments_1 (struct frame_info *frame, int nargs,
+			   struct type **args_in, struct value **args_out,
+			   int struct_return)
+{
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  int *integer_regs = tdep->call_dummy_integer_regs;
+  int num_integer_regs = tdep->call_dummy_num_integer_regs;
+
+  static int sse_regnum[] =
+  {
+    /* %xmm0 ... %xmm7 */
+    AMD64_XMM0_REGNUM + 0, AMD64_XMM1_REGNUM,
+    AMD64_XMM0_REGNUM + 2, AMD64_XMM0_REGNUM + 3,
+    AMD64_XMM0_REGNUM + 4, AMD64_XMM0_REGNUM + 5,
+    AMD64_XMM0_REGNUM + 6, AMD64_XMM0_REGNUM + 7,
+  };
+  struct type **stack_args = alloca (nargs * sizeof (struct type *));
+  /* We ignore that some arguments that are passed by MEMORY are
+     mirrored in registers.  */
+  int num_stack_args = 0;
+  int element = 0;
+  int integer_reg = 0;
+  int sse_reg = 0;
+  int i;
+  CORE_ADDR sp;
+
+  gdb_assert (tdep->classify);
+
+  /* Reserve a register for the "hidden" argument.  */
+  if (struct_return)
+    integer_reg++;
+
+  for (i = 0; i < nargs; i++)
+    {
+      struct type *type = args_in[i];
+      int len = TYPE_LENGTH (type);
+      enum amd64_reg_class class[2];
+      int needed_integer_regs = 0;
+      int needed_sse_regs = 0;
+      int j;
+
+      /* Classify argument.  */
+      tdep->classify (type, class);
+
+      /* Calculate the number of integer and SSE registers needed for
+         this argument.  */
+      for (j = 0; j < 2; j++)
+	{
+	  if (class[j] == AMD64_INTEGER)
+	    needed_integer_regs++;
+	  else if (class[j] == AMD64_SSE)
+	    needed_sse_regs++;
+	}
+
+      /* Check whether the argument was passed in registers.  */
+      if (integer_reg + needed_integer_regs > num_integer_regs
+	  || sse_reg + needed_sse_regs > ARRAY_SIZE (sse_regnum)
+	  || (needed_integer_regs == 0 && needed_sse_regs == 0))
+	{
+	  /* The argument was passed on the stack.  */
+	  stack_args[num_stack_args] = args_in[i];
+          num_stack_args++;
+	}
+      else
+	{
+	  /* The argument was passed in registers.  We assume if more
+	     than one register was necessary, then N contiguous
+	     registers starting from REGNUM were used.  */
+
+	  struct value *reg_val;
+	  int regnum = -1;
+	  int offset = 0;
+
+	  switch (class[0])
+	    {
+	    case AMD64_INTEGER:
+	      regnum = integer_regs[integer_reg++];
+	      break;
+
+	    case AMD64_SSE:
+	      regnum = sse_regnum[sse_reg++];
+	      break;
+
+	    case AMD64_SSEUP:
+	      gdb_assert (sse_reg > 0);
+	      regnum = sse_regnum[sse_reg - 1];
+	      offset = 8;
+	      break;
+
+	    default:
+	      gdb_assert (!"Unexpected register class.");
+	    }
+
+	  gdb_assert (regnum != -1);
+
+	  reg_val = allocate_value_lazy (args_in[i]);
+	  VALUE_LVAL (reg_val) = lval_register;
+	  VALUE_REGNUM (reg_val) = regnum;
+	  VALUE_FRAME_ID (reg_val) = get_frame_id (frame);
+	  set_value_offset (reg_val, offset);
+
+	  args_out[i] = reg_val;
+	}
+    }
+
+  /* Get the first arg's slot, but unwind, rather than assuming a
+     regular frame with frame pointer.  */
+  sp = get_frame_register_unsigned (get_prev_frame (frame),
+				    AMD64_RSP_REGNUM);
+
+  /* Extract the stack arguments.  */
+  for (i = 0; i < num_stack_args; i++)
+    {
+      struct type *type = stack_args[i];
+      int len = TYPE_LENGTH (type);
+      CORE_ADDR arg_addr = sp + element * 8;
+
+      args_out[i] = value_from_contents_and_address (type, NULL, arg_addr);
+      element += ((len + 7) / 8);
+    }
+}
+
+/* Implementation of gdbarch method extract_arguments.  */
+
+static void
+amd64_extract_arguments (struct frame_info *frame,
+			 int nargs, struct type **args_in,
+			 struct value **args_out,
+			 struct type *struct_return_in,
+			 struct value **struct_return_out)
+{
+  /* Extract arguments.  */
+  amd64_extract_arguments_1 (frame, nargs, args_in, args_out,
+			     struct_return_in != NULL);
+
+  /* Extract "hidden" argument".  */
+  if (struct_return_in)
+    {
+      struct gdbarch *gdbarch = get_frame_arch (frame);
+      struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+      CORE_ADDR struct_addr;
+      /* The "hidden" argument is passed throught the first argument
+         register.  */
+      const int arg_regnum = tdep->call_dummy_integer_regs[0];
+
+      struct_addr = get_frame_register_unsigned (frame, arg_regnum);
+      *struct_return_out
+	= value_from_contents_and_address (struct_return_in,
+					   NULL,
+					   struct_addr);
+    }
+}
+
 \f
 /* Displaced instruction handling.  */
 
@@ -2656,6 +2812,7 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
     ARRAY_SIZE (amd64_dummy_call_integer_regs);
   tdep->call_dummy_integer_regs = amd64_dummy_call_integer_regs;
   tdep->classify = amd64_classify;
+  set_gdbarch_extract_arguments (gdbarch, amd64_extract_arguments);
 
   set_gdbarch_convert_register_p (gdbarch, i387_convert_register_p);
   set_gdbarch_register_to_value (gdbarch, i387_register_to_value);
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 1ada504..88e6728 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -183,6 +183,7 @@ struct gdbarch
   gdbarch_push_dummy_call_ftype *push_dummy_call;
   int call_dummy_location;
   gdbarch_push_dummy_code_ftype *push_dummy_code;
+  gdbarch_extract_arguments_ftype *extract_arguments;
   gdbarch_print_registers_info_ftype *print_registers_info;
   gdbarch_print_float_info_ftype *print_float_info;
   gdbarch_print_vector_info_ftype *print_vector_info;
@@ -338,6 +339,7 @@ struct gdbarch startup_gdbarch =
   0,  /* push_dummy_call */
   0,  /* call_dummy_location */
   0,  /* push_dummy_code */
+  0,  /* extract_arguments */
   default_print_registers_info,  /* print_registers_info */
   0,  /* print_float_info */
   0,  /* print_vector_info */
@@ -626,6 +628,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of push_dummy_call, has predicate.  */
   /* Skip verify of call_dummy_location, invalid_p == 0 */
   /* Skip verify of push_dummy_code, has predicate.  */
+  /* Skip verify of extract_arguments, has predicate.  */
   /* Skip verify of print_registers_info, invalid_p == 0 */
   /* Skip verify of print_float_info, has predicate.  */
   /* Skip verify of print_vector_info, has predicate.  */
@@ -907,6 +910,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: elf_make_msymbol_special = <%s>\n",
                       host_address_to_string (gdbarch->elf_make_msymbol_special));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_extract_arguments_p() = %d\n",
+                      gdbarch_extract_arguments_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: extract_arguments = <%s>\n",
+                      host_address_to_string (gdbarch->extract_arguments));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: fast_tracepoint_valid_at = <%s>\n",
                       host_address_to_string (gdbarch->fast_tracepoint_valid_at));
   fprintf_unfiltered (file,
@@ -2153,6 +2162,30 @@ set_gdbarch_push_dummy_code (struct gdbarch *gdbarch,
   gdbarch->push_dummy_code = push_dummy_code;
 }
 
+int
+gdbarch_extract_arguments_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->extract_arguments != NULL;
+}
+
+void
+gdbarch_extract_arguments (struct gdbarch *gdbarch, struct frame_info *frame, int nargs, struct type **args_in, struct value **args_out, struct type *struct_return_in, struct value **struct_return_out)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->extract_arguments != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_extract_arguments called\n");
+  gdbarch->extract_arguments (frame, nargs, args_in, args_out, struct_return_in, struct_return_out);
+}
+
+void
+set_gdbarch_extract_arguments (struct gdbarch *gdbarch,
+                               gdbarch_extract_arguments_ftype extract_arguments)
+{
+  gdbarch->extract_arguments = extract_arguments;
+}
+
 void
 gdbarch_print_registers_info (struct gdbarch *gdbarch, struct ui_file *file, struct frame_info *frame, int regnum, int all)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 46c5afa..fb0a51c 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -356,6 +356,12 @@ typedef CORE_ADDR (gdbarch_push_dummy_code_ftype) (struct gdbarch *gdbarch, CORE
 extern CORE_ADDR gdbarch_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache);
 extern void set_gdbarch_push_dummy_code (struct gdbarch *gdbarch, gdbarch_push_dummy_code_ftype *push_dummy_code);
 
+extern int gdbarch_extract_arguments_p (struct gdbarch *gdbarch);
+
+typedef void (gdbarch_extract_arguments_ftype) (struct frame_info *frame, int nargs, struct type **args_in, struct value **args_out, struct type *struct_return_in, struct value **struct_return_out);
+extern void gdbarch_extract_arguments (struct gdbarch *gdbarch, struct frame_info *frame, int nargs, struct type **args_in, struct value **args_out, struct type *struct_return_in, struct value **struct_return_out);
+extern void set_gdbarch_extract_arguments (struct gdbarch *gdbarch, gdbarch_extract_arguments_ftype *extract_arguments);
+
 typedef void (gdbarch_print_registers_info_ftype) (struct gdbarch *gdbarch, struct ui_file *file, struct frame_info *frame, int regnum, int all);
 extern void gdbarch_print_registers_info (struct gdbarch *gdbarch, struct ui_file *file, struct frame_info *frame, int regnum, int all);
 extern void set_gdbarch_print_registers_info (struct gdbarch *gdbarch, gdbarch_print_registers_info_ftype *print_registers_info);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index a9ca03d..d04f923 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -478,6 +478,13 @@ M:CORE_ADDR:push_dummy_call:struct value *function, struct regcache *regcache, C
 v:int:call_dummy_location::::AT_ENTRY_POINT::0
 M:CORE_ADDR:push_dummy_code:CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache:sp, funaddr, args, nargs, value_type, real_pc, bp_addr, regcache
 
+# Given a method prototype defined by NARGS args, with each arg of
+# corresponding type in the ARGS_IN type array, build NARGS value
+# objects that extract each of the arguments from the frame.  If
+# STRUCT_RETURN_IN is not NULL, then STRUCT_RETURN_OUT is set to value
+# that extracts the hidden argument of type STRUCT_RETURN_IN
+F:void:extract_arguments:struct frame_info *frame, int nargs, struct type **args_in, struct value **args_out, struct type *struct_return_in, struct value **struct_return_out:frame, nargs, args_in, args_out, struct_return_in, struct_return_out
+
 m:void:print_registers_info:struct ui_file *file, struct frame_info *frame, int regnum, int all:file, frame, regnum, all::default_print_registers_info::0
 M:void:print_float_info:struct ui_file *file, struct frame_info *frame, const char *args:file, frame, args
 M:void:print_vector_info:struct ui_file *file, struct frame_info *frame, const char *args:file, frame, args
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a4e3a22..4ca873d 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2417,6 +2417,54 @@ i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   return sp + 8;
 }
 
+/* Implementation of gdbarch method extract_arguments.  */
+
+static void
+i386_extract_arguments (struct frame_info *frame,
+			int nargs, struct type **args_in,
+			struct value **args_out,
+			struct type *struct_return_in,
+			struct value **struct_return_out)
+{
+  int i;
+  CORE_ADDR sp = 0;
+  int args_space_used = 0;
+
+  /* Get the first arg's slot ( 8(%ebp) ), but unwind, rather than
+     assuming a regular frame with frame pointer.  */
+  sp = get_frame_register_unsigned (get_prev_frame (frame),
+				    I386_ESP_REGNUM);
+
+  if (struct_return_in)
+    {
+      *struct_return_out
+	= value_from_contents_and_address (struct_return_in, NULL, sp);
+
+      args_space_used += 4;
+    }
+
+  for (i = 0; i < nargs; i++)
+    {
+      int len = TYPE_LENGTH (args_in[i]);
+
+      if (i386_16_byte_align_p (args_in[i]))
+	args_space_used = align_up (args_space_used, 16);
+
+      args_out[i]
+	= value_from_contents_and_address (args_in[i], NULL,
+					   (sp + args_space_used));
+
+      /* The System V ABI says that:
+
+	 "An argument's size is increased, if necessary, to make it a
+	 multiple of [32-bit] words.  This may require tail padding,
+	 depending on the size of the argument."
+
+	 This makes sure the stack stays word-aligned.  */
+      args_space_used += align_up (len, 4);
+    }
+}
+
 /* These registers are used for returning integers (and on some
    targets also for returning `struct' and `union' values when their
    size and alignment match an integer type).  */
@@ -7369,6 +7417,8 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_push_dummy_call (gdbarch, i386_push_dummy_call);
   set_gdbarch_frame_align (gdbarch, i386_frame_align);
 
+  set_gdbarch_extract_arguments (gdbarch, i386_extract_arguments);
+
   set_gdbarch_convert_register_p (gdbarch, i386_convert_register_p);
   set_gdbarch_register_to_value (gdbarch,  i386_register_to_value);
   set_gdbarch_value_to_register (gdbarch, i386_value_to_register);

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

* [commit] Ada exception catchpoint support cleanup.
  2011-12-07 10:07 ` Joel Brobecker
  2011-12-07 15:29   ` Pedro Alves
  2011-12-07 22:01   ` Tom Tromey
@ 2011-12-11 17:42   ` Joel Brobecker
  2011-12-11 17:44   ` [commit] Warn if missing debug info for Ada exception catchpoints Joel Brobecker
  2011-12-11 18:04   ` [commit/Ada] improve message when cannot insert Ada exception catchpoint Joel Brobecker
  4 siblings, 0 replies; 18+ messages in thread
From: Joel Brobecker @ 2011-12-11 17:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker

Hello,

This patch cleans up a bit the way we detect which type of runtime
the program uses with respect to Ada exceptions. It also removes
an unnecessary check in ada_exception_sal which is already performed
by ada_exception_support_info_sniffer.

Some of the changes are preparation work for detecting the situation
where the Ada runtime is found, but lacking debugging info.

gdb/ChangeLog:

        * ada-lang.c (ada_has_this_exception_support): New function,
        extracted out of ada_exception_sal and ada_exception_sal.
        (ada_exception_support_info_sniffer): Simplify by using
        ada_has_this_exception_support.
        (ada_exception_sal): Replace unnecessary checks by assertions.
        Minor simplifications.

Tested on x86_64-linux, with both normal and stripped runtimes.
Checked in.

---
 gdb/ChangeLog  |    9 +++++++
 gdb/ada-lang.c |   74 +++++++++++++++++++++++++++++--------------------------
 2 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a4698d8..cbd770b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2011-12-11  Joel Brobecker  <brobecker@adacore.com>
+
+	* ada-lang.c (ada_has_this_exception_support): New function,
+	extracted out of ada_exception_sal and ada_exception_sal.
+	(ada_exception_support_info_sniffer): Simplify by using
+	ada_has_this_exception_support.
+	(ada_exception_sal): Replace unnecessary checks by assertions.
+	Minor simplifications.
+
 2011-12-10  Andrey Smirnov  <andrew.smirnov@gmail.com>
 
 	* breakpoint.c (update_global_location_list): Remove nested
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 7499dfb..97558f1 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -10648,6 +10648,35 @@ static const struct exception_support_info exception_support_info_fallback =
   ada_unhandled_exception_name_addr_from_raise
 };
 
+/* Return nonzero if we can detect the exception support routines
+   described in EINFO.
+
+   This function errors out if an abnormal situation is detected
+   (for instance, if we find the exception support routines, but
+   that support is found to be incomplete).  */
+
+static int
+ada_has_this_exception_support (const struct exception_support_info *einfo)
+{
+  struct symbol *sym;
+
+  /* The symbol we're looking up is provided by a unit in the GNAT runtime
+     that should be compiled with debugging information.  As a result, we
+     expect to find that symbol in the symtabs.  */
+
+  sym = standard_lookup (einfo->catch_exception_sym, NULL, VAR_DOMAIN);
+  if (sym == NULL)
+    return 0;
+
+  /* Make sure that the symbol we found corresponds to a function.  */
+
+  if (SYMBOL_CLASS (sym) != LOC_BLOCK)
+    error (_("Symbol \"%s\" is not a function (class = %d)"),
+           SYMBOL_LINKAGE_NAME (sym), SYMBOL_CLASS (sym));
+
+  return 1;
+}
+
 /* For each executable, we sniff which exception info structure to use
    and cache it in the following global variable.  */
 
@@ -10668,18 +10697,14 @@ ada_exception_support_info_sniffer (void)
     return;
 
   /* Check the latest (default) exception support info.  */
-  sym = standard_lookup (default_exception_support_info.catch_exception_sym,
-                         NULL, VAR_DOMAIN);
-  if (sym != NULL)
+  if (ada_has_this_exception_support (&default_exception_support_info))
     {
       exception_info = &default_exception_support_info;
       return;
     }
 
   /* Try our fallback exception suport info.  */
-  sym = standard_lookup (exception_support_info_fallback.catch_exception_sym,
-                         NULL, VAR_DOMAIN);
-  if (sym != NULL)
+  if (ada_has_this_exception_support (&exception_support_info_fallback))
     {
       exception_info = &exception_support_info_fallback;
       return;
@@ -11693,52 +11718,31 @@ ada_exception_sal (enum exception_catchpoint_kind ex, char *excep_string,
 {
   const char *sym_name;
   struct symbol *sym;
-  struct symtab_and_line sal;
 
   /* First, find out which exception support info to use.  */
   ada_exception_support_info_sniffer ();
 
   /* Then lookup the function on which we will break in order to catch
      the Ada exceptions requested by the user.  */
-
   sym_name = ada_exception_sym_name (ex);
   sym = standard_lookup (sym_name, NULL, VAR_DOMAIN);
 
-  /* The symbol we're looking up is provided by a unit in the GNAT runtime
-     that should be compiled with debugging information.  As a result, we
-     expect to find that symbol in the symtabs.  If we don't find it, then
-     the target most likely does not support Ada exceptions, or we cannot
-     insert exception breakpoints yet, because the GNAT runtime hasn't been
-     loaded yet.  */
-
-  /* brobecker/2006-12-26: It is conceivable that the runtime was compiled
-     in such a way that no debugging information is produced for the symbol
-     we are looking for.  In this case, we could search the minimal symbols
-     as a fall-back mechanism.  This would still be operating in degraded
-     mode, however, as we would still be missing the debugging information
-     that is needed in order to extract the name of the exception being
-     raised (this name is printed in the catchpoint message, and is also
-     used when trying to catch a specific exception).  We do not handle
-     this case for now.  */
-
-  if (sym == NULL)
-    error (_("Unable to break on '%s' in this configuration."), sym_name);
-
-  /* Make sure that the symbol we found corresponds to a function.  */
-  if (SYMBOL_CLASS (sym) != LOC_BLOCK)
-    error (_("Symbol \"%s\" is not a function (class = %d)"),
-           sym_name, SYMBOL_CLASS (sym));
+  /* We can assume that SYM is not NULL at this stage.  If the symbol
+     did not exist, ada_exception_support_info_sniffer would have
+     raised an exception.
 
-  sal = find_function_start_sal (sym, 1);
+     Also, ada_exception_support_info_sniffer should have already
+     verified that SYM is a function symbol.  */
+  gdb_assert (sym != NULL);
+  gdb_assert (SYMBOL_CLASS (sym) == LOC_BLOCK);
 
   /* Set ADDR_STRING.  */
-
   *addr_string = xstrdup (sym_name);
 
   /* Set OPS.  */
   *ops = ada_exception_breakpoint_ops (ex);
 
-  return sal;
+  return find_function_start_sal (sym, 1);
 }
 
 /* Parse the arguments (ARGS) of the "catch exception" command.
-- 
1.7.1

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

* [commit] Warn if missing debug info for Ada exception catchpoints
  2011-12-07 10:07 ` Joel Brobecker
                     ` (2 preceding siblings ...)
  2011-12-11 17:42   ` [commit] Ada exception catchpoint support cleanup Joel Brobecker
@ 2011-12-11 17:44   ` Joel Brobecker
  2011-12-11 18:04   ` [commit/Ada] improve message when cannot insert Ada exception catchpoint Joel Brobecker
  4 siblings, 0 replies; 18+ messages in thread
From: Joel Brobecker @ 2011-12-11 17:44 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker

This is the second part that takes advantage of the cleanup we just
did.

This patch should help the user understand why the debugger is not
able to insert Ada exception catchpoints when the Ada runtime was
stripped of debugging info, as is often the case on many GNU/Linux
distros:

    (gdb) catch exception
    Your Ada runtime appears to be missing some debugging information.
    Cannot insert Ada exception catchpoint in this configuration.

gdb/ChangeLog:

        * ada-lang.c (ada_has_this_exception_support): Raise an error
        if we could find the Ada exception hook in the Ada runtime,
        but no debugging info for that hook.

gdb/testsuite/ChangeLog:

        * gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp: Adjust
        expected output for unsupported case.

Tested on x86_64-linux, with normal and stripped runtime.
Checked in.

---
 gdb/ChangeLog                         |    6 ++++++
 gdb/ada-lang.c                        |   23 ++++++++++++++++++++++-
 gdb/testsuite/ChangeLog               |    5 +++++
 gdb/testsuite/gdb.ada/catch_ex.exp    |    2 +-
 gdb/testsuite/gdb.ada/mi_catch_ex.exp |    2 +-
 5 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cbd770b..31397e0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2011-12-11  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-lang.c (ada_has_this_exception_support): Raise an error
+	if we could find the Ada exception hook in the Ada runtime,
+	but no debugging info for that hook.
+
+2011-12-11  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-lang.c (ada_has_this_exception_support): New function,
 	extracted out of ada_exception_sal and ada_exception_sal.
 	(ada_exception_support_info_sniffer): Simplify by using
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 97558f1..b3c0a24 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -10666,7 +10666,28 @@ ada_has_this_exception_support (const struct exception_support_info *einfo)
 
   sym = standard_lookup (einfo->catch_exception_sym, NULL, VAR_DOMAIN);
   if (sym == NULL)
-    return 0;
+    {
+      /* Perhaps we did not find our symbol because the Ada runtime was
+	 compiled without debugging info, or simply stripped of it.
+	 It happens on some GNU/Linux distributions for instance, where
+	 users have to install a separate debug package in order to get
+	 the runtime's debugging info.  In that situation, let the user
+	 know why we cannot insert an Ada exception catchpoint.
+
+	 Note: Just for the purpose of inserting our Ada exception
+	 catchpoint, we could rely purely on the associated minimal symbol.
+	 But we would be operating in degraded mode anyway, since we are
+	 still lacking the debugging info needed later on to extract
+	 the name of the exception being raised (this name is printed in
+	 the catchpoint message, and is also used when trying to catch
+	 a specific exception).  We do not handle this case for now.  */
+      if (lookup_minimal_symbol (einfo->catch_exception_sym, NULL, NULL))
+	error (_("Your Ada runtime appears to be missing some debugging "
+		 "information.\nCannot insert Ada exception catchpoint "
+		 "in this configuration."));
+
+      return 0;
+    }
 
   /* Make sure that the symbol we found corresponds to a function.  */
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 1a02cf9..f95ac4d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2011-12-11  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp: Adjust
+	expected output for unsupported case.
+
 2011-12-10  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	PR testsuite/12649
diff --git a/gdb/testsuite/gdb.ada/catch_ex.exp b/gdb/testsuite/gdb.ada/catch_ex.exp
index 0ea3256..0cb8874 100644
--- a/gdb/testsuite/gdb.ada/catch_ex.exp
+++ b/gdb/testsuite/gdb.ada/catch_ex.exp
@@ -56,7 +56,7 @@ gdb_test_multiple "catch exception" $msg {
     -re "Catchpoint $any_nb: all Ada exceptions$eol$gdb_prompt $" {
 	pass $msg
     }
-    -re "Cannot (insert catchpoints|break on __gnat_raise_nodefer_with_msg) in this configuration\.$eol$gdb_prompt $" {
+    -re "Your Ada runtime appears to be missing some debugging information.*$eol$gdb_prompt $" {
 	# If the runtime was not built with enough debug information,
 	# or if it was stripped, we can not test exception
 	# catchpoints.
diff --git a/gdb/testsuite/gdb.ada/mi_catch_ex.exp b/gdb/testsuite/gdb.ada/mi_catch_ex.exp
index 4be1cad..584cc00 100644
--- a/gdb/testsuite/gdb.ada/mi_catch_ex.exp
+++ b/gdb/testsuite/gdb.ada/mi_catch_ex.exp
@@ -46,7 +46,7 @@ gdb_test_multiple "catch exception" $msg {
     -re "Catchpoint $any_nb: all Ada exceptions$eol$gdb_prompt $" {
 	pass $msg
     }
-    -re "Cannot (insert catchpoints|break on __gnat_raise_nodefer_with_msg) in this configuration\.\[\r\n\]+$gdb_prompt $" {
+    -re "Your Ada runtime appears to be missing some debugging information.*\[\r\n\]+$gdb_prompt $" {
 	# If the runtime was not built with enough debug information,
 	# or if it was stripped, we can not test exception
 	# catchpoints.
-- 
1.7.1

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

* [commit/Ada] improve message when cannot insert Ada exception catchpoint.
  2011-12-07 10:07 ` Joel Brobecker
                     ` (3 preceding siblings ...)
  2011-12-11 17:44   ` [commit] Warn if missing debug info for Ada exception catchpoints Joel Brobecker
@ 2011-12-11 18:04   ` Joel Brobecker
  4 siblings, 0 replies; 18+ messages in thread
From: Joel Brobecker @ 2011-12-11 18:04 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker

Hello,

This tries to improve the error message pointed out by Pedro.
I think the problem was the generality of the error message, so
fixed thusly.

gdb/ChangeLog:

        * ada-lang.c (ada_exception_support_info_sniffer): Improve
        error message.

Tested on x86_64-linux, checked in.

---
 gdb/ChangeLog  |    5 +++++
 gdb/ada-lang.c |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9089e21..28bc272 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2011-12-11  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-lang.c (ada_exception_support_info_sniffer): Improve
+	error message.
+
+2011-12-11  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-lang.c (struct ada_inferior_data) [exception_info]:
 	New field.
 	(exception_info): Delete.
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index ff43ca4..30a561d 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -10757,7 +10757,7 @@ ada_exception_support_info_sniffer (void)
      out by the linker...  In any case, at this point it is not worth
      supporting this feature.  */
 
-  error (_("Cannot insert catchpoints in this configuration."));
+  error (_("Cannot insert Ada exception catchpoints in this configuration."));
 }
 
 /* True iff FRAME is very likely to be that of a function that is
-- 
1.7.1

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-10 22:53               ` Pedro Alves
@ 2011-12-11 20:33                 ` Joel Brobecker
  2011-12-20 14:53                 ` Tom Tromey
  1 sibling, 0 replies; 18+ messages in thread
From: Joel Brobecker @ 2011-12-11 20:33 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

> Here's a start.  I didn't really try to confirm if I was extracting the
> arguments correctly for any other function bug the Ada runtime's
> function in question, but at least it works in the case we're
> interested in.  :-)

Not bad :). On the other hand, I'm wondering if this is really worth
the effort.  With a stripped libgnat, this is not the worse problem
you are ever going to face.

I think a much worse problem is the fact that you are not going to be
able to print the value of what we call tagged types (aka classes in
C++).  For GDB to decode a tagged variable, it needs to find know how
ada__tags__type_specific_data is defined.  Tagged variables unfortunately
do not reference this type directly, which means that a unit using
tagged variables/types will NOT contain a description of that type.
So, the only unit that contains that data is usually Ada.Tags
(a-tags.ad[sb]). This is why stripping lignat also breaks that feature.

I think that the long term plan would be for the compiler to describe
our tagged variables using an adequate DWARF description, yet to be
determined.

Back to the problem with exceptions, I just checked in a series of
patches that cleanup a bit the support for exceptions. One of the
improvements brings a check for stripped runtimes, and a better
error message when we detect this situation:

    (gdb) catch exception
    Your Ada runtime appears to be missing some debugging information.
    Cannot insert Ada exception catchpoint in this configuration.

That should help a user understand better what's going on.  Perhaps
one last step to help him figure out what to do next is to expand
the documentation about the catch exception command a bit.

> Unfortunately, libgnat is fully stripped (at least on ubuntu), symbol
> table and all,

That reaffirms my belief that it's a battle not worth fighting. Better
to help the users figure out what's going on and how to fix it rather
than trying to support this setup.

-- 
Joel

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-10 22:53               ` Pedro Alves
  2011-12-11 20:33                 ` Joel Brobecker
@ 2011-12-20 14:53                 ` Tom Tromey
  2012-01-10 20:26                   ` Pedro Alves
  1 sibling, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2011-12-20 14:53 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Joel Brobecker, gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> Here's a start.  I didn't really try to confirm if I was extracting the
Pedro> arguments correctly for any other function bug the Ada runtime's
Pedro> function in question, but at least it works in the case we're
Pedro> interested in.  :-)

I think it looks pretty good.

If you put this in, I will look at making the next-over-throw code use
it.

Pedro> +      /* The FULL_NAME pointer is at offset 8 of the object pointed by
Pedro> +	 E.  */
Pedro> +      full_name_val = value_at (void_ptr_type, e + 8);

8 seems wrong on 32 bit.
Now, of course, we need struct layout code... :-)

Tom

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

* Re: [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints
  2011-12-20 14:53                 ` Tom Tromey
@ 2012-01-10 20:26                   ` Pedro Alves
  0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2012-01-10 20:26 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Pedro Alves, Joel Brobecker, gdb-patches

On 12/20/2011 02:51 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Pedro> Here's a start.  I didn't really try to confirm if I was extracting the
> Pedro> arguments correctly for any other function bug the Ada runtime's
> Pedro> function in question, but at least it works in the case we're
> Pedro> interested in.  :-)
> 
> I think it looks pretty good.
> 
> If you put this in, I will look at making the next-over-throw code use
> it.

That sounds nice.  Joel wasn't very keen on it, so I'm not sure about the Ada
parts.  I've now put it at:

  https://github.com/palves/gdb/tree/build_prototype

> Pedro> +      /* The FULL_NAME pointer is at offset 8 of the object pointed by
> Pedro> +	 E.  */
> Pedro> +      full_name_val = value_at (void_ptr_type, e + 8);
> 
> 8 seems wrong on 32 bit.

It looked okay to me, as the two preceding members are 4 bytes
each, on both 32-bit and 64-bit, but ICBW.

> Now, of course, we need struct layout code... :-)

In any case, that'd be nice.  :-)

-- 
Pedro Alves

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

end of thread, other threads:[~2012-01-10 20:26 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-06 17:33 [PATCH] gdb.ada/catch_ex.exp, gdb.ada/mi_catch_ex.exp and unsupported catchpoints Pedro Alves
2011-12-07 10:07 ` Joel Brobecker
2011-12-07 15:29   ` Pedro Alves
2011-12-07 16:10     ` Pedro Alves
2011-12-07 22:01   ` Tom Tromey
2011-12-07 23:18     ` Joel Brobecker
2011-12-09  3:50       ` Tom Tromey
2011-12-09 17:20       ` Pedro Alves
2011-12-09 18:00         ` Tom Tromey
2011-12-09 18:13           ` Pedro Alves
2011-12-09 18:40             ` Tom Tromey
2011-12-10 22:53               ` Pedro Alves
2011-12-11 20:33                 ` Joel Brobecker
2011-12-20 14:53                 ` Tom Tromey
2012-01-10 20:26                   ` Pedro Alves
2011-12-11 17:42   ` [commit] Ada exception catchpoint support cleanup Joel Brobecker
2011-12-11 17:44   ` [commit] Warn if missing debug info for Ada exception catchpoints Joel Brobecker
2011-12-11 18:04   ` [commit/Ada] improve message when cannot insert Ada exception catchpoint Joel Brobecker

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