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

[Eugene Loh]

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?

> 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