Bug 26167

Summary: -Wconversion fails to detect signedness conversion from int to unsigned int in fuction call
Product: gcc Reporter: Kristian Hermansen <kristian.hermansen>
Component: c++Assignee: Not yet assigned to anyone <unassigned>
Severity: enhancement CC: gcc-bugs, m.becker, manu, mueller
Priority: P3 Keywords: diagnostic
Version: 4.0.2   
Target Milestone: 4.3.0   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2006-08-21 05:41:07
Bug Depends on:    
Bug Blocks: 26298    
Attachments: wcoercion patch r116922

Description Kristian Hermansen 2006-02-08 01:10:01 UTC
gcc reports the signedness probelm correctly, but g++ does not. See below for a demonstration:

$ cat signedness.cpp
#include <iostream>
using namespace std;

void foo (unsigned int a){
cout << a << endl;

int main(){
int b = -1;
foo (b);
return 0;

$ g++ -Wall -Wconversion -o signedness signedness.cpp


$ g++ -v -save-temps -Wall -Wconversion -o signedness signedness.cpp Using built-in specs.
Target: i486-linux-gnu
Configured with: ../src/configure -v --enable-languages=c,c++,java,f95,objc,ada,treelang --prefix=/usr --with-gxx-include-dir=/usr/include/c++/4.0.2 --enable-shared --with-system-zlib --libexecdir=/usr/lib --enable-nls --without-included-gettext --enable-threads=posix --program-suffix=-4.0 --enable-__cxa_atexit --enable-libstdcxx-allocator=mt --enable-clocale=gnu --enable-libstdcxx-debug --enable-java-gc=boehm --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-4.0- --enable-mpfr --disable-werror --enable-checking=release i486-linux-gnu
Thread model: posix
gcc version 4.0.2 20050808 (prerelease) (Ubuntu 4.0.1-4ubuntu9)
 /usr/lib/gcc/i486-linux-gnu/4.0.2/cc1plus -E -quiet -v -D_GNU_SOURCE signedness.cpp -mtune=i486 -Wall -Wconversion -fpch-preprocess -o signedness.ii
ignoring nonexistent directory "/usr/local/include/i486-linux-gnu" ignoring nonexistent directory "/usr/include/i486-linux-gnu"
#include "..." search starts here:
#include <...> search starts here:
End of search list.
 /usr/lib/gcc/i486-linux-gnu/4.0.2/cc1plus -fpreprocessed signedness.ii -quiet -dumpbase signedness.cpp -mtune=i486 -auxbase signedness -Wall -Wconversion -version -o signedness.s
GNU C++ version 4.0.2 20050808 (prerelease) (Ubuntu 4.0.1-4ubuntu9) (i486-linux-gnu)
        compiled by GNU C version 4.0.2 20050808 (prerelease) (Ubuntu 4.0.1-4ubuntu9).
GGC heuristics: --param ggc-min-expand=98 --param ggc-min-heapsize=128479
 as -V -Qy --32 -o signedness.o signedness.s
GNU assembler version 2.16.1 (i486-linux-gnu) using BFD version 2.16.1 Debian GNU/Linux
 /usr/lib/gcc/i486-linux-gnu/4.0.2/collect2 --eh-frame-hdr -m elf_i386 -dynamic-linker /lib/ld-linux.so.2 -o signedness /usr/lib/gcc/i486-linux-gnu/4.0.2/../../../../lib/crt1.o /usr/lib/gcc/i486-linux-gnu/4.0.2/../../../../lib/crti.o /usr/lib/gcc/i486-linux-gnu/4.0.2/crtbegin.o -L/usr/lib/gcc/i486-linux-gnu/4.0.2 -L/usr/lib/gcc/i486-linux-gnu/4.0.2 -L/usr/lib/gcc/i486-linux-gnu/4.0.2/../../../../lib -L/usr/lib/gcc/i486-linux-gnu/4.0.2/../../.. -L/lib/../lib -L/usr/lib/../lib signedness.o -lstdc++ -lm -lgcc_s -lgcc -lc -lgcc_s -lgcc /usr/lib/gcc/i486-linux-gnu/4.0.2/crtend.o /usr/lib/gcc/i486-linux-gnu/4.0.2/../../../../lib/crtn.o
Kristian Hermansen
Comment 1 Richard Biener 2006-02-08 11:36:49 UTC
Confirmed.  The C frontend warns about this.
Comment 2 Dirk Mueller 2006-02-08 14:46:10 UTC
ugh, that warning isn't even in -Wextra. 
Comment 3 Gabriel Dos Reis 2006-02-08 15:32:12 UTC
Subject: Re:  -Wconversion fails to detect signedness conversion from int to unsigned int in fuction call

"mueller at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| ugh, that warning isn't even in -Wextra. 

I'll be happy to review patch to make the warning work in the C++
front-end.  I'll do it myself if nobody feels like tackling it :-)

-- Gaby
Comment 4 Kristian Hermansen 2006-02-26 05:56:12 UTC
Go ahead and tackle it.  I'm not going to fix this myself...
Kristian Hermansen
Comment 5 Andrew Pinski 2006-08-21 05:41:07 UTC
This should never have been in waiting.
Comment 6 Manuel López-Ibáñez 2006-09-16 01:08:09 UTC
In which way gcc reports the problem correctly? What gcc currently reports is that if the prototype were missing the value would be passed as a signed int. It is not warning you about the conversion, it warns you about the effect of having a prototype.

If you mean that gcc (and g++) should warn that a signed variable is passed to a function that expects an unsigned variable, then when using the -Wcoercion flag (provided by the Wcoercion project [*]), both cc1 and cc1plus report:

pr26167.cpp:10: warning: coercion to 'unsigned int' from 'int' may alter its value

[*] http://gcc.gnu.org/wiki/Wcoercion
Comment 7 Kristian Hermansen 2006-09-17 18:37:04 UTC
Will gcc/g++ integrate the Wcoercion project's solution?  It seems to have been sponsored by Google's Summer of Code 2006.  Maybe it will be tested/included soon?  Here is the developer: lopezibanez@gmail.com

Should we contact him?  -Wcoercion would be very useful for warning about many security flaws in software...
Comment 8 Kristian Hermansen 2006-09-17 18:39:45 UTC
Sorry, I didn't notice at first that this was you!!!  Will you integrate Wcoercion into gcc/g++ soon or will it always remain a side project?  It would be nice to see such a warning come from using -Wall.
Comment 9 Martin Michlmayr 2006-09-17 20:31:20 UTC
It's aiming for inclusion in GCC 4.3, see http://gcc.gnu.org/wiki/GCC_4.3_Release_Planning
Comment 10 Manuel López-Ibáñez 2006-09-18 08:16:05 UTC
Yes, I hope to get it into 4.3. Nonetheless, if you wish to test it, I can add the patch here.
Comment 11 Dirk Mueller 2006-09-18 12:11:18 UTC
yes please. Actually I created my own patch for bringing the C++ frontend on ear with the C frontend, but I didn't submit it because it produced bazillions of (legal) warnings in the code I usually compile, too many to be useful. 

Comment 12 Manuel López-Ibáñez 2006-09-18 12:22:49 UTC
Created attachment 12291 [details]
wcoercion patch r116922

Patch for trunk revision 116922 (bootstrapped and tested on i686-pc-linux-gnu).
Comment 13 Manuel López-Ibáñez 2006-09-18 12:45:38 UTC
(In reply to comment #11)
> yes please. Actually I created my own patch for bringing the C++ frontend on
> ear with the C frontend, but I didn't submit it because it produced bazillions
> of (legal) warnings in the code I usually compile, too many to be useful. 

I would like to take a look to your patch if that is possible.

A few things worth commenting on the wcoercion patch:

1) One of the development goals is to separate the original behaviour of Wconversion in the C front end from the "other behaviour" of Wconversion (slightly present on the C front end and pervasive in the C++ front end). Therefore, Wconversion has been replaced by Wtraditional-conversion and Wcoercion (the interesting one). The names are tentative, pending discussion by GCC developers, and likely will change in the future. If you find all this confusing, please read http://gcc.gnu.org/wiki/Wcoercion#Background. If you work in the KDE project, it would be interesting (for me) to know your opinion on how to handle the transition.

2) Wconversion option is not completely deleted by my patch, it still exists for C++/Fortran/ObjC++ when the behaviour has not been modified and it didn't conflict with Wtraditional-conversion or Wcoercion. Properly speaking, these Wconversion warnings should be moved to Wcoercion. However, there is little point on doing this until there is a definitive decision on the names of the new options. Thus, to obtain the full set of coercion warnings in C++ you may need to specify "-Wconversion -Wcoercion" (In short, Wconversion are the ones that were already there undocumented, Wcoercion are new or modified).

3) There are a few issuess pending the review of GCC developers. For example, warning messages could be more specific/informative.

