Funky code in gnuv2_virtual_fn_field

Funky code in gnuv2_virtual_fn_field

[Jim Blandy]

I'm looking at lines 112--118 in gnu-v2-abi.c:

  if (TYPE_TARGET_TYPE (context) != type1)
    {
      value_ptr tmp = value_cast (context, value_addr (arg1));
      VALUE_POINTED_TO_OFFSET (tmp) = 0;
      arg1 = value_ind (tmp);
      type1 = check_typedef (VALUE_TYPE (arg1));
    }

This looks fishy to me.  If we smash the POINTED_TO_OFFSET without
smashing the ENCLOSING_TYPE in a corresponding manner, and then we
indirect through that pointer, don't we get a value whose
ENCLOSING_TYPE is set, but whose address points to the embedded
object, and not the enclosing object?

Re: Funky code in gnuv2_virtual_fn_field

[Daniel Berlin]

Jim Blandy <jimb@zwingli.cygnus.com> writes:

> I'm looking at lines 112--118 in gnu-v2-abi.c:
> 
>   if (TYPE_TARGET_TYPE (context) != type1)
>     {
>       value_ptr tmp = value_cast (context, value_addr (arg1));
>       VALUE_POINTED_TO_OFFSET (tmp) = 0;
>       arg1 = value_ind (tmp);
>       type1 = check_typedef (VALUE_TYPE (arg1));
>     }
> 
> This looks fishy to me.  If we smash the POINTED_TO_OFFSET without
> smashing the ENCLOSING_TYPE in a corresponding manner, and then we
> indirect through that pointer, don't we get a value whose
> ENCLOSING_TYPE is set, but whose address points to the embedded
> object, and not the enclosing object?

Yup.
However, although it's not documented anywhere, value_cast
approriately smashes the enclosing type.
In fact, it should also smash the pointed_to_offset when necessary,
making this explicit smashing pointless, and possibly incorrect.

Though now that i look at it some more, I'm not positive value_cast is
doing the right thing in all cases.  It's pretty hard to follow,
however, and I just got home from a 7 hour car ride (graduated
yesterday, and moved out), so i may just be loopy.

IMHO, in any case, we shouldn't be needing to set the
pointed_to_offset here.  If we have to, value_cast is doing something
wrong, or not enough of the right thing.
This is because all we are trying to do is a simple cast, which is what
value_cast is supposed to do for us. If we have to start mucking
around with it's results to get a correct value, then it's not doing
it's job right, or completely.

-- 
"I hooked up my accelerator pedal in my car to my brake lights.
I hit the gas, people behind me stop, and I'm gone.
"-Steven Wright

RFA: Re: Funky code in gnuv2_virtual_fn_field

[Jim Blandy]

Daniel Berlin <dan@cgsoftware.com> writes:
> Jim Blandy <jimb@zwingli.cygnus.com> writes:
> 
> > I'm looking at lines 112--118 in gnu-v2-abi.c:
> > 
> >   if (TYPE_TARGET_TYPE (context) != type1)
> >     {
> >       value_ptr tmp = value_cast (context, value_addr (arg1));
> >       VALUE_POINTED_TO_OFFSET (tmp) = 0;
> >       arg1 = value_ind (tmp);
> >       type1 = check_typedef (VALUE_TYPE (arg1));
> >     }
> > 
> > This looks fishy to me.  If we smash the POINTED_TO_OFFSET without
> > smashing the ENCLOSING_TYPE in a corresponding manner, and then we
> > indirect through that pointer, don't we get a value whose
> > ENCLOSING_TYPE is set, but whose address points to the embedded
> > object, and not the enclosing object?
> 
> Yup.
> However, although it's not documented anywhere, value_cast
> approriately smashes the enclosing type.

That's what I was afraid of.  (I *hate* it when GDB does something
with a `struct value' that isn't really legal, but just happens to be
okay because we know internal details about where that `struct value'
came from...)

> IMHO, in any case, we shouldn't be needing to set the
> pointed_to_offset here.  If we have to, value_cast is doing something
> wrong, or not enough of the right thing.
> This is because all we are trying to do is a simple cast, which is what
> value_cast is supposed to do for us. If we have to start mucking
> around with it's results to get a correct value, then it's not doing
> it's job right, or completely.

