[Bug c/35579] New: __attribute__(( warn_unused_result )) warns when it shouldn't, doesn't warn when it should

[Bug c/35579] New: __attribute__(( warn_unused_result )) warns when it shouldn't, doesn't warn when it should

[pgut001 at cs dot auckland dot ac dot nz]

The handling of warn_unused_result is rather inconsistent, it warns in cases
where it shouldn't and sometimes doesn't warn in cases where it should.  Try
the following to see this:

-- Snip --

> cat > test.c
#include <stdlib.h>

__attribute__(( warn_unused_result )) int foo( void )
  {
  return 1;
  }

int main( int argc, char *argv[] )
  {
  int result;

  result = foo();

  result = foo();
  if( !result )
    exit( EXIT_FAILURE );

  ( void ) foo();      // Line 18

  return EXIT_SUCCESS;
  }
^D

>gcc -Wall test.c
test.c: In function 'main':
test.c:18: warning: ignoring return value of 'foo', declared with attribute
warn_unused_result

-- Snip --

The result from the first call to foo() is never used, but gcc doesn't warn
about it.  On the other hand the result from the third call to foo() (line 18)
is explicitly discarded but this time gcc does warn about it.  Just to provide
more detail on the gcc version:

-- Snip --

>gcc --ver
Using built-in specs.
Target: powerpc-linux-gnu
Configured with: ../src/configure -v
--enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr
--enable-shared --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix --enable-nls
--program-suffix=-4.1 --enable-__cxa_atexit --enable-clocale=gnu
--enable-libstdcxx-debug --enable-mpfr --disable-softfloat --enable-secureplt
--enable-targets=powerpc-linux,powerpc64-linux --with-cpu=default32
--with-long-double-128 --enable-checking=release powerpc-linux-gnu
Thread model: posix
gcc version 4.1.2 (Ubuntu 4.1.2-0ubuntu4)

-- Snip --

I've seen the same behaviour on one of the 4.2 versions as well so this doesn't
seem to be specific to 4.1.2, that's just the one that happens to be on the
machine I'm currently using.


-- 
           Summary: __attribute__(( warn_unused_result )) warns when it
                    shouldn't, doesn't warn when it should
           Product: gcc
           Version: 4.1.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: pgut001 at cs dot auckland dot ac dot nz


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35579

[Bug c/35579] __attribute__(( warn_unused_result )) warns when it shouldn't, doesn't warn when it should

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2008-03-14 04:54 -------
This is the design of warn_unused_result, you cannot ignore the value, that is
why casting to void does not work.

The warning happens in the front-end so we don't know if you really ignore the
value if you don't use the actually variable.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35579

[Bug c/35579] __attribute__(( warn_unused_result )) warns when it shouldn't, doesn't warn when it should

[pgut001 at cs dot auckland dot ac dot nz]

------- Comment #2 from pgut001 at cs dot auckland dot ac dot nz  2008-03-14 06:55 -------
>This is the design of warn_unused_result, you cannot ignore the value, 
>that is why casting to void does not work.

Doesn't this violate a C convention going back to the 1970s?  I did a fairly
long web/newsgroup search before filing this bug report and the advice given
was always to use the void cast, a practice going back to (at least) lint in
the late 1970s to get rid of its "function returns value which is always
ignored" warning, so the expectation from users seems to be that a void cast
will resolve this.

>The warning happens in the front-end so we don't know if you really ignore the
>value if you don't use the actually variable.

Is there any way of fixing this?  warn_unused_result is used in a number of
quite significant pieces of code to enforce safety properties (the Linux kernel
  springs to mind), having a compiler-enforced safety check that doesn't work
reliably seems rather dangerous.  If it can't be fixed then could it at least
be documented that the check isn't always reliable?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35579

[Bug c/35579] __attribute__(( warn_unused_result )) warns when it shouldn't, doesn't warn when it should

[manu at gcc dot gnu dot org]

