public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
* [RFC] cygport: use -Werror=implicit-function-declarations
@ 2013-10-16 23:42 Yaakov (Cygwin/X)
  2013-10-17  9:38 ` Corinna Vinschen
  0 siblings, 1 reply; 4+ messages in thread
From: Yaakov (Cygwin/X) @ 2013-10-16 23:42 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]

One of the more common causes of SEGVs with x86_64-cygwin packages 
occurs when an implicit function declaration causes an assumption that 
all arguments and the return value are of type 'int' (32 bits) where 
some or all of these are longs or pointers (64 bits on x86_64).  One way 
of avoiding such errors is to add -Werror=implicit-function-declarations 
to the default CFLAGS/OBJCFLAGS, and in my brief testing to date, this 
has already caught a number of cases where this would have broken the 
package on x86_64.

One potential drawback is that this flag may cause false negatives in 
poorly written configure tests; without extensive testing, I can't say 
yet how prevalent this would be, but I think that making this an error 
and perusing config.log is generally easier than making it just a 
warning and having to sift through the entire output of make.

My proposed patch for cygport git master is attached.  Any thoughts? 
And are there any other warning flags which cause similar errors on 
x86_64 that should be added as well?


Yaakov

[-- Attachment #2: cygport-Werror-implicit-decls.patch --]
[-- Type: text/x-patch, Size: 2165 bytes --]

diff --git a/lib/compilers.cygpart b/lib/compilers.cygpart
index d2c9449..91fc16d 100644
--- a/lib/compilers.cygpart
+++ b/lib/compilers.cygpart
@@ -31,9 +31,9 @@ declare -x CC="gcc";
 #  Flags passed to CC when compiling C code.  Individual packages may append
 #  or override this value if they will not build correctly without it.
 #  DEFAULT VALUE
-#  -ggdb -O2 -pipe
+#  -ggdb -O2 -pipe -Werror=implicit-function-declaration
 #****
-declare -x CFLAGS="-ggdb -O2 -pipe";
+declare -x CFLAGS="-ggdb -O2 -pipe -Werror=implicit-function-declaration";
 
 #****v* Compiling/CPPFLAGS
 #  DESCRIPTION
@@ -55,7 +55,7 @@ declare -x CXX="g++";
 #  DEFAULT VALUE
 #  -ggdb -O2 -pipe
 #****
-declare -x CXXFLAGS="${CFLAGS}";
+declare -x CXXFLAGS="-ggdb -O2 -pipe";
 
 #****d* Compiling/F77
 #  DESCRIPTION
@@ -72,7 +72,7 @@ declare -x F77="gfortran";
 #  DEFAULT VALUE
 #  -ggdb -O2 -pipe
 #****
-declare -x FFLAGS="${CFLAGS}";
+declare -x FFLAGS="-ggdb -O2 -pipe";
 
 #****d* Compiling/FC
 #  DESCRIPTION
@@ -89,7 +89,7 @@ declare -x FC="gfortran";
 #  DEFAULT VALUE
 #  -ggdb -O2 -pipe
 #****
-declare -x FCFLAGS="${CFLAGS}";
+declare -x FCFLAGS="-ggdb -O2 -pipe";
 
 #****d* Compiling/GCJ
 #  DESCRIPTION
@@ -106,7 +106,7 @@ declare -x GCJ="gcj";
 #  DEFAULT VALUE
 #  -ggdb -O2 -pipe
 #****
-declare -x GCJFLAGS="${CFLAGS}";
+declare -x GCJFLAGS="-ggdb -O2 -pipe";
 
 #****d* Compiling/GOC
 #  DESCRIPTION
@@ -121,7 +121,7 @@ declare -x GOC="gccgo";
 #  DEFAULT VALUE
 #  -ggdb -O2 -pipe
 #****
-declare -x GOFLAGS="${CFLAGS}";
+declare -x GOFLAGS="-ggdb -O2 -pipe";
 
 #****d* Compiling/OBJC
 #  DESCRIPTION
@@ -134,7 +134,7 @@ declare -x OBJC="${CC}";
 #  Flags passed to OBJC when compiling Objective C code.  Individual packages
 #  may append or override this value if they will not build correctly without it.
 #  DEFAULT VALUE
-#  -ggdb -O2 -pipe
+#  -ggdb -O2 -pipe -Werror=implicit-function-declaration
 #****
 declare -x OBJCFLAGS="${CFLAGS}";
 
@@ -151,7 +151,7 @@ declare -x OBJCXX="${CXX}";
 #  DEFAULT VALUE
 #  -ggdb -O2 -pipe
 #****
-declare -x OBJCXXFLAGS="${CFLAGS}";
+declare -x OBJCXXFLAGS="${CXXFLAGS}";
 
 #****v* Compiling/LDFLAGS
 #  DESCRIPTION

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] cygport: use -Werror=implicit-function-declarations
  2013-10-16 23:42 [RFC] cygport: use -Werror=implicit-function-declarations Yaakov (Cygwin/X)
@ 2013-10-17  9:38 ` Corinna Vinschen
  2013-10-17 17:06   ` Yaakov (Cygwin/X)
  0 siblings, 1 reply; 4+ messages in thread
From: Corinna Vinschen @ 2013-10-17  9:38 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]

On Oct 16 18:42, Yaakov (Cygwin/X) wrote:
> One of the more common causes of SEGVs with x86_64-cygwin packages
> occurs when an implicit function declaration causes an assumption
> that all arguments and the return value are of type 'int' (32 bits)
> where some or all of these are longs or pointers (64 bits on
> x86_64).  One way of avoiding such errors is to add
> -Werror=implicit-function-declarations to the default
> CFLAGS/OBJCFLAGS, and in my brief testing to date, this has already
> caught a number of cases where this would have broken the package on
> x86_64.
> 
> One potential drawback is that this flag may cause false negatives
> in poorly written configure tests; without extensive testing, I
> can't say yet how prevalent this would be, but I think that making
> this an error and perusing config.log is generally easier than
> making it just a warning and having to sift through the entire
> output of make.

Uh... I'm not so sure about that.  I would rather see the configure
tests running through, but like the idea to have
-Werror=implicit-function-declaration set for the actual make stage.

It's more important that configure tests run to check for existence
of stuff, and not being able to configure is a really annoying
problem.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] cygport: use -Werror=implicit-function-declarations
  2013-10-17  9:38 ` Corinna Vinschen
