Thread: BUG #16157: handling of pg_malloc(0)

The following bug has been logged on the website:

Bug reference:      16157
Logged by:          cili
Email address:      cilizili@protonmail.com
PostgreSQL version: 12.1
Operating system:   CentOS 7.4
Description:

While checking code, I found a potential bug. To avoid unportable behavior
of malloc(0), pg_malloc family functions in fe_memutils.c replace the size 0
with 1. I think 1 is poor, any size of chunk enought for structure or
pointer may be required.  
The pg_malloc() is used instead of malloc(), and the output is casted into
other types. For example, in initdb.c:L508, if nlines equals to 2^32-1,
    result = (char **) pg_malloc((nlines + 1) * sizeof(char *));
will be 
    result = (char **) pg_malloc(0);
and, according to replacement with 1, it is equivalent to
    result = (char **) malloc(1);
however, I think that the expected code may be, a size of pointer, 
    result = (char **) malloc(1 * sizeof(char *));

Of course,  2^32-1 lines of postgresq.conf.sample or others are too huge and
unlikely. Fortunately, when a huge file has been read, initdb will report
'out of memory' and safely stop.
In the previous case, pg_malloc0() is better than pg_malloc().

BTW, I can't find any real issue of pg_malloc(0). It a good news. However,
since many codes cast the outputs of pg_malloc, palloc, and other allocation
functions, I consider that the replacement of the size for 0 in pg_malloc()
should be more large value enought to store any type of structure. For
example, if pg_malloc(0) is treated as malloc(1024), it my avoid previous
small allocation problems.

PG Bug reporting form <noreply@postgresql.org> writes:
> While checking code, I found a potential bug. To avoid unportable behavior
> of malloc(0), pg_malloc family functions in fe_memutils.c replace the size 0
> with 1. I think 1 is poor, any size of chunk enought for structure or
> pointer may be required.  

I don't see your point.  The reason we're doing anything special here
is that the C/POSIX standard for malloc() says

    If the size of the space requested is 0, the behavior is
    implementation-defined: either a null pointer shall be returned, or
    the behavior shall be as if the size were some non-zero value, except
    that the behavior is undefined if the returned pointer is used to
    access an object.

We don't want the first behavior, so we're forcing it to be the second
one.  But the behavior is still undefined if you try to actually store
anything in the returned space, so there's no point in making it any
bigger than we have to.  Indeed, doing so would make it easier for
buggy code that tries to store something there to escape detection.

> For example, in initdb.c:L508, if nlines equals to 2^32-1,
>     result = (char **) pg_malloc((nlines + 1) * sizeof(char *));

Well, arguably it's a bug that this code isn't being careful about
integer overflow in the size request, but no amount of hacking of
pg_malloc() will fix that; the bug is in the caller.  (In practice,
since initdb is only run with known and very finite inputs, there is
zero point in complicating this particular logic with overflow worries.
Elsewhere, we'd more likely want to fix it.)

            regards, tom lane

> PG Bug reporting form noreply@postgresql.org writes:
>
> > While checking code, I found a potential bug. To avoid unportable behavior
> > of malloc(0), pg_malloc family functions in fe_memutils.c replace the size 0
> > with 1. I think 1 is poor, any size of chunk enought for structure or
> > pointer may be required.
>
> I don't see your point. The reason we're doing anything special here
> is that the C/POSIX standard for malloc() says
>
> If the size of the space requested is 0, the behavior is
> implementation-defined: either a null pointer shall be returned, or
> the behavior shall be as if the size were some non-zero value, except
> that the behavior is undefined if the returned pointer is used to
> access an object.
>
> We don't want the first behavior, so we're forcing it to be the second
> one. But the behavior is still undefined if you try to actually store
> anything in the returned space, so there's no point in making it any
> bigger than we have to. Indeed, doing so would make it easier for
> buggy code that tries to store something there to escape detection.

Thank you for your explanation. I've read again the comments near the codes written malloc() or realloc(), I understand
well.My reported issue is not a bug. 

regards,