Created attachment 13753 [details]
Demo for abs / printf bug

Use fabs/fabsf for double/float operands. Especially with %f.

GCC printed no warning about disliking a conversion.


I hacked the program in accordance with your suggestion and now it prints:

n= 4  QUESTION1=+0.123  QUESTION2=+0.123  Q(n)=+0.000
n= 5  QUESTION1=+0.099  QUESTION3=+0.099  Q(n)=-0.000
n= 5  QUESTION1=+0.099  QUESTION2=+0.099  Q(n)=-0.000
n= 6  QUESTION1=+0.083  QUESTION3=+0.083  Q(n)=-0.000
n= 6  QUESTION1=+0.083  QUESTION2=+0.083  Q(n)=-0.000
n= 7  QUESTION1=+0.071  QUESTION3=+0.071  Q(n)=+0.000
n= 7  QUESTION1=+0.071  QUESTION2=+0.071  Q(n)=+0.000
n= 8  QUESTION1=+0.062  QUESTION3=+0.062  Q(n)=-0.000


Sometimes the answer is _positive_ zero and sometimes the answer is _negative_ zero.

abs converts the float/double to an integer type so this is not a bug.  If you use 4.3, you can use -Wconversion.

(In reply to comment #3)
> GCC printed no warning about disliking a conversion.

It just happens that gcc is not like microwave oven that has to include warnings about not drying animals in it.

> Sometimes the answer is _positive_ zero and sometimes the answer is _negative_
> zero.

http://en.wikipedia.org/wiki/Floating-point

Thanks for everyones input.


The only issues related to this 'bug' are:

1): printf _sometimes_ prints "-0.000" and sometimes prints "+0.000" - the reason it is even showing the "+" or "-" is because I enabled them using "%+f", this I understand. 

My 2 cents is that the correct answer is "+0.000" and never "-0.000" - UNLESS, that is related to _accuracy_ and nothing to do with "printf"; in which cause that is a different bug.


2): Using a "sprintf()" to print the answer to a char string (which itself is never even used!) "fixes" the bug. This suggests to me that something is upsetting the folding. It can't be an optimization issue since this occurs at any optimization level (even -O0, none) so unless it is the parsing it may be the folding.

Sometimes GCC goes "crazy" (see 6, 8, and 9). If indeed GCC is operating "correctly" it should do the SAME thing everytime - Correct ?

If GCC want to convert it to an INT, fine. the correct answer is ZERO and not:
a): -16522743262502092537856.000
b): +60316137357217275485696900081464849817...(many more digits)...
c): -25531721305534761946185249871767754587...(many more digits)...

Cygwin gives the same (ridiculous) answer _everytime_ which is better though it ought to be ZERO.

If GCC thinks it is doing the correct thing (there is NO BUG) then why doesn't it do the SAME thing - it is saying that the previous time it was wrong and wants to give a different answer, when it ought to give the same answer.

3): If this belongs on http://gcc.gnu.org/bugs.html#nonbugs then this is a documentation bug.

Thanks for considering this report.

Uros Bizjak - has to include warnings about not drying animals in it

I have an older model with no such label therefore I am OK.   ;)

--- Here is another program that demonstrates that there is some problem ---

/*
>>------- Comment #4 From Andrew Pinski 2007-06-21 09:06 [reply] ------- 
>>abs converts the float/double to an integer type so this is not a bug.  If you
>>use 4.3, you can use -Wconversion.

Notice that "b" and "d" are different - there is no (equal or otherwise) "conversion".


The operation of abs (according to printf output) seems to be:

If it is an integer accept and use it. If it is a float then it is = 0 .

Look what printf does to "i", thats an easy conversion.
*/

#include <math.h>
#include <stdio.h>
int main ( ) { 
/* float a, b, c, d, e, f, g, h, i, j; */       /* gives warning "argument x has type 'double'", it */
  double a, b, c, d, e, f, g, h, i, j;  /* might be better to say type "float" instead */
  a = 16.000;
  b =  abs(a);
  c = fabs(a);
  d =  abs((int)(a));
  e = fabs((int)(a));
  f =  abs((float)(a));
  g = fabs((float)(a));
  h = (int)abs(a);
  i = (float)abs(a);
  j = 0.0;

  printf("a = %.2f  b = %.2f  c = %.2f  d = %.2f  e = %.2f  ", a, b, c, d, e);
  printf("f = %.2f  g = %.2f  h = %.2f  i = %.2f  j = %.2f\n", f, g, h, i, j);
  printf("a = %d  b = %d  c = %d  d = %d  e = %d  ", a, b, c, d, e);
  printf("f = %d  g = %d  h = %d  i = %d  j = %d\n", f, g, h, i, j);
  return 0;
}

