[DTrace-devel] [PATCH] Fix value of post-decrement expressions

[Kris Van Hees]

On Sat, Aug 22, 2020 at 07:55:05AM -0700, Eugene Loh wrote:
> On 08/21/2020 11:31 PM, Kris Van Hees wrote:
> 
> > Please change the subject of your patch to not be identical to the commit
> > you are fixing.  It gets really confusing to get multiple patches with the
> > same name, especially when one is fixing an issue with an earlier one.
> >
> > A good alternative wuold be:
> >
> > Reverse order of operands in postarith expressions
> >
> > or something like that (since that is in fact the fix).
> 
> Okay.  FWIW, the messages are similar but not identical.  (The original 
> patch did happen to work for increment.  This one makes it work for 
> decrement.  So the messages differ, but their similarity was intended to 
> reflect their relationship.)  But no worries;  I'll just change it to 
> your suggestion to make them look more different.
> 
> BTW, does this mean "Reviewed-by"?  And that I can push to 2.0-branch-dev?

Woops, yes, sorry for not adding that:

Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
> 
> > Other than that, nice catch!
> >
> > On Sat, Aug 22, 2020 at 01:46:22AM -0400, eugene.loh at oracle.com wrote:
> >> From: Eugene Loh <eugene.loh at oracle.com>
> >>
> >> An earlier commit ("17b0da163 Fix value of post-increment expressions")
> >> purported to fix postfix expressions, but it formed (1 op arg) rather than
> >> (arg op 1), as is evident not only in the code but even in the commit
> >> message.  Arguably, the distinction is unimportant for increment, since
> >> addition is commutative, but the patch produced incorrect results for
> >> decrement expressions.
> >>
> >> Fix the generated code and remove the XFAIL on the associated test.
> >>
> >> Orabug: 31787365
> >> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> >> ---
> >>   libdtrace/dt_cg.c                      | 4 ++--
> >>   test/unittest/arithmetic/tst.complex.d | 1 -
> >>   2 files changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> >> index f183d774..95a6c813 100644
> >> --- a/libdtrace/dt_cg.c
> >> +++ b/libdtrace/dt_cg.c
> >> @@ -1911,9 +1911,9 @@ dt_cg_postarith_op(dt_node_t *dnp, dt_irlist_t *dlp,
> >>   	if (nreg == -1)
> >>   		longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> >>   
> >> -	instr = BPF_MOV_IMM(nreg, size);
> >> +	instr = BPF_MOV_REG(nreg, oreg);
> >>   	dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> >> -	instr = BPF_ALU64_REG(op, nreg, dnp->dn_reg);
> >> +	instr = BPF_ALU64_IMM(op, nreg, size);
> >>   	dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> >>   
> >>   	/*
> >> diff --git a/test/unittest/arithmetic/tst.complex.d b/test/unittest/arithmetic/tst.complex.d
> >> index 6a8a19a7..f9d7480b 100644
> >> --- a/test/unittest/arithmetic/tst.complex.d
> >> +++ b/test/unittest/arithmetic/tst.complex.d
> >> @@ -14,7 +14,6 @@
> >>    * SECTION: Types, Operators, and Expressions/Arithmetic Operators
> >>    *
> >>    */
> >> -/* @@xfail: dtv2 */
> >>   
> >>   #pragma D option quiet
> >>   
> >> -- 
> >> 2.18.4
> >>
> >>
> >> _______________________________________________
> >> DTrace-devel mailing list
> >> DTrace-devel at oss.oracle.com
> >> https://oss.oracle.com/mailman/listinfo/dtrace-devel
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel