[RFA] no frame needed when computing address of subprogram

[RFA] no frame needed when computing address of subprogram

[Joel Brobecker]

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

Trying to compute the address of a procedure fails when the inferior
is not running:

        (gdb) p foo'address
        No registers.

To reproduce, use the trivial Ada program pasted at the end of
this email, and compile it with:

        % gnatmake -g foo

Then try the command above.

The problem is inside eval.c:evaluate_subexp_for_address, in the case
handling the OP_VAR_VALUE case. Unless we are in EVAL_AVOID_SIDE_EFFECT
mode, we end up doing:

      if (noside == EVAL_AVOID_SIDE_EFFECTS)
      [...]
      else
        return
          locate_var_value
          (var,
           block_innermost_frame (exp->elts[pc + 1].block));

In particular, we are trying to get the block_innermost_frame,
which doesn't exist in our case. The attached patch changes
that to first check that we need a frame to get the object
address, otherwise, we locate the value without a frame.

2008-01-01  Paul N. Hilfinger  <hilfinger@adacore.com>

        * eval.c (evaluate_subexp_for_address): Provide frame address to
        locate_var_value only if it will be needed.

I also wrote a tiny testcase that verifies the fix.

2008-01-01  Joel Brobecker  <brobecker@adacore.com>

        * gdb.ada/fun_addr/foo.adb: New file.
        * gdb.ada/fun_addr.exp: New testcase.

All tested on x86-linux, no regression.  The patch has also been in
AdaCore's tree since Aug 2003 (shame on us!).

OK to commit?

Thanks,
-- 
Joel

procedure Foo is
begin
   null;
end Foo;


