[RFA] dwarf2loc, guard against null

[RFA] dwarf2loc, guard against null

[Jerome Guitton]

[-- Attachment #1: Type: text/plain, Size: 189 bytes --]


get_frame_function can return null; in this case, crash...

OK to apply?

2007-09-17  Jerome Guitton  <guitton@adacore.com>

	* dwarf2loc.c (dwarf_expr_frame_base): Guard against NULL.




[-- Attachment #2: dwarf2loc.c.diff --]
[-- Type: text/x-diff, Size: 436 bytes --]

Index: dwarf2loc.c
===================================================================
--- dwarf2loc.c	(revision 14570)
+++ dwarf2loc.c	(working copy)
@@ -151,6 +151,9 @@ dwarf_expr_frame_base (void *baton, gdb_
 
   framefunc = get_frame_function (debaton->frame);
 
+  if (!framefunc)
+    error (_("No frame function."));
+
   if (SYMBOL_OPS (framefunc) == &dwarf2_loclist_funcs)
     {
       struct dwarf2_loclist_baton *symbaton;

Re: [RFA] dwarf2loc, guard against null

[Daniel Jacobowitz]

On Mon, Sep 17, 2007 at 02:04:43PM +0200, Jerome Guitton wrote:
> 
> get_frame_function can return null; in this case, crash...
> 
> OK to apply?
> 
> 2007-09-17  Jerome Guitton  <guitton@adacore.com>
> 
> 	* dwarf2loc.c (dwarf_expr_frame_base): Guard against NULL.

I'm not sure - there must be another bug so it might be this should be
a gdb_assert instead.  The DW_AT_frame_base that brought us here came
from .debug_info inside an entry for a function.  So we must have
found the function at some point.  But we didn't find the function
this time - why not?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] dwarf2loc, guard against null

[Jerome Guitton]

Daniel Jacobowitz (drow@false.org):

> I'm not sure - there must be another bug so it might be this should be
> a gdb_assert instead.  The DW_AT_frame_base that brought us here came
> from .debug_info inside an entry for a function.  So we must have
> found the function at some point.  But we didn't find the function
> this time - why not?

Hmmm... Let me have a closer look. In my case, the problem appears when
debugging an application on VxWorks, with this scenario:
* breakpoint hit;
* print the address of a variable in the current frame (print &DB);
* resume the execution; program exited normally;
* retry to print the variable => end up in dwarf_expr_frame_base,
no frame, crash.

We are (probably) still using a wrong .debug_info at this point.

Re: [RFA] dwarf2loc, guard against null

[Daniel Jacobowitz]

On Mon, Sep 17, 2007 at 02:26:54PM +0200, Jerome Guitton wrote:
> * retry to print the variable => end up in dwarf_expr_frame_base,
> no frame, crash.
> 
> We are (probably) still using a wrong .debug_info at this point.

Yeah.  Shouldn't you have gotten a "no such variable in this scope"
error?  I'm not sure how symbol lookup found a local variable without
any frame.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] dwarf2loc, guard against null

[Jerome Guitton]

Daniel Jacobowitz (drow@false.org):

> On Mon, Sep 17, 2007 at 02:26:54PM +0200, Jerome Guitton wrote:
> > * retry to print the variable => end up in dwarf_expr_frame_base,
> > no frame, crash.
> >
> > We are (probably) still using a wrong .debug_info at this point.
>
> Yeah.  Shouldn't you have gotten a "no such variable in this scope"
> error?  I'm not sure how symbol lookup found a local variable without
> any frame.


Right. Actually, it got the selected frame (deprecated_selected_frame)
in deprecated_safe_get_selected_frame. This function reads:

struct frame_info *
deprecated_safe_get_selected_frame (void)
{
  if (!target_has_registers || !target_has_stack || !target_has_memory)
    return NULL;
  return get_selected_frame (NULL);
}


When the inferior is dead, target_has_registers (resp.
target_has_memory, target_has_stack) should be false and
deprecated_safe_get_selected_frame should return NULL. This is the bug
in my vxWorks-specific backend (I should pop some target vector at
mourn_inferior, I guess).

Anyway, when deprecated_safe_get_selected_frame returns NULL, I get the
message "Address of "db" is unknown". Much better.


Now back to the original problem: assertion or error? I guess that the
question is: in which cases get_frame_block can return NULL?

Re: [RFA] dwarf2loc, guard against null

[Daniel Jacobowitz]

On Mon, Sep 17, 2007 at 03:10:52PM +0200, Jerome Guitton wrote:
> When the inferior is dead, target_has_registers (resp.
> target_has_memory, target_has_stack) should be false and
> deprecated_safe_get_selected_frame should return NULL. This is the bug
> in my vxWorks-specific backend (I should pop some target vector at
> mourn_inferior, I guess).

Yes, that's probably it.  If you stay connected to the target after
mourning, you may want to look at the new target_mark_exited.  I
haven't had time to contribute the target extended-remote bits that
go along with that yet but soon, soon...

> Now back to the original problem: assertion or error? I guess that the
> question is: in which cases get_frame_block can return NULL?

Assertion, please.  If we found a frame-relative symbol then it was
certainly within some function associated with a frame.  If we can't
find the frame, something has gone wrong.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] dwarf2loc, guard against null

[Jerome Guitton]

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

Daniel Jacobowitz (drow@false.org):

> Yes, that's probably it.  If you stay connected to the target after
> mourning, you may want to look at the new target_mark_exited.  I
> haven't had time to contribute the target extended-remote bits that
> go along with that yet but soon, soon...

OK, thanks for your help.


> Assertion, please.  If we found a frame-relative symbol then it was
> certainly within some function associated with a frame.  If we can't
> find the frame, something has gone wrong.

OK, makes sense. I will commit the patch in attachment.

2007-09-17  Jerome Guitton  <guitton@adacore.com>

	* dwarf2loc.c (dwarf_expr_frame_base): Guard against NULL.
	Part of G917-008.
	* Makefile.in (dwarf2loc.o): Depend on gdb_assert.h.

[-- Attachment #2: dwarf2loc.c.diff --]
[-- Type: text/x-diff, Size: 1821 bytes --]

Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.936
diff -u -p -r1.936 Makefile.in
--- gdb/Makefile.in	5 Sep 2007 00:14:02 -0000	1.936
+++ gdb/Makefile.in	17 Sep 2007 13:41:35 -0000
@@ -1963,7 +1963,7 @@ dwarf2-frame.o: dwarf2-frame.c $(defs_h)
 dwarf2loc.o: dwarf2loc.c $(defs_h) $(ui_out_h) $(value_h) $(frame_h) \
 	$(gdbcore_h) $(target_h) $(inferior_h) $(ax_h) $(ax_gdb_h) \
 	$(regcache_h) $(objfiles_h) $(exceptions_h) $(elf_dwarf2_h) \
-	$(dwarf2expr_h) $(dwarf2loc_h) $(gdb_string_h)
+	$(dwarf2expr_h) $(dwarf2loc_h) $(gdb_string_h) $(gdb_assert_h)
 dwarf2read.o: dwarf2read.c $(defs_h) $(bfd_h) $(symtab_h) $(gdbtypes_h) \
 	$(objfiles_h) $(elf_dwarf2_h) $(buildsym_h) $(demangle_h) \
 	$(expression_h) $(filenames_h) $(macrotab_h) $(language_h) \
Index: gdb/dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.45
diff -u -p -r1.45 dwarf2loc.c
--- gdb/dwarf2loc.c	23 Aug 2007 18:08:28 -0000	1.45
+++ gdb/dwarf2loc.c	17 Sep 2007 13:41:36 -0000
@@ -37,6 +37,7 @@
 #include "dwarf2loc.h"
 
 #include "gdb_string.h"
+#include "gdb_assert.h"
 
 /* A helper function for dealing with location lists.  Given a
    symbol baton (BATON) and a pc value (PC), find the appropriate
@@ -145,6 +146,11 @@ dwarf_expr_frame_base (void *baton, gdb_
 
   framefunc = get_frame_function (debaton->frame);
 
+  /* If we found a frame-relative symbol then it was certainly within
+     some function associated with a frame. If we can't find the frame,
+     something has gone wrong.  */
+  gdb_assert (framefunc != NULL);
+
   if (SYMBOL_OPS (framefunc) == &dwarf2_loclist_funcs)
     {
       struct dwarf2_loclist_baton *symbaton;

Re: [RFA] dwarf2loc, guard against null

[Daniel Jacobowitz]

On Mon, Sep 17, 2007 at 03:42:46PM +0200, Jerome Guitton wrote:
> OK, makes sense. I will commit the patch in attachment.
> 
> 2007-09-17  Jerome Guitton  <guitton@adacore.com>
> 
> 	* dwarf2loc.c (dwarf_expr_frame_base): Guard against NULL.
> 	Part of G917-008.
> 	* Makefile.in (dwarf2loc.o): Depend on gdb_assert.h.

Thanks!

-- 
Daniel Jacobowitz
CodeSourcery