nonnull attribute in installed header vs implementation

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

nonnull attribute in installed header vs implementation

Sourceware - libc-alpha mailing list
We declare fexecve like this:

| /* Execute the file FD refers to, overlaying the running program image.
|    ARGV and ENVP are passed to the new program, as for `execve'.  */
| extern int fexecve (int __fd, char *const __argv[], char *const __envp[])
|      __THROW __nonnull ((2));

But the Linux implementation still has a NULL check for argv:

| int
| fexecve (int fd, char *const argv[], char *const envp[])
| {
|   if (fd < 0 || argv == NULL || envp == NULL)
|     {
|       __set_errno (EINVAL);
|       return -1;
|     }

If I read the nonnull attribute documentation in the GCC manual
correctly, GCC could optimize out the argv == NULL check:

| Unless disabled by the '-fno-delete-null-pointer-checks' option the
| compiler may also perform optimizations based on the knowledge that
| certain function arguments cannot be null.

Should we remove the nonnull attribute, drop the check, or somehow make
sure that the check cannot be compiled out?

Thanks,
Florian

Reply | Threaded
Open this post in threaded view
|

Re: nonnull attribute in installed header vs implementation

Sourceware - libc-alpha mailing list
On Wed, 2020-04-08 at 18:20 +0200, Florian Weimer via Libc-alpha wrote:

> We declare fexecve like this:
>
> > /* Execute the file FD refers to, overlaying the running program image.
> >    ARGV and ENVP are passed to the new program, as for `execve'.  */
> > extern int fexecve (int __fd, char *const __argv[], char *const __envp[])
> >      __THROW __nonnull ((2));
>
> But the Linux implementation still has a NULL check for argv:
>
> > int
> > fexecve (int fd, char *const argv[], char *const envp[])
> > {
> >   if (fd < 0 || argv == NULL || envp == NULL)
> >     {
> >       __set_errno (EINVAL);
> >       return -1;
> >     }
>
> If I read the nonnull attribute documentation in the GCC manual
> correctly, GCC could optimize out the argv == NULL check:
>
> > Unless disabled by the '-fno-delete-null-pointer-checks' option the
> > compiler may also perform optimizations based on the knowledge that
> > certain function arguments cannot be null.
>
> Should we remove the nonnull attribute, drop the check, or somehow make
> sure that the check cannot be compiled out?
I'd recommend keeping the attribute -- GCC should be issuing warnings at the call
site if it detects a NULL pointer for that argument.

I guess the choice between the latter two is a QOI question...

jeff

Reply | Threaded
Open this post in threaded view
|

Re: nonnull attribute in installed header vs implementation

Paul Eggert
On 4/8/20 9:48 AM, Jeff Law via Libc-alpha wrote:
>> Should we remove the nonnull attribute, drop the check, or somehow make
>> sure that the check cannot be compiled out?
> I'd recommend keeping the attribute -- GCC should be issuing warnings at the call
> site if it detects a NULL pointer for that argument.

Yes, we should keep the attribute, as that helps improve overall reliability.

I suggest dropping the check as the alternative (i.e., making sure the check
cannot be compiled out) is more trouble than it's worth overall.