/* /usr/test/bin/gcc -Wall -Wconversion -o math_test_7 math_test_7.c */

/* Output:
a = 16.00  b = 0.00  c = 16.00  d = 16.00  e = 16.00  f = 0.00  g = 16.00  h = 0.00  i = 0.00  j = 0.00
a = 0  b = 1076887552  c = 0  d = 0  e = 0  f = 0  g = 0  h = 0  i = 1076887552  j = 0
*/

How can that be correct conversion ?

A earlier version of this program had these lines in it:

...
    f =  abs((float)(a));
    g = fabs((float)(a));
    h = (int)abs(a);
    i = 0.0;

    printf("a = %.2f  b = %.2f  c = %.2f  d = %.2f  e = %.2f  ", a, b, c, d, e);
    printf("f = %.2f  g = %.2f  h = %.2f  i = %.2f\n", f, g, h, i);
    printf("a = %d  b = %d  c = %d  d = %d  e = %d  ", a, b, c, d, e);
    printf("f = %d  g = %d  h = %d  i = %d\n", f, g, h, i);
printf("i2 = %d\n", i);
    return 0;
...

Here is the output:

/* Output:
a = 16.00  b = 0.00  c = 16.00  d = 16.00  e = 16.00  f = 0.00  g = 16.00  h = 0.00  i = 0.00
a = 0  b = 1076887552  c = 0  d = 0  e = 0  f = 0  g = 0  h = 0  i = 1076887552
i2 = 0
*/


Notice that the prior time that the 9th variable was "1076887552" (as it is in comment 7). Using gdb I see that print /x 1076887552 = 0x40300000 . It might be using the stack pointer for the value. 

Notice that "i" and "i2" both print the value of "i" and that it is different - it is as if gcc is using a pointer into memory instead of the actual value.

I'll work on some debugging and see if I can find out how it derives the value of "i" that it prints differently each time.
 

