This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: RFA: Fix tracepoints for LOC_COMPUTED - sort of
- From: Jim Blandy <jimb at redhat dot com>
- To: Daniel Jacobowitz <drow at mvista dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: 24 Feb 2003 16:50:27 -0500
- Subject: Re: RFA: Fix tracepoints for LOC_COMPUTED - sort of
- References: <20030223183002.GA13814@nevyn.them.org>
Daniel Jacobowitz <drow at mvista dot com> writes:
> Like the description function, this is pretty minimal; I don't have time to
> write a full evaluator right now, and I don't have access to a tracepoint
> stub, so I suggest someone who does implement this properly. For now,
> though, this handles the most likely types: DW_OP_reg and DW_OP_fbreg. It's
> enough to fix the test failures; it's correct as far as it goes; and it
> properly raises an error if it gets confused.
>
> Is this OK? Should I file a PR about the need for a proper
> evaluator?
Well, the problem is likely to sit around indefinitely:
1) I don't think anyone's using tracepoints at the moment.
2) Compiling Dwarf 2 bytecodes into ax bytecodes is, in general, not
a simple thing at all.
So its importance and difficulty both work against it. Would a
comment be better in this sort of situation? I don't know.
> Index: dwarf2expr.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 dwarf2expr.c
> --- dwarf2expr.c 21 Feb 2003 15:24:17 -0000 1.1
> +++ dwarf2expr.c 23 Feb 2003 18:24:14 -0000
> @@ -115,7 +115,7 @@ dwarf_expr_eval (struct dwarf_expr_conte
> by R, and return the new value of BUF. Verify that it doesn't extend
> past BUF_END. */
>
> -static unsigned char *
> +unsigned char *
> read_uleb128 (unsigned char *buf, unsigned char *buf_end, ULONGEST * r)
> {
> unsigned shift = 0;
> @@ -141,7 +141,7 @@ read_uleb128 (unsigned char *buf, unsign
> by R, and return the new value of BUF. Verify that it doesn't extend
> past BUF_END. */
>
> -static unsigned char *
> +unsigned char *
> read_sleb128 (unsigned char *buf, unsigned char *buf_end, LONGEST * r)
> {
> unsigned shift = 0;
> Index: dwarf2expr.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2expr.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 dwarf2expr.h
> --- dwarf2expr.h 21 Feb 2003 15:24:17 -0000 1.1
> +++ dwarf2expr.h 23 Feb 2003 18:24:14 -0000
> @@ -94,4 +94,10 @@ void dwarf_expr_eval (struct dwarf_expr_
> size_t len);
> CORE_ADDR dwarf_expr_fetch (struct dwarf_expr_context *ctx, int n);
>
> +
> +unsigned char *read_uleb128 (unsigned char *buf, unsigned char *buf_end,
> + ULONGEST * r);
> +unsigned char *read_sleb128 (unsigned char *buf, unsigned char *buf_end,
> + LONGEST * r);
> +
> #endif
You know, dwarf2read.c's read_{un,}signed_leb128 don't actually need
their BFD argument for anything. The call to bfd_get_8 ignores it;
it's just a char fetch anyway. In the future, I wonder if we could
just use those everywhere.
> Index: dwarf2loc.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 dwarf2loc.c
> --- dwarf2loc.c 21 Feb 2003 15:24:17 -0000 1.1
> +++ dwarf2loc.c 23 Feb 2003 18:24:14 -0000
> @@ -26,6 +26,8 @@
> #include "gdbcore.h"
> #include "target.h"
> #include "inferior.h"
> +#include "ax.h"
> +#include "ax-gdb.h"
>
> #include "elf/dwarf2.h"
> #include "dwarf2expr.h"
> @@ -277,11 +279,58 @@ locexpr_describe_location (struct symbol
> return 1;
> }
>
> +void
> +locexpr_tracepoint_var_ref (struct symbol * symbol, struct agent_expr * ax,
> + struct axs_value * value)
> +{
> + /* FIXME: This one is also a bit minimal. */
> + struct dwarf2_locexpr_baton *dlbaton = SYMBOL_LOCATION_BATON (symbol);
> +
> + if (dlbaton->size == 0)
> + error ("Optimized out.");
The error message ought to be a complete sentence, and mention
SYMBOL_NAME.
> + if (dlbaton->size == 1
> + && dlbaton->data[0] >= DW_OP_reg0
> + && dlbaton->data[0] <= DW_OP_reg31)
> + {
> + value->kind = axs_lvalue_register;
> + value->u.reg = dlbaton->data[0] - DW_OP_reg0;
> + }
> + else if (dlbaton->data[0] == DW_OP_regx)
> + {
> + ULONGEST reg;
> + read_uleb128 (dlbaton->data + 1, dlbaton->data + dlbaton->size,
> + ®);
> + value->kind = axs_lvalue_register;
> + value->u.reg = reg;
> + }
> + else if (dlbaton->data[0] == DW_OP_fbreg)
> + {
> + /* And this is worse than just minimal; we should honor the frame base
> + as above. */
> + int frame_reg;
> + LONGEST frame_offset;
> +
> + TARGET_VIRTUAL_FRAME_POINTER (ax->scope, &frame_reg, &frame_offset);
> + ax_reg (ax, frame_reg);
> + ax_const_l (ax, frame_offset);
> + ax_simple (ax, aop_add);
> +
> + read_sleb128 (dlbaton->data + 1, dlbaton->data + dlbaton->size,
> + &frame_offset);
> + ax_const_l (ax, frame_offset);
> + ax_simple (ax, aop_add);
> + value->kind = axs_lvalue_memory;
> + }
> + else
> + error ("Can't do it.");
Gotta have a better error message, dude. It only takes a few
seconsd. Think of the poor user.
You should check that that's actually the end of the expression, too.