Re: [HACKERS] Small fix: avoid passing null pointers to memcpy() - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [HACKERS] Small fix: avoid passing null pointers to memcpy()
Date
Msg-id 18991.1558715725@sss.pgh.pa.us
Whole thread Raw
In response to Re: [HACKERS] Small fix: avoid passing null pointers to memcpy()  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] Small fix: avoid passing null pointers to memcpy()  (didier <did447@gmail.com>)
List pgsql-hackers
I wrote:
> didier <did447@gmail.com> writes:
>> Without this patch make check fails 3 tests if pg is compiled with
>> -fsanitize=address,undefined

> ... but if that's the only evidence of an actual problem, I can't
> get excited about it.  ASAN complains about many things in Postgres,
> and most of them are pretty hypothetical.

BTW, I did try the described test case, so I suppose I might as well
summarize the results while I have them.  The postmaster log from
a regression run with today's HEAD shows ASAN errors at

clog.c:299:3:
indexcmds.c:1054:4:
relcache.c:5905:6:
snapmgr.c:597:2:
snapmgr.c:601:2:
xact.c:5204:3:

The above all seem to be the same ilk as the problem in print.c,
ie passing a NULL pointer with zero count to memcpy, memset, or
the like.  At least some of them are fairly old.

pg_crc32c_sse42.c:37:18:
pg_crc32c_sse42.c:44:9:

This is an intentional use of unaligned access:

     * NB: We do unaligned accesses here. The Intel architecture allows that,
     * and performance testing didn't show any performance gain from aligning
     * the begin address.

This comment is unclear about whether it would actually hurt to have a
preparatory loop to align the begin address.

... and a whole bunch of places in arrayaccess.h and arrayfuncs.c.

These seem to be down to use of AnyArrayType:

typedef union AnyArrayType
{
    ArrayType    flt;
    ExpandedArrayHeader xpn;
} AnyArrayType;

ASAN seems to believe that use of this union entitles the compiler to
assume 8-byte alignment even when touching fields of a short-header
array.  Maybe it's right, but this code has been like this for awhile
and we've not heard trouble reports.  I'm afraid that avoiding the
use of the union would be a notational mess, so I'm loath to change
it unless we're forced to.

This is just from the core tests, there could well be more issues in
contrib or PLs.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: [HACKERS] Runtime Partition Pruning
Next
From: Konstantin Knizhnik
Date:
Subject: Re: Read-only access to temp tables for 2PC transactions