Re: Optimize numeric multiplication for one and two base-NBASE digit multiplicands.

From: "Joel Jacobson" <joel(at)compiler(dot)org>
To: "Dean Rasheed" <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Optimize numeric multiplication for one and two base-NBASE digit multiplicands.
Date: 2024-07-03 19:05:51
Message-ID: b2079087-86be-43c3-a298-6bc0b49e2d24@app.fastmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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(at)compiler(dot)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 enforce this with an Assert(), to prevent the otherwise possible out-of-bounds access (negative indexing) and provide early detection?

Regards,
Joel

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2024-07-03 19:13:37 Re: Add a GUC check hook to ensure summarize_wal cannot be enabled when wal_level is minimal
Previous Message Dean Rasheed 2024-07-03 18:57:48 Re: Optimize numeric multiplication for one and two base-NBASE digit multiplicands.