Horton

Thread

Commits

  1. Fix integer underflow in shared memory debugging

  2. Introduce dynamic shared memory areas.

  1. Integer undeflow in fprintf in dsa.c

    Ilyasov Ian <ianilyasov@outlook.com> — 2024-02-20T11:28:03Z

    Hello hackers,

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.
(https://postgrespro.com/list/thread-id/2419512,
https://www.postgresql.org/message-id/15e9501170d.e4b5a3858707.3339083113985275726%40zohocorp.com)

​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.

Juniour Software Developer at Postgres Professional

  2. Re: Integer undeflow in fprintf in dsa.c

    Daniel Gustafsson <daniel@yesql.se> — 2024-02-20T12:00:19Z

    > 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

  3. Re: Integer undeflow in fprintf in dsa.c

    Robert Haas <robertmhaas@gmail.com> — 2024-02-20T12:22:48Z

    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

  4. RE: Integer undeflow in fprintf in dsa.c

    Ilyasov Ian <ianilyasov@outlook.com> — 2024-02-20T16:13:41Z

    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.

Juniour Software Developer at Postgres Professional

  5. Re: Integer undeflow in fprintf in dsa.c

    Daniel Gustafsson <daniel@yesql.se> — 2024-02-20T17:00:07Z

    > 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