Great.  So how about this patch?

2001-05-22  Jim Blandy  <jimb@redhat.com>

	* gnu-v2-abi.c (gnuv2_virtual_fn_field): There's no need to clear
	VALUE_POINTED_TO_OFFSET here; if value_cast doesn't return a
	useful value, then we should fix that instead.

Index: gdb/gnu-v2-abi.c
===================================================================
RCS file: /cvs/src/src/gdb/gnu-v2-abi.c,v
retrieving revision 1.2
diff -c -r1.2 gnu-v2-abi.c
*** gdb/gnu-v2-abi.c	2001/05/12 04:01:16	1.2
--- gdb/gnu-v2-abi.c	2001/05/22 21:14:35
***************
*** 111,117 ****
    if (TYPE_TARGET_TYPE (context) != type1)
      {
        value_ptr tmp = value_cast (context, value_addr (arg1));
-       VALUE_POINTED_TO_OFFSET (tmp) = 0;
        arg1 = value_ind (tmp);
        type1 = check_typedef (VALUE_TYPE (arg1));
      }
--- 111,116 ----

Re: RFA: Re: Funky code in gnuv2_virtual_fn_field

[Daniel Berlin]

Jim Blandy <jimb@zwingli.cygnus.com> writes:

> Daniel Berlin <dan@cgsoftware.com> writes:
> > Jim Blandy <jimb@zwingli.cygnus.com> writes:
> > 
> > > I'm looking at lines 112--118 in gnu-v2-abi.c:
> > > 
> > >   if (TYPE_TARGET_TYPE (context) != type1)
> > >     {
> > >       value_ptr tmp = value_cast (context, value_addr (arg1));
> > >       VALUE_POINTED_TO_OFFSET (tmp) = 0;
> > >       arg1 = value_ind (tmp);
> > >       type1 = check_typedef (VALUE_TYPE (arg1));
> > >     }
> > > 
> > > This looks fishy to me.  If we smash the POINTED_TO_OFFSET without
> > > smashing the ENCLOSING_TYPE in a corresponding manner, and then we
> > > indirect through that pointer, don't we get a value whose
> > > ENCLOSING_TYPE is set, but whose address points to the embedded
> > > object, and not the enclosing object?
> > 
> > Yup.
> > However, although it's not documented anywhere, value_cast
> > approriately smashes the enclosing type.
> 
> That's what I was afraid of.  (I *hate* it when GDB does something
> with a `struct value' that isn't really legal, but just happens to be
> okay because we know internal details about where that `struct value'
> came from...)

This is actually why i stopped using value_ptrs when doing the
location expression evaluation.  Even the value operations and the
read_var_value routine seem to "know too much" about struct value, and
don't just do what they should.

> 
> > IMHO, in any case, we shouldn't be needing to set the
> > pointed_to_offset here.  If we have to, value_cast is doing something
> > wrong, or not enough of the right thing.
> > This is because all we are trying to do is a simple cast, which is what
> > value_cast is supposed to do for us. If we have to start mucking
> > around with it's results to get a correct value, then it's not doing
> > it's job right, or completely.
> 
> Great.  So how about this patch?
> 

Approved.

> 2001-05-22  Jim Blandy  <jimb@redhat.com>
> 
> 	* gnu-v2-abi.c (gnuv2_virtual_fn_field): There's no need to clear
> 	VALUE_POINTED_TO_OFFSET here; if value_cast doesn't return a
> 	useful value, then we should fix that instead.
> 
> Index: gdb/gnu-v2-abi.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gnu-v2-abi.c,v
> retrieving revision 1.2
> diff -c -r1.2 gnu-v2-abi.c
> *** gdb/gnu-v2-abi.c	2001/05/12 04:01:16	1.2
> --- gdb/gnu-v2-abi.c	2001/05/22 21:14:35
> ***************
> *** 111,117 ****
>     if (TYPE_TARGET_TYPE (context) != type1)
>       {
>         value_ptr tmp = value_cast (context, value_addr (arg1));
> -       VALUE_POINTED_TO_OFFSET (tmp) = 0;
>         arg1 = value_ind (tmp);
>         type1 = check_typedef (VALUE_TYPE (arg1));
>       }
> --- 111,116 ----

-- 
""-Steven Wright