lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lmi] How to deal with this instance of UB?


From: Greg Chicares
Subject: [lmi] How to deal with this instance of UB?
Date: Thu, 16 Jun 2022 20:30:09 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0

Vadim--The patch below shows two ways of dealing with UB at a
particular point in fdlibm; of course want to choose one way.

The first way is to use an __attribute__ that blinds UBSan to
the problem. I dislike doing that, except as a last resort
with a compelling reason. OTOH, this is UB only because the C
standard committee says so; if (as is unlikely) I understand
what C++20 says, it's no longer UB in C++ because integers are
required to be two's complement; and therefore any machine on
which C++20 is implementable IIUC, or on which fdlibm (and
thus glibc) is correct, already executes this operation in
the expected way. Yet this is C, and it remains UB, and a
rude or careless compiler author could devise an optimization
that stomps on this rationale.

The second way is to use casts to replace UB with defined
behavior. That seems like the righteous path, and I think
I've followed it correctly below, but I'd rather not commit
this without your prior review.

(A third way is to keep 'math_functions_test' suppressed
for UBSan unit-test runs. But enabling it showed me an
unintended division by zero, so that way is just wrong.)

[A fourth way would be to compile this as C++, where the
behavior is well defined AIUI; but that seems wrong, too.]

So my questions are two:
 - Do you agree that the second way is better in principle?
 - Have I implemented the second way correctly?

--8<----8<----8<----8<----8<----8<----8<----8<----8<--
diff --git a/fdlibm_expm1.c b/fdlibm_expm1.c
index 5e05e10f7..f8be05602 100644
--- a/fdlibm_expm1.c
+++ b/fdlibm_expm1.c
@@ -168,6 +168,9 @@ Q[] = { 1.0
 #define Q4  Q[4]
 #define Q5  Q[5]
 
+#if defined LMI_GCC || defined LMI_CLANG
+__attribute__((no_sanitize("shift-base")))
+#endif // defined LMI_GCC || defined LMI_CLANG
 double fdlibm_expm1(double x)
 {
     double y=0,hi=0,lo=0,c=0,t=0,e=0,hxs=0,hfx=0,r1=0,h2=0,h4=0,R1=0,R2=0,R3=0;
@@ -239,7 +242,8 @@ double fdlibm_expm1(double x)
         }
         if (k <= -2 || k>56) {     // suffice to return exp(x)-1
             y = one-(e-x);
-            SET_HIGH_WORD(y, hi_uint(y) + (k<<20));    // add k to y's exponent
+            // add k to y's exponent
+            SET_HIGH_WORD(y, hi_uint(y) + (int32_t)(((uint32_t)k)<<20));
             return y-one;
         }
         t = one;
--8<----8<----8<----8<----8<----8<----8<----8<----8<--


reply via email to

[Prev in Thread] Current Thread [Next in Thread]