@ 2013-10-17 17:06   ` Yaakov (Cygwin/X)
  2013-10-17 19:14     ` Corinna Vinschen
  0 siblings, 1 reply; 4+ messages in thread
From: Yaakov (Cygwin/X) @ 2013-10-17 17:06 UTC (permalink / raw)
  To: cygwin-apps

On 2013-10-17 04:38, Corinna Vinschen wrote:
> On Oct 16 18:42, Yaakov (Cygwin/X) wrote:
>> One potential drawback is that this flag may cause false negatives
>> in poorly written configure tests; without extensive testing, I
>> can't say yet how prevalent this would be, but I think that making
>> this an error and perusing config.log is generally easier than
>> making it just a warning and having to sift through the entire
>> output of make.
>
> Uh... I'm not so sure about that.  I would rather see the configure
> tests running through, but like the idea to have
> -Werror=implicit-function-declaration set for the actual make stage.

There is no practical way of changing CFLAGS between the configure and 
make stages.

> It's more important that configure tests run to check for existence
> of stuff, and not being able to configure is a really annoying
> problem.

configure would still run, but poorly written tests could return "no" 
when the correct answer is "yes".


Yaakov

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] cygport: use -Werror=implicit-function-declarations
  2013-10-17 17:06   ` Yaakov (Cygwin/X)
@ 2013-10-17 19:14     ` Corinna Vinschen
  0 siblings, 0 replies; 4+ messages in thread
From: Corinna Vinschen @ 2013-10-17 19:14 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 2306 bytes --]

On Oct 17 12:06, Yaakov wrote:
> On 2013-10-17 04:38, Corinna Vinschen wrote:
> >On Oct 16 18:42, Yaakov (Cygwin/X) wrote:
> >>One potential drawback is that this flag may cause false negatives
> >>in poorly written configure tests; without extensive testing, I
> >>can't say yet how prevalent this would be, but I think that making
> >>this an error and perusing config.log is generally easier than
> >>making it just a warning and having to sift through the entire
> >>output of make.
> >
> >Uh... I'm not so sure about that.  I would rather see the configure
> >tests running through, but like the idea to have
> >-Werror=implicit-function-declaration set for the actual make stage.
> 
> There is no practical way of changing CFLAGS between the configure
> and make stages.

Too bad.

> >It's more important that configure tests run to check for existence
> >of stuff, and not being able to configure is a really annoying
> >problem.
> 
> configure would still run, but poorly written tests could return
> "no" when the correct answer is "yes".

That doesn't sound right to me.  The standard tests in configure should
not fail due to a CFLAGS setting.  They usually won't in case of later
autoconf versions which always add a prototype when checking for the
existance of a function, but what about configure tests which test
for *required* functionality.  There are enough configure scripts
out there which stop execution if some test fails, often version
tests of libs.  Consider a test which checks for at least version 2.3.4
of libfoo, which fails because of the -Werror=... setting.  The build
failure might lead to a misleading message along the lines of:

"You don't have libfoo installed.  Please install at least version 2.3.4"

That would be not exactly helpful, and not exactly obvious to autoconf
non-experts.

So, I'm still unsure.  As much as I like the idea to add the
implicit-function-declaration error to the build stage, it should not
break an otherwise working configure, IMHO.

But it might be interesting to see what other people think.  Maybe
I'm just too cautious here.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-10-17 19:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-16 23:42 [RFC] cygport: use -Werror=implicit-function-declarations Yaakov (Cygwin/X)
2013-10-17  9:38 ` Corinna Vinschen
2013-10-17 17:06   ` Yaakov (Cygwin/X)
2013-10-17 19:14     ` Corinna Vinschen

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).