Re: CVS commit: src/sys/sys

[christos%astron.com@localhost (Christos Zoulas)]

In article <20170117155009.GA13465%britannica.bec.de@localhost>,
Joerg Sonnenberger  <joerg%bec.de@localhost> wrote:
>On Tue, Jan 17, 2017 at 01:13:07PM +0000, Maya Rashish wrote:
>> Module Name:	src
>> Committed By:	maya
>> Date:		Tue Jan 17 13:13:07 UTC 2017
>> 
>> Modified Files:
>> 	src/sys/sys: time.h
>> 
>> Log Message:
>> define constants as being wider than int when needed, instead of casting
>> them at use. makes compilers happier.
>
>Besides the already fixed bug, can we please just go back to
>"-Wsystem-header considered harmful"?

The default state of having system header warnings disabled is broken
because it hides all kinds of bugs in the headers, and also conseals
unintended user interaction with the headers:

	- one header does #define FOO 1, another does #define FOO 2
	  you don't get warned; case in point MACHINE_ARCH on ppc64,
	  MACHINE on cesfic.

	- fortify is castrated (we found bugs in sysinst):

		char foo[5];
		strncat(foo, "bazfoo", sizeof(foo));

		cc -O2 --param ssp-buffer-size=1 -Wstack-protector \
		    -D_FORTIFY_SOURCE=2 -Wall -Wsystem-headers \
		    -fstack-protector -c ssp.c
		In file included from /usr/include/string.h:126:0,
		from ssp.c:2:
		ssp.c: In function 'main':
		ssp.c:8:2: warning: call to __builtin___strncat_chk will \
		always overflow destination buffer

	- since there is an arbitrary boundary between user + system header
	  code, context is lost (the ctype problem):

		# 5 "isblank.c" 3 4
		       ((int)((_ctype_tab_ + 1)[(
		# 5 "isblank.c"
		       ac
		# 5 "isblank.c" 3 4
		       )] & 0x0200))
		# 5 "isblank.c"
                  ;

          this basically says, 3 == system header, 4 == in "c" context
	  (extern "C")... So the array expression now (gcc 5.x) does not
	  produce a warning anymore. gcc 4.8 did this as:

	       ((int)((_ctype_tab_ + 1)[(ac)] & 0x0200));
 
	  Another set of bugs were revealed in ATF code, because we
	  have large macros defined in <atf-c.h>, so user code warnings
          get hidden .
	  
	- Who decides what warnings are disabled for system headers?
	  should I be warned for a structure declaration inside a parameter
          list (I don't get warned).

	- If I do:

		#define INFTIM 3
		#include <poll.h>

	  shouldn't I get a warning? Because I don't!
	
Anyway, what they did is a gross hack with unintended consequences. If
we want to encourage bugs and abuse in the system headers, we should
turn it back off.

christos