Re: Patch to execve

["Kevin M. Kilbride" <kmk@xxxxxxx>]

Joerg Sonnenberger wrote:

> On Fri, Feb 25, 2005 at 02:21:59PM -0800, Kevin M. Kilbride wrote:
>  
>
> > > I agree. Adding a const attribute to system interfaces will _never_ 
> > > break calling code, since it simply advertises a guarantee that the 
> > > code will not change what is supplied. The code either does or it 
> > > doesn't. If it truly doesn't, then why not advertise it as such? It 
> > > can hardly be argued as "breaking" the POSIX interface. There is no 
> > > real hope of making all of userland truly WARNS=6 compliant without it.
> > >      
>
> What about code which correctly uses -Wbad-function-cast or
> -Wcast-qual in combination with -Werror? That code breaks.
> const char ** and char ** are not type compatibel in ISO C.
> That's IMO a shortcoming in ISO C, but we can't fix it.
> Changing the interface adds problems. It is possible to use
> e.g. __DECONST for exactly this occurences, but it has to be
> done explicitly and it is easy to find all users of such
> macros to verify the correctness of those.
>
>  
Actually, the use of -Wbad-function-cast does not apply, as it deals 
only with return values---I am not suggesting that any qualifiers be 
added to the function type. That _would_ break things. The issue of type 
convertibility for call parameters and the GCC -Wcast-qual option is, 
however, a far more subtle matter....

I do not profess to be an expert on standard C---it has been a very long 
time since I have programmed in it (as opposed to C++). Nevertheless, I 
was able to download draft n843 of the C99 standard and (to my immense 
relief) found that you overlooked a significant provision. Specifically, 
although qualified types are only considered to be compatible if their 
qualifiers match, in section 6.3.2.3 (Pointers), in paragraph 2, the 
draft of the standard I have explicitly states that:

"For any qualifier q, a pointer to a non-q-qualified type may be 
converted to a pointer to the q-qualified version of the type; the 
values stored in the original and converted pointers shall compare equal."

The const qualifier, in particular, would be nearly useless without this 
provision. It is, therefore, completely standards-compliant for the 
formal parameters of a function to provide qualification guarantees more 
restrictive than those of the actual call parameters.

You did, however, bring to light two long-undiscovered bugs in the 
-Wcast-qual debug facility of GCC---ones which are still in version 3.4 
of the compiler. I will report them immediately. The documentation for 
GCC claims (appropriately) that -Wcast-qual will:

"Warn whenever a pointer is cast so as to REMOVE a type qualifier from 
the target type. For example, warn if a const char * is cast to an 
ordinary char * " [emphasis mine]

It fails to do this on two counts. First, it fails to warn about the 
removal of type qualifiers from void pointers, as someone obviously 
discovered with their "clever" implementation of __DECONST in cdefs. It 
is precisely this kind of hack that the -Wcast-qual option is supposed 
to catch. If programmers can engage in unsafe behaviors, yet cleverly 
evade detection by a project's unit testers and integrators with hacks 
like __DECONST, then I deem the -Wcast-qual option utterly useless for 
enterprise programming. Secondly, it not only warns about _stripping_ 
qualifiers, but, in direct violation of the standard, it also warns 
about the explicitly-permitted conversion of an unqualified pointer to a 
qualified one. It is GCC that is non-compliant, even according to its 
own documentation, and you are exploiting a bug in GCC to silently 
discard the const qualifier of objects. By the ISO spec, the statement I 
made originally is absolutely correct in all circumstances.

Fortunately, the broken -Wcast-qual facility in GCC is hardly a show 
stopper. No portable code would ever attempt to make use of such 
compilation options, and any package that did could simply be patched in 
the ports system to make it compile. Insisting on having a warnings-free 
userland, in conjunction with untouchable kernel and library call 
signatures, will encourage, ironically, the conversion of existing safe 
behaviors into unsafe ones. For example, owing to the fatal combination 
of -Wwrite-strings and (the broken) -Wcast-qual in the WARNS=6 
combination, if you want to pass a constant string to a function that 
does not provide a const guarantee, you have to declare or allocate a 
temporary string and pass that, after copying the constant instance into 
it. Not only is this asinine, it is also completely unsafe. It actually 
_reduces_ the robustness of the code in a vain attempt to achieve 
"warnings-free" status.

Unless you use the -fwritable-strings compilation option, any attempt to 
modify a string literal will protection-fault your application. This is 
very safe behavior. Dead programs never do anything dangerous. If, on 
the other hand, you copy that string literal into a buffer that is 
allocated on the stack and you pass the writable copy to a function that 
"should" treat it as a constant, but doesn't, you immediately open the 
door to failure modes that could be much worse than a simple program 
crash. Ironically, in the attempt to improve robustness and security by 
eliminating compiler warnings, you will have reduced both. Even if the 
buffer is statically declared and lives in the BSS segment, you still 
have reduced the intrinsic safety of your application.

This calls into question the very wisdom of the WARNS=6 project. If it 
is unacceptable to alter the signatures of library and kernel calls 
(even in completely safe and standards-compliant ways), then eliminating 
these warnings from userland code may not be a very good idea.