Обсуждение: Integer undeflow in fprintf in dsa.c
Hello hackers,
Using Svace* I think I've found a little bug in src/backend/utils/mmgr/dsa.c.
Using Svace* I think I've found a little bug in src/backend/utils/mmgr/dsa.c.
This bug is presented in REL_12_STABLE, REL_13_STABLE, REL_14_STABLE,
REL_15_STABLE, REL_16_STABLE and master. I see that it was introduced together
with dynamic shared memory areas in the commit 13df76a537cca3b8884911d8fdf7c89a457a8dd3.
I also see that at least two people have encountered this fprintf output.
fprintf(stderr,
" segment bin %zu (at least %d contiguous pages free):\n",
i, 1 << (i - 1));
In case
i
equals zero user will get "at least -2147483648 contiguous pages free".I believe that this is a mistake, and
fprintf
should print "at least 0 contiguous pages free"in case
i
equals zero.The patch that has a fix of this is attached.
* - https://svace.pages.ispras.ru/svace-website/en/
Kind regards,
Ian Ilyasov.
Kind regards,
Ian Ilyasov.
Juniour Software Developer at Postgres Professional
Вложения
> On 20 Feb 2024, at 12:28, Ильясов Ян <ianilyasov@outlook.com> wrote: > fprintf(stderr, > " segment bin %zu (at least %d contiguous pages free):\n", > i, 1 << (i - 1)); > > In case i equals zero user will get "at least -2147483648 contiguous pages free". That does indeed seem like an oversight. > I believe that this is a mistake, and fprintf should print "at least 0 contiguous pages free" > in case i equals zero. The message "at least 0 contiguous pages free" reads a bit nonsensical though, wouldn't it be preferrable to check for i being zero and print a custom message for that case? Something like the below untested sketch? + if (i == 0) + fprintf(stderr, + " segment bin %zu (no contiguous free pages):\n", i); + else + fprintf(stderr, + " segment bin %zu (at least %d contiguous pages free):\n", + i, 1 << (i - 1)); -- Daniel Gustafsson
On Tue, Feb 20, 2024 at 5:30 PM Daniel Gustafsson <daniel@yesql.se> wrote: > The message "at least 0 contiguous pages free" reads a bit nonsensical though, > wouldn't it be preferrable to check for i being zero and print a custom message > for that case? Something like the below untested sketch? > > + if (i == 0) > + fprintf(stderr, > + " segment bin %zu (no contiguous free pages):\n", i); > + else > + fprintf(stderr, > + " segment bin %zu (at least %d contiguous pages free):\n", > + i, 1 << (i - 1)); That does seem reasonable. However, this is just debugging code, so it also probably isn't necessary to sweat anything too much. -- Robert Haas EDB: http://www.enterprisedb.com
Sorry for not answering quickly.
Thank you for your comments.
I attached a patch to the letter with changes to take into account Daniel Gustafsson's comment.
Kind regards,
Ian Ilyasov.
Ian Ilyasov.
Juniour Software Developer at Postgres Professional
Вложения
> On 20 Feb 2024, at 17:13, Ilyasov Ian <ianilyasov@outlook.com> wrote: > Sorry for not answering quickly. There is no need for any apology, there is no obligation to answer within any specific timeframe. > I attached a patch to the letter with changes to take into account Daniel Gustafsson's comment. Looks good on a quick skim, I'll take care of this shortly. -- Daniel Gustafsson