Re: [PATCH 1/2] libdecnumber: fixed undefined behavior in `decFloatFMA`

2024-02-02 Thread Ian McCormack 
I've confirmed that these changes fix the error in MIRI, too. I'll post
an updated patch once I confirm that there aren't any regressions.

On Fri, Feb 2, 2024 at 10:38 AM Jakub Jelinek  wrote:

> On Fri, Feb 02, 2024 at 04:32:09PM +0100, Jakub Jelinek wrote:
> > Anyway, I think all of
> > decBasic.c: for (; UBTOUI(umsd)==0 && umsd+3 > decBasic.c: for (; *umsd==0 && umsd > decBasic.c:  for (; UBTOUI(hi->msd)==0 && hi->msd+3lsd;) hi->msd+=4;
> > decBasic.c:  for (; *hi->msd==0 && hi->msdlsd;) hi->msd++;
> > decBasic.c:  for (; UBTOUI(lo->msd)==0 && lo->msd+3lsd;) lo->msd+=4;
> > decBasic.c:  for (; *lo->msd==0 && lo->msdlsd;) lo->msd++;
> > decBasic.c:  for (; UBTOUI(lo->msd)==0 && lo->msd+3lsd;)
> lo->msd+=4;
> > decBasic.c:  for (; *lo->msd==0 && lo->msdlsd;) lo->msd++;
> > decCommon.c:  for (; *umsd==0 && umsd
> even more than that:
> decBasic.c:  for (; *msua==0 && msua>=lsua;) msua--;
> decBasic.c:if (*msua==0 && msua==lsua) break;
> decCommon.c:  if (*ulsd==0 && ulsd==umsd) { /* have zero */
> decNumber.c:for (; *msu1==0 && msu1>var1; msu1--) var1units--;
> too just from simple grep.
>
> Jakub
>
>

Re: [PATCH 1/2] libdecnumber: fixed undefined behavior in `decFloatFMA`

2024-02-02 Thread Jakub Jelinek 
On Fri, Feb 02, 2024 at 04:32:09PM +0100, Jakub Jelinek wrote:
> Anyway, I think all of
> decBasic.c: for (; UBTOUI(umsd)==0 && umsd+3 decBasic.c: for (; *umsd==0 && umsd decBasic.c:  for (; UBTOUI(hi->msd)==0 && hi->msd+3lsd;) hi->msd+=4;
> decBasic.c:  for (; *hi->msd==0 && hi->msdlsd;) hi->msd++;
> decBasic.c:  for (; UBTOUI(lo->msd)==0 && lo->msd+3lsd;) lo->msd+=4;
> decBasic.c:  for (; *lo->msd==0 && lo->msdlsd;) lo->msd++;
> decBasic.c:  for (; UBTOUI(lo->msd)==0 && lo->msd+3lsd;) lo->msd+=4;
> decBasic.c:  for (; *lo->msd==0 && lo->msdlsd;) lo->msd++;
> decCommon.c:  for (; *umsd==0 && umsd=lsua;) msua--;
decBasic.c:if (*msua==0 && msua==lsua) break;
decCommon.c:  if (*ulsd==0 && ulsd==umsd) { /* have zero */
decNumber.c:for (; *msu1==0 && msu1>var1; msu1--) var1units--;
too just from simple grep.

Jakub

Re: [PATCH 1/2] libdecnumber: fixed undefined behavior in `decFloatFMA`

2024-02-02 Thread Jakub Jelinek 
On Fri, Feb 02, 2024 at 10:09:05AM -0500, Ian McCormack wrote:
> This patch fixes a minor instance of undefined behavior in libdecnumber. It 
> was discovered in the Rust bindings for libdecnumber (`dec`) using a custom 
> version of MIRI that can execute foreign functions.
> 
> Within the function `decFloatFMA`, the pointer `lo->msd` is initialized to 
> point to a byte array of size 56. 
> 
> ```
> uByte  acc[FMALEN];  /* for multiplied coefficient in BCD */
> ...
> ub=acc+FMALEN-1; /* where lsd of result will go */
> ...
> lo->msd=ub+1;
> lo->lsd=acc+FMALEN-1;
> ```
> However, `lo->msd` is then offset in increments of 4, which leads to a read 
> of two bytes beyond the size of `acc`. This patch switches to reading in 
> increments of 2 instead of 4.
> 
> Bootstrapped on x86_64-pc-linux-gnu with no regressions.
> 
> libdecnumber/ChangeLog
>* decBasic.c: Increment `lo->msd` by 2 instead of 4 in `decFloatFMA` 
> to avoid undefined behavior.

Isn't 56 divisible by 4?
Anyway, I think all of
decBasic.c:   for (; UBTOUI(umsd)==0 && umsd+3msd)==0 && hi->msd+3lsd;) hi->msd+=4;
decBasic.c:  for (; *hi->msd==0 && hi->msdlsd;) hi->msd++;
decBasic.c:  for (; UBTOUI(lo->msd)==0 && lo->msd+3lsd;) lo->msd+=4;
decBasic.c:  for (; *lo->msd==0 && lo->msdlsd;) lo->msd++;
decBasic.c:  for (; UBTOUI(lo->msd)==0 && lo->msd+3lsd;) lo->msd+=4;
decBasic.c:  for (; *lo->msd==0 && lo->msdlsd;) lo->msd++;
decCommon.c:  for (; *umsd==0 && umsd

[PATCH 1/2] libdecnumber: fixed undefined behavior in `decFloatFMA`

2024-02-02 Thread Ian McCormack 
This patch fixes a minor instance of undefined behavior in libdecnumber. It was 
discovered in the Rust bindings for libdecnumber (`dec`) using a custom version 
of MIRI that can execute foreign functions.

Within the function `decFloatFMA`, the pointer `lo->msd` is initialized to 
point to a byte array of size 56. 

```
uByte  acc[FMALEN];/* for multiplied coefficient in BCD */
...
ub=acc+FMALEN-1;   /* where lsd of result will go */
...
lo->msd=ub+1;
lo->lsd=acc+FMALEN-1;
```
However, `lo->msd` is then offset in increments of 4, which leads to a read of 
two bytes beyond the size of `acc`. This patch switches to reading in 
increments of 2 instead of 4.

Bootstrapped on x86_64-pc-linux-gnu with no regressions.

libdecnumber/ChangeLog
   * decBasic.c: Increment `lo->msd` by 2 instead of 4 in `decFloatFMA` to 
avoid undefined behavior.

---
 libdecnumber/decBasic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libdecnumber/decBasic.c b/libdecnumber/decBasic.c
index 6319f66b25d..3c8d71a2949 100644
--- a/libdecnumber/decBasic.c
+++ b/libdecnumber/decBasic.c
@@ -2023,6 +2023,7 @@ decFloat * decFloatFMA(decFloat *result, const decFloat 
*dfl,
   uInt  carry;/* +1 for ten's complement and during add */
   uByte  *ub, *uh, *ul;   /* work */
   uInt  uiwork;   /* for macros */
+  uShort uswork;
 
   /* handle all the special values [any special operand leads to a */
   /* special result] */
@@ -2252,7 +2253,7 @@ decFloat * decFloatFMA(decFloat *result, const decFloat 
*dfl,
   /* all done except for the special IEEE 754 exact-zero-result */
   /* rule (see above); while testing for zero, strip leading */
   /* zeros (which will save decFinalize doing it) */
-  for (; UBTOUI(lo->msd)==0 && lo->msd+3lsd;) lo->msd+=4;
+  for (; UBTOUS(lo->msd)==0 && lo->msd+1lsd;) lo->msd+=2;
   for (; *lo->msd==0 && lo->msdlsd;) lo->msd++;
   if (*lo->msd==0) {  /* must be true zero (and diffsign) */
lo->sign=0;/* assume + */
-- 
2.39.3 (Apple Git-145)