bugmake - Bugs: bug #34608, comparison of unsigned expression...

 
 

bug #34608: comparison of unsigned expression < 0 is always false

Submitter:  Sebastian Pipping <hartwork>
Submitted:  Thu 20 Oct 2011 10:28:03 PM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  4.0 Operating System:  Any
Fixed Release:  4.0 Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 14 Nov 2011 06:55:30 PM UTC, comment #13: 

Doing it at configure time is quite easy, and just as portable.  Specifying negative array lengths will reliably fail at compile time.  This trick is already used by several autoconf tests (including AC_COMPUTE_INT last I looked).

Again, what you have looks like it works just fine; there's no reason to change.  I just wanted to raise awareness of the configure-time option.

Daniel Herring <dherring>
Mon 14 Nov 2011 06:18:14 PM UTC, comment #12: 

Doing it at configure time is surprisingly difficult.  You have to remember that, for any package which wants to properly cross-compile (which we do), you can't actually RUN any test programs during configure time.  In order to test it you'd need to determine a way to do so that works reliably, is portable between all the different compilers, and can be detected solely by the compiler or linker, without running any resulting code.

Also, the macro looks gross but any compiler in the world will simplify it to a constant at compile-time and optimize out all the unused stuff anyway.

Paul D. Smith <psmith>
Group administrator
Mon 14 Nov 2011 06:08:28 PM UTC, comment #11: 

Don't reopen this for me, but you might consider doing this check at configure time.

From a quick look, I didn't see something like AC_IS_SIGNED.  However it shouldn't be hard to wrap AC_COMPILE_IFELSE around an array-bound test that triggers a failure if a type is unsigned.  From a quick web search, here's an example test.
http://blog.tsunanet.net/2008/09/compile-time-assertion-to-detect-signed.html

Daniel Herring <dherring>
Mon 14 Nov 2011 11:31:44 AM UTC, comment #10: 

Very nice, thanks!

Sebastian Pipping <hartwork>
Mon 14 Nov 2011 02:27:25 AM UTC, comment #9: 

OK I fixed it along the lines you suggested.  Also cleaned up a few other warnings.  Currently make compiles with -Wall -Wextra (plus a few others) and -O2, with no warnings.

Paul D. Smith <psmith>
Group administrator
Sun 13 Nov 2011 11:23:49 PM UTC, comment #8: 

Not sure why that happens although the C standard requires special handling of char in various ways.  The warnings you're seeing in the source, though, are related to invoking this macro with uintmax_t not char.

Paul D. Smith <psmith>
Group administrator
Sun 13 Nov 2011 11:17:38 PM UTC, comment #7: 

Interesting.  For you code (with "unsigned int") I get the warning, too.  For "unsigned char", I don't.  No warnings in either case for "> 0" (as opposed to ">= 0".

Sebastian Pipping <hartwork>
Sun 13 Nov 2011 10:37:23 PM UTC, comment #6: 

This is what I get:


$ cat foo.c
#define INTEGER_TYPE_SIGNED(t) !((t) -1 >= 0)
int foo() { return INTEGER_TYPE_SIGNED(unsigned int); }

$ gcc --version
gcc (Ubuntu/Linaro 4.4.4-14ubuntu5) 4.4.5

$ gcc -Wextra -g -c -o foo.o foo.c
foo.c: In function 'foo':
foo.c:2: warning: comparison of unsigned expression >= 0 is always true


Paul D. Smith <psmith>
Group administrator
Sun 13 Nov 2011 09:50:35 PM UTC, comment #5: 

You mentioned !((t) -1 >= 0) .
I do not get any warnings when using that in the mentioned code sample compiled with -Wall -Wextra and GCC 4.4.5.

What warnings do you get?

Sebastian Pipping <hartwork>
Sun 13 Nov 2011 09:34:24 PM UTC, comment #4: 

Can you re-open the bug until you made a decision?

Sebastian Pipping <hartwork>
Sun 13 Nov 2011 08:51:36 PM UTC, comment #3: 

Hm.  This change is not identical to the original.  To get that you'd need to write !((t) -1 >= 0) and if you do that then GCC warns again.

However I'm not aware offhand of any numeric implementations where casting a -1 to an unsigned value yields 0 so maybe this change would be sufficient.

Paul D. Smith <psmith>
Group administrator
Sun 13 Nov 2011 08:23:10 PM UTC, comment #2: 

Thanks for these details.  Let me propose

  #define INTEGER_TYPE_SIGNED(t) !((t) -1 > 0)

for a warning-less replacement then.  Like it?

Please re-open the bug for me, as I lack permissions to.

I used the program below with

  # gcc -Wall -Wextra main.c && ./a.out

as a test:

================================================================
#include <stdio.h>

#define INTEGER_TYPE_SIGNED(t) !((t) -1 > 0)

int main() {
printf("signed %d\n", INTEGER_TYPE_SIGNED(unsigned char));
printf("signed %d\n", INTEGER_TYPE_SIGNED(signed char));
printf("signed %d\n", INTEGER_TYPE_SIGNED(char));
return 0;
}
================================================================

Sebastian Pipping <hartwork>
Sun 13 Nov 2011 07:51:08 PM UTC, comment #1: 

It turns out that this warning is actually intentional.  If you follow this through you'll see it ends up in an invocation of this macro:

/* Nonzero if the integer type T is signed.  */
#define INTEGER_TYPE_SIGNED(t) ((t) -1 < 0)

The idea is to determine whether a type is signed or not, and to do this we cast a negative number to the type and see if it's still negative (less than 0).  If it is then the type is signed; if not, not.

For systems where the type we pass in is actually unsigned (in this case uintmax_t) you'll get this warning.  However the behavior of the program is exactly what we expect and want.

Paul D. Smith <psmith>
Group administrator
Thu 20 Oct 2011 10:28:03 PM UTC, original submission:  

I wouldn't report warnings here if they didn't look like strong bug candidates:

# make clean all |& grep "always false"
file.c:797: warning: comparison of unsigned expression < 0 is always false
file.c:798: warning: comparison of unsigned expression < 0 is always false
file.c:801: warning: comparison of unsigned expression < 0 is always false
main.c:1872: warning: comparison of unsigned expression < 0 is always false
main.c:2214: warning: comparison of unsigned expression < 0 is always false
remake.c:475: warning: comparison of unsigned expression < 0 is always false
remake.c:920: warning: comparison of unsigned expression < 0 is always false
remake.c:1312: warning: comparison of unsigned expression < 0 is always false
remake.c:1325: warning: comparison of unsigned expression < 0 is always false
vpath.c:441: warning: comparison of unsigned expression < 0 is always false

Sebastian Pipping <hartwork>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dherring (Posted a comment)
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by hartwork (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-11-14 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Operating SystemNone Any
        Fixed ReleaseNone 4.0
    2011-11-13 psmith StatusNot A Bug None
        Open/ClosedClosed Open
    2011-11-13 psmith StatusNone Not A Bug
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code