[-- Attachment #2: fun_addr.diff --]
[-- Type: text/plain, Size: 533 bytes --]

Index: eval.c
===================================================================
--- eval.c	(revision 29)
+++ eval.c	(revision 30)
@@ -2150,11 +2150,13 @@ evaluate_subexp_for_address (struct expr
 	  return
 	    value_zero (type, not_lval);
 	}
-      else
+      else if (symbol_read_needs_frame (var))
 	return
 	  locate_var_value
 	  (var,
 	   block_innermost_frame (exp->elts[pc + 1].block));
+      else
+	return locate_var_value (var, NULL);
 
     case OP_SCOPE:
       tem = longest_to_int (exp->elts[pc + 2].longconst);

[-- Attachment #3: fun_addr-testcase.diff --]
[-- Type: text/plain, Size: 2525 bytes --]

Index: gdb.ada/fun_addr.exp
===================================================================
--- gdb.ada/fun_addr.exp	(revision 0)
+++ gdb.ada/fun_addr.exp	(revision 31)
@@ -0,0 +1,44 @@
+# Copyright 2008 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib "ada.exp"
+
+set testdir "fun_addr"
+set testfile "${testdir}/foo"
+set srcfile ${srcdir}/${subdir}/${testfile}.adb
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# Verify that we are able to print the address of a function when
+# the inferior is *not* running (no frame).
+
+gdb_test "print foo'address" \
+         "0x\[0-9a-zA-Z\]+" \
+         "print foo'address"
+
+
Index: gdb.ada/fun_addr/foo.adb
===================================================================
--- gdb.ada/fun_addr/foo.adb	(revision 0)
+++ gdb.ada/fun_addr/foo.adb	(revision 31)
@@ -0,0 +1,19 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+procedure Foo is
+begin
+   null;
+end Foo;

Re: [RFA] no frame needed when computing address of subprogram

[Daniel Jacobowitz]

On Tue, Jan 01, 2008 at 05:46:52AM -0800, Joel Brobecker wrote:
> The problem is inside eval.c:evaluate_subexp_for_address, in the case
> handling the OP_VAR_VALUE case. Unless we are in EVAL_AVOID_SIDE_EFFECT
> mode, we end up doing:
> 
>       if (noside == EVAL_AVOID_SIDE_EFFECTS)
>       [...]
>       else
>         return
>           locate_var_value
>           (var,
>            block_innermost_frame (exp->elts[pc + 1].block));
> 
> In particular, we are trying to get the block_innermost_frame,
> which doesn't exist in our case. The attached patch changes
> that to first check that we need a frame to get the object
> address, otherwise, we locate the value without a frame.

Seems reasonable - but why does it have a block set?

block_innermost_frame:

353       if (block == NULL)
354         return NULL;

c-exp.y:

                              if (symbol_read_needs_frame (sym))
                                {
                                  if (innermost_block == 0 ||
                                      contained_in (block_found,
                                                    innermost_block))
                                    innermost_block = block_found;
                                }

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] no frame needed when computing address of subprogram

[Joel Brobecker]

> Seems reasonable - but why does it have a block set?

Hmmm, that's an interesting question. ada-exp.y:

    static void
    write_var_from_sym (struct block *orig_left_context,
                        struct block *block,
                        struct symbol *sym)
    {
      if (orig_left_context == NULL && symbol_read_needs_frame (sym))
        {
          if (innermost_block == 0
              || contained_in (block, innermost_block))
            innermost_block = block;
        }

But the interesting part is the part that just follows:

    write_exp_elt_opcode (OP_VAR_VALUE);
    write_exp_elt_block (block);
    write_exp_elt_sym (sym);
    write_exp_elt_opcode (OP_VAR_VALUE);

Whereas, for C, we do:

    /* We want to use the selected frame, not
       another more inner frame which happens to
       be in the same block.  */
    write_exp_elt_block (NULL);

IIRC, this is because we provide in Ada support for specifying
the "context" of an expression. See "12.4.6.3 Additions to Ada":

    B::var means "the variable named var that appears in function or
    file B." When B is a file name, you must typically surround it in
    single quotes.
    
We use this particularly in the context of up-level reference, when
you are inside a nested procedure and trying to print the variable
defined in one of the enclosing scopes. Most of the time, this is
possible by directly using the variable name, but sometimes it's not,
because the variable is actually hidden by another variable of the
same name.

The rule where we set (or not) the block is:

    var_or_type:    NAME        %prec VAR
                                    { $$ = write_var_or_type (NULL, $1); }
            |       block NAME  %prec VAR
                                    { $$ = write_var_or_type ($1, $2); }

Still, your question still stands, who sets the block? I found
inside write_var_or_type the following code:

  if (block == NULL)
    block = expression_context_block;

Which at first sight would seem reasonable.  This code predates me,
so I'm not sure of the consequence of removing it. And there might
be some other places down the road where we actually do the same.
I will investigate more.

I suggest, however, that we still go ahead with the patch I sent,
because it protects all languages.

-- 
Joel

Re: [RFA] no frame needed when computing address of subprogram

[Daniel Jacobowitz]

On Wed, Jan 02, 2008 at 05:49:10AM -0800, Joel Brobecker wrote:
> I suggest, however, that we still go ahead with the patch I sent,
> because it protects all languages.

Yes, I agree; the patch is OK.  I just wanted to understand the
difference for Ada.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] no frame needed when computing address of subprogram

[Joel Brobecker]

> > Still, your question still stands, who sets the block? I found
> > inside write_var_or_type the following code:
> > 
> >   if (block == NULL)
> >     block = expression_context_block;
> > 
> > Which at first sight would seem reasonable.  This code predates me,
> > so I'm not sure of the consequence of removing it. And there might
> > be some other places down the road where we actually do the same.
> > I will investigate more.

Mamia mia, mucho complicacion if you remove this code... The function
relies on the block being set properly to do the lookup. For instance,
just slightly further down in that function, you have the following
code:

          nsyms = ada_lookup_symbol_list (encoded_name, block,
                                          VAR_DOMAIN, &syms);

If the block is not set, then the local symbols will not be found.
There may be other places where we have the same issue, I didn't
look further.  Sooooo....

> Yes, I agree; the patch is OK.  I just wanted to understand the
> difference for Ada.

... Thanks! I checked the patch in.

-- 
Joel