Re: NULL passed as an argument to memcmp() in parse_func.c

From: Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)me>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: joerg(at)NetBSD(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: NULL passed as an argument to memcmp() in parse_func.c
Date: 2015-07-01 05:12:38
Message-ID: BLU436-SMTP463216A1B9351D1CFF629BF2A80@phx.gbl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 06/22/2015 08:55 PM, Tom Lane wrote:
> Are you seeing any observable problem here, and if so what is it?

On 06/27/2015 11:47 PM, Tom Lane wrote:
> Given the utter lack of any evidence that this actually causes any
> problems in the field, I don't feel a need to back-patch this change.

I'm under the impression that you don't care about not avoiding
undefined behavior as much as you care about "solving real problems"
caused by it, whenever they show up in a report from one platform or
another, or worse - when it's too late and somebody has reported an
actual program misbehavior. The problem with that kind of thinking is
that bugs caused by aggressive compiler optimizations taking advantage
of invoking UB are a moving target (since compilers come and go, and the
existing ones evolve) while the list of things not to do is constant and
mostly clearly defined by the standard.

Take this for an example: when 4.7 was the most recent version of GCC
you could safely pass a null pointer to memcmp() and expect the program
below to print nothing. If this were part of Postgres, you could have
said "even though this is UB, nothing seems to be broken - so I don't
feel a need to fix this". Today, that wouldn't be the case; since GCC
5.0 (or perhaps 4.9) on higher optimization levels will assume that
since you must not pass a null pointer then you'll surely never do it,
so the expression in the if statement doesn't need to be evaluated and
can be assumed to always yield true and ultimately the test will be
optimized out. It would be a ticking bomb.

#include <string.h>
#include <stdlib.h>
#include <stdio.h>
void f(int *p, int *q, size_t l) {
if (memcmp(p, q, l))
puts("memcmp");
if (q != NULL)
puts("q != NULL");
}
int main(void) {
f(NULL, NULL, 0);
return 0;
}

(I'm CC'ing Joerg, the author of the small test case above).

So while my answer to your question currently is "no, I'm not seeing any
observable problems here", I don't think it's a relevant question in any
case of this class of problems. What I do think is that it's quite
important to make effort and target undefined behavior before it has a
chance of becoming a real problem.

The reason I'm getting a bit more vocal about this than I have been
until now is that I plan to report more of this kind of stuff and I want
to be clear now, in advance, about why I'm not going to be too concerned
about lack of any observable problems in my future reports.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2015-07-01 05:25:46 Re: Bug in bttext_abbrev_convert()
Previous Message Michael Paquier 2015-07-01 04:39:57 Re: Bug in bttext_abbrev_convert()