[DTrace-devel] [PATCH 01/20] Correct storage size assignment for variables

[Kris Van Hees]

On Tue, Jun 08, 2021 at 02:05:00AM -0400, Eugene Loh wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> 
> I understand the commit message, but I don't understand the parser and 
> so I cannot say I really reviewed this.  I'm curious though: no tests?  
> Or, are the tests in the alignment patch?  If so, should the two patches 
> be combined?

Test added.

> On 6/2/21 1:47 AM, Kris Van Hees wrote:
> > The implementation of global and local variables using BPF maps added
> > calls to dt_ident_set_storage() to set the offset and size of the data
> > storage for each variable.  It was setting the storage requirement for
> > any automatically declared variable to 8 bytes regardless of its type.
> > Since the datatype of such a variable is determined dynamically based
> > on its use, the storage assignment must be done once the datatype is
> > known.
> >
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> >   libdtrace/dt_parser.c | 14 +++++++-------
> >   1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/libdtrace/dt_parser.c b/libdtrace/dt_parser.c
> > index 948ef2d8..9d71cd43 100644
> > --- a/libdtrace/dt_parser.c
> > +++ b/libdtrace/dt_parser.c
> > @@ -1661,10 +1661,10 @@ dt_node_decl(void)
> >   			if (idp == NULL)
> >   				longjmp(yypcb->pcb_jmpbuf, EDT_NOMEM);
> >   
> > -			dt_ident_set_storage(idp, 8,
> > -					     ctf_type_size(dtt.dtt_ctfp, type));
> > -
> >   			dt_ident_type_assign(idp, dtt.dtt_ctfp, dtt.dtt_type);
> > +			dt_ident_set_storage(idp, 8,
> > +					     ctf_type_size(dtt.dtt_ctfp,
> > +							   dtt.dtt_type));
> >   
> >   			/*
> >   			 * If we are declaring an associative array, use our
> > @@ -2754,10 +2754,6 @@ dt_xcook_ident(dt_node_t *dnp, dt_idhash_t *dhp, uint_t idkind, int create)
> >   		if (idp == NULL)
> >   			longjmp(yypcb->pcb_jmpbuf, EDT_NOMEM);
> >   
> > -		/* aggregation storage size is set later, in dt_cg.c */
> > -		if (idkind != DT_IDENT_AGG)
> > -			dt_ident_set_storage(idp, 8, 8);
> > -
> >   		/*
> >   		 * Arrays and aggregations are not cooked individually. They
> >   		 * have dynamic types and must be referenced using operator [].
> > @@ -2871,6 +2867,8 @@ dt_cook_op1(dt_node_t *dnp, uint_t idflags)
> >   			xyerror(D_TYPE_ERR, "failed to lookup int64_t\n");
> >   
> >   		dt_ident_type_assign(cp->dn_ident, dtt.dtt_ctfp, dtt.dtt_type);
> > +		dt_ident_set_storage(cp->dn_ident, 8,
> > +				     ctf_type_size(dtt.dtt_ctfp, dtt.dtt_type));
> >   		dt_node_type_assign(cp, dtt.dtt_ctfp, dtt.dtt_type);
> >   	}
> >   
> > @@ -3485,6 +3483,8 @@ dt_cook_op2(dt_node_t *dnp, uint_t idflags)
> >   		    dt_ident_unref(lp->dn_ident)) {
> >   			dt_node_type_assign(lp, ctfp, type);
> >   			dt_ident_type_assign(lp->dn_ident, ctfp, type);
> > +			dt_ident_set_storage(lp->dn_ident, 8,
> > +					     ctf_type_size(ctfp, type));
> >   
> >   			if (uref) {
> >   				lp->dn_flags |= DT_NF_USERLAND;
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel