Bug 575984 – x11-libs/vte-0.42.4 uses NULL as sentinel in variadic functions, fails to emerge with musl

• Product: Gentoo Linux
• Status: RESOLVED FIXED

[Description Anthony Basile 2016-02-29 10:36:20 UTC]

Created attachment 426976 [details]
Replace NULL with (void*)0 when used as sentinel in variadic functions

NULL is defined as a null pointer constant, but this causes problems
when used in variadic functions, leading to hard to diagnose crashes.
In fact, musl intentionally defines NULL to be 0L to throw errors
in these cases.  This patch replaces NULL used as sentinels in variadic
functions with (void *)0.  See

[1] http://ewontfix.com/11/
[2] https://gustedt.wordpress.com/2010/11/07/dont-use-null/

[Comment 1 Anthony Basile 2016-02-29 10:39:12 UTC]

I have tested the patch on glibc/uclibc and musl and it works.  I have submitted the patch upstream.

[Comment 2 Tolga Dalman 2016-02-29 12:32:23 UTC]

Umm ... while your patch looks correct, I don't think this is the right place to fix. My reading is that musl's NULL implementation (0L) is wrong (albeit complying to C99/C11) and can lead to various interesting problems.

From glib API point-of-view, this also seems wrong. The API expects NULL as terminator, not (void*) 0.

Note, that this is the definition for __attribute__((sentinel)):

---------------------------------
sentinel
This function attribute ensures that a parameter in a function call is an explicit NULL. The attribute is only valid on variadic functions. By default, the sentinel is located at position zero, the last parameter of the function call. If an optional integer position argument P is supplied to the attribute, the sentinel must be located at position P counting backwards from the end of the argument list.
          __attribute__ ((sentinel))
          is equivalent to
          __attribute__ ((sentinel(0)))
The attribute is automatically set with a position of 0 for the built-in functions execl and execlp. The built-in function execle has the attribute set with a position of 1.

A valid NULL in this context is defined as zero with any pointer type. If your system defines the NULL macro with an integer type then you need to add an explicit cast. GCC replaces stddef.h with a copy that redefines NULL appropriately.

The warnings for missing or incorrect sentinels are enabled with -Wformat. 
---------------------------------
(https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes)

[Comment 3 Anthony Basile 2016-02-29 18:20:59 UTC]

Upsteam has included the fix in git/master and 0.44.

[Comment 4 Pacho Ramos 2016-02-29 19:42:18 UTC]

(In reply to Anthony Basile from comment #3)
> Upsteam has included the fix in git/master and 0.44.

Feel free to go ahead and commit it yourself if you need it just now :) (please add a comment with a link to the bug for example for future reference)

Thanks

[Comment 5 Anthony Basile 2016-02-29 22:44:36 UTC]

(In reply to Pacho Ramos from comment #4)
> (In reply to Anthony Basile from comment #3)
> > Upsteam has included the fix in git/master and 0.44.
> 
> Feel free to go ahead and commit it yourself if you need it just now :)
> (please add a comment with a link to the bug for example for future
> reference)
> 
> Thanks

Unfortunately nullptr is only in c++11 and above [1], and we don't want to be introducing c++11 yet.  If I back port this, I would only be able to do so as ((void*)0).  I think we can wait, but can the next stabilization candidate be 0.44 so we don't have to wait forever?

[1] http://en.cppreference.com/w/cpp/language/nullptr

[Comment 6 Pacho Ramos 2016-03-05 10:24:53 UTC]

Well, I cannot promise we will be able to stabilize vte-0.44 for Gnome 3.18 as they are usually tied to gnome versions (and, then, you would need to wait for Gnome 3.20)... but if 3.18 terminal and co. work with newer vte, no problem

[Comment 7 Anthony Basile 2016-03-05 12:27:23 UTC]

(In reply to Pacho Ramos from comment #6)
> Well, I cannot promise we will be able to stabilize vte-0.44 for Gnome 3.18
> as they are usually tied to gnome versions (and, then, you would need to
> wait for Gnome 3.20)... but if 3.18 terminal and co. work with newer vte, no
> problem

Okay, let's play it by ear.  The musl overlay is pretty big, and while the hope is to get everything into the tree, this really means teaching a lot of upstreams a lot about proper standards.  musl is interesting because of its very close adherence to POSIX, SUSv3/4, etc as well as secure programming so it exposes a lot of these problems.