public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Xi Ruoyao <xry111@xry111.site>
To: Jonny Grant <jg@jguk.org>,
	Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>,
	GNU C Library <libc-alpha@sourceware.org>,
	Paul Eggert <eggert@cs.ucla.edu>
Subject: Re: glibc misc/sys/cdefs.h nonull - typo in comment
Date: Thu, 13 Apr 2023 00:26:48 +0800	[thread overview]
Message-ID: <21bc9125ab8ced26aa85f3f787f084c4af460a18.camel@xry111.site> (raw)
In-Reply-To: <514c11a4-405b-f7f3-9a67-0b6c10ad7740@jguk.org>

On Wed, 2023-04-12 at 16:56 +0100, Jonny Grant wrote:

> Do you think it is risky to have the nonnull attribute in use in glibc?
> Some projects have abandoned attribute nonnull due to the GCC optimizer using the nonnull attribute to remove the runtime null pointer checks.
> https://gitlab.com/gnuwget/wget2/-/issues/200

No because in the C standard & POSIX standard the text is clear that
many arguments just should not be NULL.  So the implementation does not
need to check if such an argument is NULL.

If the standard allows an argument to be NULL but there is attribute
nonnull there, it's a bug.  Please open a ticket in
https://sourceware.org/bugzilla if you find such a bug.  But when there
is no bugs, we have no reason to remove avoid nonnull for "comforting
some people".

> Currently cdefs.h has
> 
> /* The nonnull function attribute marks pointer parameters that
>    must not be NULL.  This has the name __nonnull in glibc,
>    and __attribute_nonnull__ in files shared with Gnulib to avoid
>    collision with a different __nonnull in DragonFlyBSD 5.9.  */
> 
> Is it better to clarify this to be something like the following?
> 
> /* The nonnull function attribute marks pointer parameters that
>    the compiler's optimizer knows will never be NULL. This means NULL checks can be optimized out.

No because such NULL checks which can be removed by nonnull attribute
should not exist in Glibc.  If any one exists, it's a bug (CWE-1164
"Irrelevant Code").  Again open a ticket if you find one.  But I guess
you won't find any, because such a bug will be rejected by "gcc -Wall -
Werror" and there are many people building Glibc with this.

And the user should not assume any implementation details in Glibc
functions.  For example:

size_t
f (const char *s)
{
  size_t r = strlen (s);
  return s ? r : 0;
}

will do strange things no matter if nonnull is used for strlen.  Even if
you remove the nonnull attribute of strlen and expect this thing to
work, the different implementations (in many Glibc ports, heavily
optimized assembly code) can still do unexpected things.  The people
writing the implementation of strlen will assume the argument is not
null anyway because the standard says so.

So this is just broken in the nature, regardless of nonnull or not.

OTOH

size_t
f (const char *s)
{
  return s ? strlen (r) : 0;
}

is well-defined.  The compiler is prohibited from removing the check, no
matter if nonnull is used for strlen.  If the compiler removed the
check, it's a bug.  Again if there is a bug, report it; but if there is
no bugs, you cannot tell people to "fix" it just because "it seems
risky".

Anyway Glibc (and GCC) are C implementations for real C programs, not
for "allowing people who don't know C to programming in C".

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

  reply	other threads:[~2023-04-12 16:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-11  9:09 Jonny Grant
2023-04-11 13:39 ` Adhemerval Zanella Netto
2023-04-12 15:56   ` Jonny Grant
2023-04-12 16:26     ` Xi Ruoyao [this message]
2023-04-12 16:31       ` Xi Ruoyao
2023-10-28 23:50       ` Jonny Grant
2023-10-29  5:24         ` Paul Eggert
2023-10-29 22:43           ` Jonny Grant
2023-10-30  9:04             ` Andreas Schwab
2023-10-30 10:10               ` Xi Ruoyao
2023-12-03 22:09                 ` Jonny Grant
2023-11-01  7:38           ` Florian Weimer
2023-11-01 19:30             ` Paul Eggert
2023-11-01 19:52               ` Jonny Grant
2023-11-01 20:05                 ` Florian Weimer
2023-11-01 20:16                   ` Jonny Grant
2023-11-01 20:06               ` Florian Weimer
2023-04-12 17:28     ` Adhemerval Zanella Netto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=21bc9125ab8ced26aa85f3f787f084c4af460a18.camel@xry111.site \
    --to=xry111@xry111.site \
    --cc=adhemerval.zanella@linaro.org \
    --cc=eggert@cs.ucla.edu \
    --cc=jg@jguk.org \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).