4) I don't think we should pollute this bug report further. Thus, feel free to write me directly. Much better would be to discuss things in public in the gcc mail list: we may even get the attention of some GCC developer. 
Comment 14 Dirk Mueller 2006-10-16 15:37:40 UTC
*** Bug 26298 has been marked as a duplicate of this bug. ***
Comment 15 Manuel López-Ibáñez 2006-11-24 01:50:46 UTC
Subject: Bug 26167

Author: manu
Date: Fri Nov 24 01:50:33 2006
New Revision: 119143

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=119143
2006-11-24  Manuel Lopez-Ibanez <manu@gcc.gnu.org>

	PR c/2707
	PR c++/26167
	* c-common.c (conversion_warning): New.
	(convert_and_check): Call conversion_warning unless there is an
	overflow warning.
	* doc/invoke.texi (-Wconversion): Update description.


	* gcc.dg/Wconversion-integer.c: New. Supersedes	
	* gcc.dg/Wconversion-real.c: New.
	* gcc.dg/Wconversion-real-integer.c: New.
	* gcc.dg/Wconversion-negative-constants.c: Deleted.
	* g++.dg/warn/Wconversion1.C: Modified.


Comment 16 Manuel López-Ibáñez 2006-11-24 01:56:39 UTC
Fixed in mainline.