[PATCH 5/8] Fix stddev() carryover computation

eugene.loh
2024-06-04T18:00:26Z

From: Eugene Loh <eugene.loh@oracle.com>

The stddev() aggregation function squares 64-bit data values. A
value is split into 32-bit high and low values. Then, (high + low)
is squared to produce high*high, 2*high*low, and low*low. Each is
managed in its own 64-bit register, with the final result residing
in two 64-bit registers.

When the 2*high*low portion is combined with the low*low portion,
care is exercised in case the combination has a carryover portion to
the higher bits. This check was broken in the case where low==0.
That is, data values whose lowest 32 bits were 0 resulted in
outrageously bad stddev() results.

Fix the check and add a test for such cases.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
libdtrace/dt_cg.c | 2 +-
test/unittest/aggs/tst.stddev2.d | 45 ++++++++++++++++++++++++++++++++
test/unittest/aggs/tst.stddev2.r | 13 +++++++++
3 files changed, 59 insertions(+), 1 deletion(-)
create mode 100644 test/unittest/aggs/tst.stddev2.d
create mode 100644 test/unittest/aggs/tst.stddev2.r

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index a1c24e37..fc2cebf0 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -8290,7 +8290,7 @@ dt_cg_agg_stddev(dt_pcb_t *pcb, dt_ident_t *aid, dt_node_t *dnp,
/* Add low value part from mid to lowreg */
emit(dlp, BPF_ALU64_REG(BPF_ADD, lowreg, lmdreg));
/* Handle the overflow/carry case */
- emit(dlp, BPF_BRANCH_REG(BPF_JLT, lmdreg, lowreg, Lncy));
+ emit(dlp, BPF_BRANCH_REG(BPF_JLE, lmdreg, lowreg, Lncy));
emit(dlp, BPF_ALU64_IMM(BPF_ADD, hi_reg, 1)) /* account for carry */;

/* Sum high value; no overflow expected nor accounted for */
diff --git a/test/unittest/aggs/tst.stddev2.d b/test/unittest/aggs/tst.stddev2.d
new file mode 100644
index 00000000..994bc3e2
--- /dev/null
+++ b/test/unittest/aggs/tst.stddev2.d
@@ -0,0 +1,45 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * Licensed under the Universal Permissive License v 1.0 as shown at
+ * http://oss.oracle.com/licenses/upl.
+ */
+
+/*
+ * ASSERTION: Positive stddev() test
+ *
+ * SECTION: Aggregations/Aggregations
+ *
+ * NOTES: This is a simple verifiable positive test of the stddev() function.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+ @a = stddev( 0); @a = stddev( 0);
+ @b = stddev( 0x10); @b = stddev( 0x20);
+ @c = stddev( 0x100); @c = stddev( 0x200);
+ @d = stddev( 0x1000); @d = stddev( 0x2000);
+ @e = stddev( 0x10000); @e = stddev( 0x20000);
+ @f = stddev( 0x100000); @f = stddev( 0x200000);
+ @g = stddev( 0x1000000); @g = stddev( 0x2000000);
+ @h = stddev( 0x10000000); @h = stddev( 0x20000000);
+ @i = stddev( 0x20000000); @i = stddev( 0x40000000);
+ @j = stddev( 0x40000000); @j = stddev( 0x80000000);
+ @k = stddev( 0x80000000); @k = stddev(0x100000000);
+ @l = stddev(0x100000000); @l = stddev(0x200000000);
+ printa("%9@x\n", @a);
+ printa("%9@x\n", @b);
+ printa("%9@x\n", @c);
+ printa("%9@x\n", @d);
+ printa("%9@x\n", @e);
+ printa("%9@x\n", @f);
+ printa("%9@x\n", @g);
+ printa("%9@x\n", @h);
+ printa("%9@x\n", @i);
+ printa("%9@x\n", @j);
+ printa("%9@x\n", @k);
+ printa("%9@x\n", @l);
+ exit(0);
+}
diff --git a/test/unittest/aggs/tst.stddev2.r b/test/unittest/aggs/tst.stddev2.r
new file mode 100644
index 00000000..16e17736
--- /dev/null
+++ b/test/unittest/aggs/tst.stddev2.r
@@ -0,0 +1,13 @@
+
+ 0
+ 8
+ 80
+ 800
+ 8000
+ 80000
+ 800000
+ 8000000
+ 10000000
+ 20000000
+ 40000000
+ 80000000
--
2.18.4