(In reply to comment #8)

> I'll work on some debugging and see if I can find out how it derives the value
> of "i" that it prints differently each time.

Don't worry, it works correctly.

The non-problem you are going after is in printf(). It takes variable arguments from the stack and interprets them according to the format string. Argument are pushed to the stack by the caller without any other communication with callee, so it is obvious that format string _must_ reflect the type of values on stack. Also note, that %f inherently converts float type to double, so your values on the stack are FUBAR as far as printf is concerned.

(In reply to comment #9)
> Don't worry, it works correctly.
> ...
> Argument are pushed to the stack by the caller without any other
> communication with callee, so it is obvious that format string _must_ 
> reflect the type of values on stack.
> Also note, that %f inherently converts float type to double, so your values on
> the stack are FUBAR as far as printf is concerned.

That is "working correctly" ?!?

Very well, but not so obvious.


_IF_ we had "argflaps" (taken from the word "mudflaps") we could get printf (or any other function) to type-check the variable on the stack (or anywhere!) at run-time and compare it to what it "thought" it should be. This could be a very powerful feature - but lack of it is not a bug :( .

(In reply to comment #10)
> (In reply to comment #9)
> > Don't worry, it works correctly.
> > ...
> > Argument are pushed to the stack by the caller without any other
> > communication with callee, so it is obvious that format string _must_ 
> > reflect the type of values on stack.
> > Also note, that %f inherently converts float type to double, so your values on
> > the stack are FUBAR as far as printf is concerned.
> 
> That is "working correctly" ?!?
> 
> Very well, but not so obvious.
> 
> 
> _IF_ we had "argflaps" (taken from the word "mudflaps") we could get printf (or
> any other function) to type-check the variable on the stack (or anywhere!) at
> run-time and compare it to what it "thought" it should be. This could be a very
> powerful feature - but lack of it is not a bug :( .
> 

(1) Try -Wformat

(2) Send a patch that implements your argflaps

(3) There is an expectation someone writing C might actually
    adhere to the Standard

> Send a patch that implements your argflaps

Cyclone uses what it calls "thin" and "fat" pointers. We could implement this in GCC. An extra byte per variable could store the type and a bit of code could check it (at run-time) and perform "run-time conversion", use "Syntactic Sugar" or "C overloading" - depending on what you would like to call it..


Investigating:

1): Cyclone is a dialect of C that retains its transparency and control, but adds the benefits of safety (no unchecked runtime errors, for instance). In Cyclone, buffer overflows and related bugs are prevented for all programs, whether written by security experts or by novices. The changes required to achieve safety are pervasive, but Cyclone is still recognizably C.

http://www.eecs.harvard.edu/~greg/cyclone/


2a): CIL (C Intermediate Language) is a high-level representation along with a set of tools that permit easy analysis and source-to-source transformation of C programs. http://hal.cs.berkeley.edu/cil/ . This intermediate language is used to stage both Deputy and CCured.


2b): CCured is a source-to-source translator for C. It analyzes the C program to determine the smallest number of run-time checks that must be inserted in the program to prevent all memory safety violations. The resulting program is memory safe, meaning that it will stop rather than overrun a buffer or scribble over memory that it shouldn't touch. Many programs can be made memory-safe this way while losing only 10–60% run-time performance (the performance cost is smaller for cleaner programs, and can be improved further by holding CCured's hand on the parts of the program that it does not understand by itself). Using CCured we have found bugs that Purify misses with an order of magnitude smaller run-time cost.

http://hal.cs.berkeley.edu/ccured/index.html


2c): Deputy is a C compiler that is capable of preventing common C programming errors, including out-of-bounds memory accesses as well as many other common type-safety errors. It is designed to work on real-world code, up to and including the Linux kernel itself.

Deputy allows C programmers to provide simple type annotations that describe pointer bounds and other important program invariants. Deputy verifies that your program adheres to these invariants through a combination of compile-time and run-time checking.

http://deputy.cs.berkeley.edu/


These projects have been operating for many years, come from a reputable source and integrate with gcc (albiet they do not seem to have tried 4.3.0).


A paper on Deputy - Static Analysis of C for Hybrid Type Checking
http://www.eecs.berkeley.edu/Pubs/TechRpts/2007/EECS-2007-1.pdf


The difference between Duputy and CCured (both implemented on top of the CIL framework) is:

CCured is a whole program analysis that instruments pointers with bounds information. As in Deputy the bounds information may need to be checked at runtime. The CCured system includes an optimization phase that also attempts to reason about bounds checks, null checks, and others. The Deputy type system presents its optimizer with some different challenges than those seen by the CCured optimizer. The primary challenge is the sheer volume of runtime 
checks added by the type system. On the benchmarks presented here, deputized code achieves better performance than cured code, however CCured also enforces allocation safety, which Deputy does not.

The "license" for these in one case is to simply credit the authors.

(In reply to comment #11)
> (1) Try -Wformat

"-Wall" includes "-Wformat" according to gcc.info. See comment 7 for the command line I used:
/* /usr/test/bin/gcc -Wall -Wconversion -o math_test_7 math_test_7.c */


> (3) There is an expectation someone writing C might actually
>     adhere to the Standard

From time to time that does not always occur. See here:
http://gcc.gnu.org/bugzilla/buglist.cgi?emailreporter1=1&emailtype1=substring&email1=kargl@gcc.gnu.org

(In reply to comment #13)
> (In reply to comment #11)
> > (1) Try -Wformat
> 
> "-Wall" includes "-Wformat" according to gcc.info. See comment 7 for the
> command line I used:
> /* /usr/test/bin/gcc -Wall -Wconversion -o math_test_7 math_test_7.c */

mobile:kargl[204] cc -o z -Wformat m.c -lm
m.c: In function 'main':
m.c:21: warning: format '%d' expects type 'int', but argument 2 has type 'double'
m.c:21: warning: format '%d' expects type 'int', but argument 3 has type 'double'
m.c:21: warning: format '%d' expects type 'int', but argument 4 has type 'double'
m.c:21: warning: format '%d' expects type 'int', but argument 5 has type 'double'
m.c:21: warning: format '%d' expects type 'int', but argument 6 has type 'double'
m.c:22: warning: format '%d' expects type 'int', but argument 2 has type 'double'
m.c:22: warning: format '%d' expects type 'int', but argument 3 has type 'double'
m.c:22: warning: format '%d' expects type 'int', but argument 4 has type 'double'
m.c:22: warning: format '%d' expects type 'int', but argument 5 has type 'double'
m.c:22: warning: format '%d' expects type 'int', but argument 6 has type 'double'

mobile:kargl[205] cc -o z -Wall m.c -lm
m.c: In function 'main':
m.c:9: warning: implicit declaration of function 'abs'
m.c:21: warning: format '%d' expects type 'int', but argument 2 has type 'double'
m.c:21: warning: format '%d' expects type 'int', but argument 3 has type 'double'
m.c:21: warning: format '%d' expects type 'int', but argument 4 has type 'double'
m.c:21: warning: format '%d' expects type 'int', but argument 5 has type 'double'
m.c:21: warning: format '%d' expects type 'int', but argument 6 has type 'double'
m.c:22: warning: format '%d' expects type 'int', but argument 2 has type 'double'
m.c:22: warning: format '%d' expects type 'int', but argument 3 has type 'double'
m.c:22: warning: format '%d' expects type 'int', but argument 4 has type 'double'
m.c:22: warning: format '%d' expects type 'int', but argument 5 has type 'double'
m.c:22: warning: format '%d' expects type 'int', but argument 6 has type 'double'

m.c is your code in comment #7

> 
> 
> > (3) There is an expectation someone writing C might actually
> >     adhere to the Standard
> 
> From time to time that does not always occur. See here:
>http://gcc.gnu.org/bugzilla/buglist.cgi?emailreporter1=1&emailtype1=substring&email1=kargl@gcc.gnu.org

I'm not sure what you're trying to demonstate.  Bug reports associated with me
are related to Fortran not C.

>Andrew Pinski
>abs converts the float/double to an integer type so this is not a bug.


>Uros Bizjak
>The non-problem you are going after is in printf(). It takes variable arguments
>from the stack and interprets them according to the format string. Argument are
>pushed to the stack by the caller without any other communication with callee,
>so it is obvious that format string _must_ reflect the type of values on stack.
>Also note, that %f inherently converts float type to double, so your values on
>the stack are FUBAR as far as printf is concerned.


So what we end up with is something like this:

main(){
  printf("%f  %f  %f  %f\n", abs(1234.5678), fabs(1234.5678), abs((int)1234.5678), abs((int)(1234.5678)));
}

and here is the incorrect output:

639356954161190327651780598819979664641251888147947213308332781758079574424033219293612810237966089410023577192498425554519097800758249636139201808050627475876578945300496861358730667076856621948936720606078976728760320.000000  
0.000000  -0.007810  -0.000000


The first and third values are obviously wrong.

The second and fourth values agree - so (in theory) that is the "correct" answer - according to the way that GCC is intended to operate (as per what Andrew says).

If the "correct" answer is zero (without concerning ourselves if it is plus zero or minus zero) then why are the _third_ and _fourth_ answer different!

 abs((int)1234.5678)  = -0.007810
abs((int)(1234.5678)) = -0.000000

Should the cast work without the extra parentheses.

(In reply to comment #14)

> m.c: In function 'main':
> m.c:9: warning: implicit declaration of function 'abs'

Also, add a prototype for integer abs(), like "int abs(int);". Then everything will work as "expected".

(In reply to comment #15)

> and here is the incorrect output:

Here is the correct input:

--cut here--
#include <stdio.h>

int abs(int);
double fabs(double);

int main()
{
  printf("%i  %f  %i  %i\n", abs(1234.5678), fabs(1234.5678),
         abs((int)1234.5678), abs((int)(1234.5678)));

  return 0;
}
--cut here--

OK, have fun, I'll stop wasting electrons here...

(In reply to comment #16)
> (In reply to comment #14)
> > m.c: In function 'main':
> > m.c:9: warning: implicit declaration of function 'abs'
> Also, add a prototype for integer abs(), like "int abs(int);". Then everything
> will work as "expected".


> Rob
> ...why are the _third_ and _fourth_ answers different!
>Should the cast work without the extra parentheses.

#include <stdio.h>
int abs(int);
double fabs(double);
int main()
{
  printf("%f  %f  %f  %f\n", abs(1234.5678), fabs(1234.5678), 
         abs((int)1234.5678), abs((int)(1234.5678)));
  return 0;
}

639356810344963382486834444601597699265113107384544848778287582042881171854556760809962494834238640539694738043963701847575050181760552125660054155932857305858816958002965674142423095065121202781270370782793943786455040.000000  
0.000000  -0.422029  -0.000000


If I re-write and swap the 3rd and fourth abs'es I get:

#include <stdio.h>
int abs(int);
double fabs(double);
int main()
{
  printf("%f  %f  %f  %f\n", abs(1234.5678), fabs(1234.5678), 
         abs((int)(1234.5678)), abs((int)1234.5678));
  return 0;
}

639356810344963382486834444601597699265113107384544848778287582042881171854556760809962494834238640539694738043963701847575050181760552125660054155932857305858816958002965674142423095065121202781270370782793943786455040.000000  
0.000000  -1.333046  -0.000000

The same sort of output, so the parentheses are not at fault.

Printf does not cast it's "%i" / "%f" 's even with constants know at compile time, let alone run-time. It is like you said in comment 2 "Especially with %f."

So if you wanted to call functions by way of a pointer and they might return int or float (or double) you would need code to test the type prior to attempting to send it to multiple types of printf statement.

Using one printf for each possible combination with only 2 vars we would need 4 printf's. At least all the extra trouble buys us that it doesn't matter if they are const or var :( here goes (there is no need for "_someone_" to post that I'm missing a main() statement!):


...
switch (TYPE_TEST_FOR_2_VARS(x,y)) {
  case TTF2V_I_I:
    printf("X= %i  Y=%i\n", x, y);
    goto done;
  case TTF2V_I_F:
    printf("X= %i  Y=%f\n", x, y);
    goto done;
  case TTF2V_F_I:
    printf("X= %f  Y=%i\n", x, y);
    goto done;
  case TTF2V_F_F:
    printf("X= %f  Y=%f\n", x, y);
    goto done;
}
done:;
...


Same idea for 4 variables, just a lot more code ...

Gotcha, thanks Uros.

So here we have it:

#include <stdio.h>
int abs(int);
double fabs(double);
int main()
{
  printf("%f  %f  %f  %f\n", abs(1234.5678), fabs(1234.5678),
         abs((int)1234.5678), abs((int)(1234.5678)));
  printf("%f  %f  %f  %f\n", (float)abs(1234.5678), (float)fabs(1234.5678),
         (float)abs((int)1234.5678), (float)abs((int)(1234.5678)));
  return 0;
}


# /opt/gcc-4_3-build-2/gcc/xgcc -Wall -B/opt/gcc-4_3-build-2/gcc/ -o math_test_11 math_test_11.c
math_test_11.c: In function 'main':
math_test_11.c:9: warning: format '%f' expects type 'double', but argument 2 has type 'int'
math_test_11.c:9: warning: format '%f' expects type 'double', but argument 4 has type 'int'
math_test_11.c:9: warning: format '%f' expects type 'double', but argument 5 has type 'int'


# ./math_test_11
639356810344963382486834444601597699265113107384544848778287582042881171854556760809962494834238640539694738043963701847575050181760552125660054155932857305858816958002965674142423095065121202781270370782793943786455040.000000 
0.000000  -0.008386  -0.000000
1234.000000  1234.567749  1234.000000  1234.000000


Conclusion:

Printf can't figure out it's own casts (even for consts) and you _must_ use the cast (float) before any variables that are to be printed with format "%f".

Since GCC can 'see' the "%f" at compile time, _If_ gcc put in the "(float)" cast (fix all other types too) for you then the printf statement would produce the correct result and provide the missing piece to what Andrew claims in comment 4 "abs converts the float/double to an integer type".

A simple modification to GCC could read the format and cast the variable. This could be integrated with ssp to avoid reading and writing (sprintf()) to the stack. This does not seem like an expensive mod. 

> Since GCC can 'see' the "%f" at compile time
Yes it can but that does not mean the behavior is the correct to cast it to float.  In fact the behavior is undefined at runtime so you are just running into the runtime undefined behavior which is really normal with C/C++ now adays.  

(In reply to comment #20)
> > Since GCC can 'see' the "%f" at compile time
> ... that does not mean the behavior is the correct to cast it to float. ...

So the correct behavior is:

a): Cast it to something other than float.

 - if it can only be float (in the example, otherwise anything else) as far as 
   printf is concerned then why do that? I know you are NOT suggesting that we
   could cast it to (int) and use "%f" to print it and you are NOT suggesting
   that we could cast it to (char *), a string, and print it with "%f".

 - If the printf contains "%f" then the var "must" be a float.


Example:

  long double A = 123.456;
  int B = A;

Is the variable B an int, or something else. Why is it incorrect behaviour to print it with %f even if that means we get (a compiler warning when compiling and) 123.000 for output - would anyone prefer gibberish for output ?


b): do as I suggested and avoid the security issue, bad output, and the need for contorted code (as shown in comment #18).


c): The Emperor's clothes look wonderful.

Why is it a bad idea to leave this flaw in GCC ?

Format String Bugs and Exploits
http://www.geocities.com/ravecoolr/fmt.doc

or if you like:
http://www.enderunix.org/docs/formatstr.txt

Allowing GCC to stay as-is and permit someone to use a user supplied format string to print an integer opens a whole field of exploits that could be closed by fixing this.

There is a -Wformat for a reason, use it.