------- Comment #3 from manu at gcc dot gnu dot org  2008-03-14 09:36 -------
(In reply to comment #2)
> >This is the design of warn_unused_result, you cannot ignore the value, 
> >that is why casting to void does not work.

I agree with the reporter. There should be a way to tell the compiler to avoid
warning for particular cases. Casting to void seems to be the best way to do
that.

> >The warning happens in the front-end so we don't know if you really ignore the
> >value if you don't use the actually variable.
> 
> Is there any way of fixing this?  warn_unused_result is used in a number of
> quite significant pieces of code to enforce safety properties (the Linux kernel
>   springs to mind), having a compiler-enforced safety check that doesn't work
> reliably seems rather dangerous.  If it can't be fixed then could it at least
> be documented that the check isn't always reliable?

Unfortunately, the front-end lacks any data-flow knowledge, so this can't be
fixed. Almost all GCC warnings have the same limitations.

Documenting that warn_unused_result does not detect certain cases seems good.
Care to write a text? I will format it and submit it as a patch if you want.

Thanks.


-- 

manu at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu dot org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-03-14 09:36:05
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35579

[Bug c/35579] __attribute__(( warn_unused_result )) warns when it shouldn't, doesn't warn when it should

[pinskia at gcc dot gnu dot org]

------- Comment #4 from pinskia at gcc dot gnu dot org  2008-03-14 12:08 -------
(In reply to comment #3)
> (In reply to comment #2)
> > >This is the design of warn_unused_result, you cannot ignore the value, 
> > >that is why casting to void does not work.
> 
> I agree with the reporter. There should be a way to tell the compiler to avoid
> warning for particular cases. Casting to void seems to be the best way to do
> that.

http://gcc.gnu.org/ml/gcc/2006-11/msg00468.html

The original patch has a testcase explicitly checking for the case where cast
to void is there:
http://gcc.gnu.org/ml/gcc-patches/2003-09/msg00875.html
+  (void) check1 ();    /* { dg-warning "ignoring return value of" } */

As for the other stuff, the manual says:

+The @code{warn_unused_result} attribute causes a warning to be emitted
+if a caller of the function with this attribute does not use its
+return value.  This is useful for functions where not checking
+the result is either a security problem or always a bug, such as
+@code{realloc}.

The return value is used, it is assigned to a variable, just that variable is
not used which is a different story.

I think we can declare this bug as invalid really.

-- Pinski


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35579

[Bug c/35579] __attribute__(( warn_unused_result )) warns when it shouldn't, doesn't warn when it should

[pgut001 at cs dot auckland dot ac dot nz]

------- Comment #5 from pgut001 at cs dot auckland dot ac dot nz  2008-03-15 09:09 -------
>Care to write a text? I will format it and submit it as a patch if you want.

How about the following, using as a starting point the latest docs (4.3.0),
section 5.27, 'warn_unused_result', which currently ends with:

"...results in warning on line 5.".

Perhaps add to this:

-- Snip --

Note that the warnings are generated by the compiler front-end before any
data-flow analysis is performed, so situations in which the return value is
assigned to a variable but the resulting variable is ignored will not be
detected.  Changing line 5 in the above code to:

  int value = fn();

would result in no warning being emitted even though the return value isn't
used.

-- Snip --

(You could get really pedantic with the wording and say that the caller should
"act on the return value of the function" rather than the somewhat ambiguous
"used", but I think the above should get the meaning across). 

Oh, I also have a feeling that "results in warning on line 5." in the current
docs should really be "results in *a* warning on line 5." :-).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35579

[Bug c/35579] [4.1/4.2/4.3/4.4 Regression] casts to void do not silence -Wunused warnings for the case of __attribute__(( warn_unused_result ))

[bonzini at gnu dot org]

------- Comment #6 from bonzini at gnu dot org  2008-10-17 09:14 -------
It seems bad that *explicit* casts to void cannot silence the warning.  This is
a problem for projects using -Werror and using (void) with due diligence
(including GNU coreutils).


-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.1.2 4.2.4 4.3.2 4.4.0
      Known to work|                            |4.0.3
            Summary|__attribute__((             |[4.1/4.2/4.3/4.4 Regression]
                   |warn_unused_result )) warns |casts to void do not silence
                   |when it shouldn't, doesn't  |-Wunused warnings for the
                   |warn when it should         |case of __attribute__((
                   |                            |warn_unused_result ))


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35579

[Bug c/35579] [4.1/4.2/4.3/4.4 Regression] casts to void do not silence -Wunused warnings for the case of __attribute__(( warn_unused_result ))

[bonzini at gnu dot org]

------- Comment #7 from bonzini at gnu dot org  2008-10-17 11:12 -------
Totally untested patch.

Index: c-common.c
===================================================================
--- c-common.c  (revisione 134435)
+++ c-common.c  (copia locale)
@@ -6762,7 +6762,8 @@ c_warn_unused_result (tree *top_p)
          ftype = TREE_TYPE (ftype);
        }

-      if (lookup_attribute ("warn_unused_result", TYPE_ATTRIBUTES (ftype)))
+      if (lookup_attribute ("warn_unused_result", TYPE_ATTRIBUTES (ftype))
+         && !TREE_NO_WARNING (t))
        {
          if (fdecl)
            warning (0, "%Hignoring return value of %qD, "
Index: c-convert.c
===================================================================
--- c-convert.c (revisione 134435)
+++ c-convert.c (copia locale)
@@ -96,7 +96,11 @@ convert (tree type, tree expr)
       return error_mark_node;
     }
   if (code == VOID_TYPE)
-    return fold_convert (type, e);
+    {
+      if (TREE_CODE (e) == CALL_EXPR)
+       TREE_NO_WARNING (e) == 1;
+      return fold_convert (type, e);
+    }
   if (code == INTEGER_TYPE || code == ENUMERAL_TYPE)
     return fold (convert_to_integer (type, e));
   if (code == BOOLEAN_TYPE)
Index: cp/cvt.c
===================================================================
--- cp/cvt.c    (revisione 134435)
+++ cp/cvt.c    (copia locale)
@@ -825,6 +825,7 @@ convert_to_void (tree expr, const char *
       break;

     case CALL_EXPR:   /* We have a special meaning for volatile void fn().  */
+      TREE_NO_WARNING (expr) = 1;
       break;

     case INDIRECT_REF:


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35579

[Bug c/35579] [4.1/4.2/4.3/4.4 Regression] casts to void do not silence -Wunused warnings for the case of __attribute__(( warn_unused_result ))

[joseph at codesourcery dot com]

------- Comment #8 from joseph at codesourcery dot com  2008-10-17 11:22 -------
Subject: Re:  [4.1/4.2/4.3/4.4 Regression] casts to void do not
 silence -Wunused warnings for the case of __attribute__(( warn_unused_result
 ))

On Fri, 17 Oct 2008, bonzini at gnu dot org wrote:

> It seems bad that *explicit* casts to void cannot silence the warning.  This is
> a problem for projects using -Werror and using (void) with due diligence
> (including GNU coreutils).

The maintainers of a library using this attribute in its headers have 
decided that casting the return value of the relevant functions to (void) 
is an incorrect use of those functions.  The testsuite duly checks for 
these semantics.  If you disagree with a library maintainer's decision, 
you should bring that up with the library maintainers.  This was discussed 
at length in bug 25509.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35579

[Bug c/35579] false negatives in warn_unused_result

[bonzini at gnu dot org]

------- Comment #9 from bonzini at gnu dot org  2008-10-17 11:37 -------
So I should reopen bug 25509 and leave this one for the false negatives.


-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.1/4.2/4.3/4.4 Regression]|false negatives in
                   |casts to void do not silence|warn_unused_result
                   |-Wunused warnings for the   |
                   |case of __attribute__((     |
                   |warn_unused_result ))       |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35579