Re: [patch 11/12] entryval#2: @entry values even for references

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Wed, 21 Sep 2011 17:32:44 +0200, Pedro Alves wrote:
> But not in the example I gave?  Or are you saying that there can't be
> an entryval whose type is a struct?

Theoretically there can be, just:

(a) GCC currently cannot produce DW_AT_GNU_call_site_data_value (for the
    struct content) in such case (it produces only DW_AT_GNU_call_site_value
    - for the struct referencing pointer).

(b) Even if I create such DWARF attribute by hand the expression `s@entry.b'
    for parameter `S &s' of type `class S { char a, b; };' never reaches this
    point of code because:
    #0  coerce_ref_if_computed (arg=0x202c8c0) at value.c:3096
    #1  in coerce_ref (arg=0x202c8c0) at value.c:3115
    #2  in coerce_array (arg=0x202c8c0) at value.c:3132
    #3  in value_struct_elt (argp=0x7fffffffd258, args=0x0, name=0x26fccd0 "b", static_memfuncp=0x0, err=0xea593f "structure") at valops.c:2306
    #4  in evaluate_subexp_standard (expect_type=0x0, exp=0x26fcc90, pos=0x7fffffffd68c, noside=EVAL_NORMAL) at eval.c:1966
    #5  in evaluate_subexp_c (expect_type=0x0, exp=0x26fcc90, pos=0x7fffffffd68c, noside=EVAL_NORMAL) at c-lang.c:720
    #6  in evaluate_subexp (expect_type=0x0, exp=0x26fcc90, pos=0x7fffffffd68c, noside=EVAL_NORMAL) at eval.c:76
    #7  in evaluate_expression (exp=0x26fcc90) at eval.c:151
    #8  in print_command_1 (exp=0x7fffffffde6f "s@entry.b", inspect=0, voidprint=1) at ./printcmd.c:973

    So in all the cases like this one c_val_print already gets original_value
    which is no longer "magic" in any way, it is already coerce-ref-ed.

(c) Your:
    On Fri, 16 Sep 2011 14:26:09 +0200, Pedro Alves wrote:
    > E.g., `struct { long l; long &r } foo;', and we're `p foo', which descends
    > into printing R at FOO + offset of R ?

    also cannot lead much to anything as if the parameter is of
    struct s { long l; long &r; };
    void f (struct s p) {}
    then `print p@entry' just does not get `entry_data_value_funcs' at all
    because `p' is not TYPE_CODE_REF.  And anyway after any operation with `p'
    it gets coerce-ref-ed, it is no longer "magic" and that new code is nop.


> If that's the case, I'm okay with the assert for now.

I think so.


> > If it should work really correctly there are more cases to catch such as
> > f77_print_array (*) being passed original_value etc.  The correct way would be
> > the rework to full struct value * printing
> > 	http://sourceware.org/ml/gdb-patches/2010-10/msg00127.html
> > but I did not want to get deeper in the patchset dependencies.
> 
> I'm not sure that'd be necessary.

As I do not see possible how to exploit the current gdb_assert()s I do not see
which parts to fix.


> (the first 2 steps are complete, btw).

I see, great, (2) thanks to val_print_scalar_formatted.


> That would change the running-offset scheme,
> allowing value contents to be even more lazy, but that does not
> mean the current scheme doesn't work.

From the larger picture the current API does not work as `struct value' should
reference all the memory it needs, not just the single contiguous block
value->contents.  It breaks archer-jankratochvil-vla where single value uses
discontiguous memory areas.  With FSF GDB one can see it on untracked virtual
method table:

class C {
  virtual void m();
} c;
void C::m() {}

(gdb) file 8.o
Reading symbols from 8.o...done.
(gdb) set $a=c
(gdb) p $a.m
$1 = {void (C * const)} 0
(gdb) file
No executable file now.
Discard symbol table from `8.o'? (y or n) y
No symbol file now.
(gdb) p $a.m
There is no member named m.


> E.g, if entryval's lval_funcs->coerce_ref took an embedded_offset,

The problem is that if embedded_offset != 0 for entry_data_value_funcs value
it means one cannot derefence it.

It is like a request to coerce_ref `struct { long &r; } foo;' with embedded
offset != 0.  It is error, its dereference exceeds its ->contents length.

entry_data_value_funcs value is created with embedded_offset == 0 and any
futher operation has to coerce_ref it first.


> and returned the target value with value_offset adjusted

The target value is at a new/different memory location, I do not see why
embedded_offset should map to value_offset there.  The existing
value_at+unpack_pointer code also does not set value_offset.

You are rather suggesting handling pointed_to_offset != 0 remapping during the
coerce_ref but that seems very out of topic here, it is also always 0 for
entry_data_value_funcs.


> you'd be almost there?

Thanks for thinking more about it but I see entry_data_value_funcs as
a special kind of value.  Sure it is not perfect as for example entry-value
`int *' will dereference to actual-value (not entry-value) `int'.


> The problem is that you share/refcount the target
> value, so we'd either need to unshare it so we could have copies with
> different offsets set, or we'd add a new lval_computed type that serves
> as view with offset into a current value, as I proposed in that URL,
> but without changing everything else in valprint.

I do not think this comment is relevant presuming offset really should
be == 0.


Thanks for the review,
Jan