Re: [HACKERS] Small fix: avoid passing null pointers to memcpy()

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: didier <did447(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Small fix: avoid passing null pointers to memcpy()
Date: 2019-05-24 16:35:25
Message-ID: 18991.1558715725@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> didier <did447(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Konstantin Knizhnik 2019-05-24 16:37:15 Re: Read-only access to temp tables for 2PC transactions
Previous Message Andres Freund 2019-05-24 16:24:28 Re: [HACKERS] Runtime Partition Pruning