Re: Add support for __attribute__((returns_nonnull))

From: "Tristan Partin" <tristan(at)neon(dot)tech>
To: "Peter Eisentraut" <peter(at)eisentraut(dot)org>
Cc: "pgsql-hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add support for __attribute__((returns_nonnull))
Date: 2023-12-27 18:20:22
Message-ID: CXZBOGW4GQ7P.3Q5OTP90OYWTN@neon.tech
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed Dec 27, 2023 at 6:42 AM CST, Peter Eisentraut wrote:
> On 19.12.23 21:43, Tristan Partin wrote:
> > Here is a patch which adds support for the returns_nonnull attribute
> > alongside all the other attributes we optionally support.
> >
> > I recently wound up in a situation where I was checking for NULL return
> > values of a function that couldn't ever return NULL because the
> > inability to allocate memory was always elog(ERROR)ed (aborted).
> >
> > I didn't go through and mark anything, but I feel like it could be
> > useful for people going forward, including myself.
>
> I think it would be useful if this patch series contained a patch that
> added some initial uses of this. That way we can check that the
> proposed definition actually works, and we can observe what it does,
> with respect to warnings, static analysis, etc.

Good point. Patch attached.

I tried to find some ways to prove some value, but I couldn't. Take this
example for instance:

static const char word[] = { 'h', 'e', 'l', 'l', 'o' };

const char * __attribute__((returns_nonnull))
hello()
{
return word;
}

int
main(void)
{
const char *r;

r = hello();
if (r == NULL)
return 1;

return 0;
}

I would have thought I could get gcc or clang to warn on a wasteful NULL
check, but alas. I also didn't see any code generation improvements, but
I am assuming that the example is too contrived. I couldn't find any
good things online that had examples of when such an annotation forced
the compiler to warn or create more optimized code.

If you return NULL from the hello() function, clang will warn that the
attribute doesn't match reality.

--
Tristan Partin
Neon (https://neon.tech)

Attachment Content-Type Size
v2-0001-Add-support-for-__attribute__-returns_nonnull.patch text/x-patch 1.6 KB
v2-0002-Mark-some-initial-candidates-as-returns_nonnull.patch text/x-patch 6.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tristan Partin 2023-12-27 18:26:42 Re: Two small bugs in guc.c
Previous Message Jelte Fennema-Nio 2023-12-27 17:59:47 Re: Should we remove -Wdeclaration-after-statement?