[RFA] dwarf2loc.c, loclist_read_variable, Assert frame not null

[RFA] dwarf2loc.c, loclist_read_variable, Assert frame not null

[Michael Snyder]

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

Now, I've heard a rumor that frame-infos can never be null.

If that's true, then instead of this patch, we should just
delete the earlier check for null.

OK?



[-- Attachment #2: null5.txt --]
[-- Type: text/plain, Size: 790 bytes --]

2011-03-04  Michael Snyder  <msnyder@vmware.com>

	* dwarf2loc.c (loclist_read_variable): Assert frame not null.

Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.110
diff -u -p -u -p -r1.110 dwarf2loc.c
--- dwarf2loc.c	27 Feb 2011 00:01:12 -0000	1.110
+++ dwarf2loc.c	4 Mar 2011 22:34:24 -0000
@@ -2687,8 +2687,11 @@ loclist_read_variable (struct symbol *sy
       set_value_optimized_out (val, 1);
     }
   else
-    val = dwarf2_evaluate_loc_desc (SYMBOL_TYPE (symbol), frame, data, size,
-				    dlbaton->per_cu);
+    {
+      gdb_asert (frame);
+      val = dwarf2_evaluate_loc_desc (SYMBOL_TYPE (symbol), frame,
+				      data, size, dlbaton->per_cu);
+    }
 
   return val;
 }

Re: [RFA] dwarf2loc.c, loclist_read_variable, Assert frame not null

[Ulrich Weigand]

Michael Snyder wrote:

> Now, I've heard a rumor that frame-infos can never be null.
> 
> If that's true, then instead of this patch, we should just
> delete the earlier check for null.

It's not quite that simple -- with the ->read_variable callbacks,
there are two classes of symbols: those that can only be evaluated
relative to a frame, and those that can be evaluated without any
reference to a frame (e.g. globals).

For the latter, it is valid to pass a NULL frame argument to the
->read_variable callback, while it is invalid for the former.  To
distinguish between the two, the caller is supposed to call the
->read_needs_frame callback before calling ->read_variable.

However, in the particular case of *location list* symbols, we
currently always require a frame anyway.  See the corresponding 
->read_needs_frame implementation:

static int
loclist_read_needs_frame (struct symbol *symbol)
{
  /* If there's a location list, then assume we need to have a frame
     to choose the appropriate location expression.  With tracking of
     global variables this is not necessarily true, but such tracking
     is disabled in GCC at the moment until we figure out how to
     represent it.  */

  return 1;
}


So, given this situation, I'd suggest to remove the check for a
NULL frame in loclist_read_variable, and replace it with a global
assertion that frame is non-NULL -- but with a comment refering
to the explanation in loclist_read_needs_frame for this specific
case ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [RFA] dwarf2loc.c, loclist_read_variable, Assert frame not null

[Pedro Alves]

On Friday 04 March 2011 22:37:29, Michael Snyder wrote:
> +      gdb_asert (frame);

Please make sure patches compile before posting them.

-- 
Pedro Alves

Re: [RFA] dwarf2loc.c, loclist_read_variable, Assert frame not null

[Michael Snyder]

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

Pedro Alves wrote:
> On Friday 04 March 2011 22:37:29, Michael Snyder wrote:
>> +      gdb_asert (frame);
> 
> Please make sure patches compile before posting them.
> 

My bad.

Corrected patch attached.



[-- Attachment #2: null5.txt --]
[-- Type: text/plain, Size: 791 bytes --]

2011-03-04  Michael Snyder  <msnyder@vmware.com>

	* dwarf2loc.c (loclist_read_variable): Assert frame not null.

Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.110
diff -u -p -u -p -r1.110 dwarf2loc.c
--- dwarf2loc.c	27 Feb 2011 00:01:12 -0000	1.110
+++ dwarf2loc.c	4 Mar 2011 22:34:24 -0000
@@ -2687,8 +2687,11 @@ loclist_read_variable (struct symbol *sy
       set_value_optimized_out (val, 1);
     }
   else
-    val = dwarf2_evaluate_loc_desc (SYMBOL_TYPE (symbol), frame, data, size,
-				    dlbaton->per_cu);
+    {
+      gdb_assert (frame);
+      val = dwarf2_evaluate_loc_desc (SYMBOL_TYPE (symbol), frame,
+				      data, size, dlbaton->per_cu);
+    }
 
   return val;
 }