Re: Optimize numeric multiplication for one and two base-NBASE digit multiplicands. - Mailing list pgsql-hackers

From Joel Jacobson
Subject Re: Optimize numeric multiplication for one and two base-NBASE digit multiplicands.
Date
Msg-id b2079087-86be-43c3-a298-6bc0b49e2d24@app.fastmail.com
Whole thread Raw
In response to Re: Optimize numeric multiplication for one and two base-NBASE digit multiplicands.  ("Joel Jacobson" <joel@compiler.org>)
List pgsql-hackers
On Wed, Jul 3, 2024, at 15:48, Joel Jacobson wrote:
> On Wed, Jul 3, 2024, at 13:17, Dean Rasheed wrote:
>> On Tue, 2 Jul 2024 at 21:10, Joel Jacobson <joel@compiler.org> wrote:
>>>
>>> I found the bug in the case 3 code,
>>> and it turns out the same type of bug also exists in the case 2 code:
>>>
>>>                         case 2:
>>>                                 newdig = (int) var1digits[1] * var2digits[res_ndigits - 4];
>>>
>>> The problem here is that res_ndigits could become less than 4,
>>
>> Yes. It can't be less than 3 though (per an earlier test), so the case
>> 2 code was correct.
>
> Hmm, I don't see how the case 2 code can be correct?
> If, like you say, res_ndigits can't be less than 3, that means it can 
> be 3, right?
> And if res_ndigits=3 then `var2digits[res_ndigits - 4]` would try to 
> access `var2digits[-1]`.

Here is an example on how to trigger the bug:

```
            case 2:
                if (res_ndigits - 4 < 0)
                {
                    printf("var1=%s\n",get_str_from_var(var1));
                    printf("var2=%s\n",get_str_from_var(var2));
                    printf("rscale=%d\n", rscale);
                    printf("res_ndigits - 4 < 0 => var2digits[%d]=%d\n", res_ndigits - 4, var2digits[res_ndigits -
4]);
                }
```

Running through my tests, I hit lots of cases, including:

var1=0.10968501
var2=0.903728177113
rscale=0
res_ndigits - 4 < 0 => var2digits[-1]=-31105

All of the spotted cases had rscale=0.

If we know that mul_var() will never be called with rscale=0 when dealing with decimal inputs, perhaps we should
enforcethis with an Assert(), to prevent the otherwise possible out-of-bounds access (negative indexing) and provide
earlydetection?
 

Regards,
Joel



pgsql-hackers by date:

Previous
From: Dean Rasheed
Date:
Subject: Re: Optimize numeric multiplication for one and two base-NBASE digit multiplicands.
Next
From: Robert Haas
Date:
Subject: Re: Add a GUC check hook to ensure summarize_wal cannot be enabled when wal_level is minimal