egcs, RFC: patch for possible -Wmissing-noreturn warning

egcs, RFC: patch for possible -Wmissing-noreturn warning

[Kaveh R. Ghazi]

	I wanted to be able to automate finding functions which
could/should have the __noreturn__ attribute set.  So I thought about
where in the compiler it could go.  This is what I came up with.

--- egcs-CVS19981009/gcc/c-decl.c~	Fri Oct  9 23:57:25 1998
+++ egcs-CVS19981009/gcc/c-decl.c	Sat Oct 10 13:23:17 1998
@@ -7190,6 +7190,10 @@ finish_function (nested)
 
   current_function_returns_null |= can_reach_end;
 
+  if (!TREE_THIS_VOLATILE (fndecl)
+      && !current_function_returns_null && !current_function_returns_value)
+    warning ("function does not return, and is not declared `noreturn' ");
+
   if (TREE_THIS_VOLATILE (fndecl) && current_function_returns_null)
     warning ("`noreturn' function does return");
   else if (warn_return_type && can_reach_end


	As you can see, I plagiarized the warning below it and simply
reversed the condition checks.  :-) It seems to work pretty well.  I did
a bootstrap with it and found ~60 new places. 

	The problem is that I'm getting a small number of false
positives when `static inline void' functions are used.  I'm also
getting some hits when exit() is called from main().  Although
technically correct (its thus a function which doesn't `return',) I
think its one case which should not be flagged. 

	Let's skip the fact that this warning is not controlled by a
-W flag for now.  I wanted to get comments on whether this is the
correct approach, and help fixing the false positives.


	Some of the false positives I'm getting during bootstrap:

 > ./frame.c: In function `start_fde_sort':
 > ./frame.c:266: warning: function does not return, and is not
 > 	declared `noreturn'
 > ./frame.c: In function `fde_insert':
 > ./frame.c:272: warning: function does not return, and is not
 > 	declared `noreturn'
 > ./frame.c: In function `fde_split':
 > ./frame.c:303: warning: function does not return, and is not
 > 	declared `noreturn'
 > ./frame.c: In function `frame_heapsort':
 > ./frame.c:368: warning: function does not return, and is not
 > 	declared `noreturn'


	Looking at these functions, they clearly fall off their ends.  Eg:

 > static inline void
 > fde_insert (fde_accumulator *accu, fde *this_fde)
 > {
 >   accu->linear.array[accu->linear.count++] = this_fde;
 > }

	So why is the test flagging them? Is there another bit I need to
check besides:
current_function_returns_null && current_function_returns_value ??


Also if I have:

 > main()
 > {
 >   return 0;
 > }

all's well.


	However, if I have:

 > main()
 > {
 >   exit(0);
 > }

	I get:

 > foo.c: In function `main':
 > foo.c:4: warning: function does not return, and is not declared `noreturn'

Again, this is technically correct.  But should we offer a pass from
`main', or do we encourage people to use the `return', not `exit' style?

Comments?

		Thanks,
		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Icon CMT Corp.

Re: egcs, RFC: patch for possible -Wmissing-noreturn warning

[Martin Mares]

Hello,

> 	Looking at these functions, they clearly fall off their ends.  Eg:
> 
>  > static inline void
>  > fde_insert (fde_accumulator *accu, fde *this_fde)
>  > {
>  >   accu->linear.array[accu->linear.count++] = this_fde;
>  > }
> 
> 	So why is the test flagging them? Is there another bit I need to
> check besides:
> current_function_returns_null && current_function_returns_value ??

   It is not as simple as it might look on the first sight. It's easy to prove
there exists _no_ algorithm distinguishing functions that really return.

Examples:

  void x(void) { for(;;); }				Doesn't return
  void x(void) { int x; for(x=0; x<1000; x++); }	Does return
  void x(void) { float x; for(x=0; x<1e30; x++); }	Doesn't return

There are three possible cases:

  [1] Functions that contain no jumps. They always return and there is nothing
      to check.
  [2] Functions that always have a call to non-returning function before each
      exit point. They are guaranteed not to return and they can be warned about
      if they are not declared non-returning.
  [3] Functions that contain jumps and don't fall into the previous category.
      Since the Halting Problem is uncomputable, you cannot tell whether they
      return or not which makes warnings of this case impossible.

Anyway, the [2] case seems to be interesting.
 
				Have a nice fortnight
-- 
Martin `MJ' Mares   <mj@ucw.cz>   http://atrey.karlin.mff.cuni.cz/~mj/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
"Computers are useless.  They can only give you answers."  -- Pablo Picasso

Re: egcs, RFC: patch for possible -Wmissing-noreturn warning

[Jeffrey A Law]

  In message < 199810111726.NAA19753@caip.rutgers.edu >you write:
  > 	The problem is that I'm getting a small number of false
  > positives when `static inline void' functions are used.
I wonder if we actually perform enough analysis on static inlines to determine
can_reach_end in c-decl.c (which in turn determines
current_function_returns_null.

  > I'm also
  > getting some hits when exit() is called from main().  Although
  > technically correct (its thus a function which doesn't `return',) I
  > think its one case which should not be flagged. 

[ ... ]
  > 
  > Also if I have:
  > 
  >  > main()
  >  > {
  >  >   return 0;
  >  > }
  > 
  > all's well.
  > 
  > 
  > 	However, if I have:
  > 
  >  > main()
  >  > {
  >  >   exit(0);
  >  > }
  > 
  > 	I get:
  > 
  >  > foo.c: In function `main':
  >  > foo.c:4: warning: function does not return, and is not declared `noreturn'
  > 
  > Again, this is technically correct.  But should we offer a pass from
  > `main', or do we encourage people to use the `return', not `exit' style?
Not sure.  I hate making special exceptions like this, but given that we've
already made several for "main" I guess another isn't so bad.

jeff

Re: egcs, RFC: patch for possible -Wmissing-noreturn warning

[Jeffrey A Law]

  In message < 19981012095243.07133@albireo.ucw.cz >you write:
  >    It is not as simple as it might look on the first sight. It's easy to
  > prove there exists _no_ algorithm distinguishing functions that really return.
True, but if it is the compilation of the function itself (not the copy that
gets integrated into callers), then for this example we should be able to
determine that each and every path through the (trivial) function exits.

The other question is for a static inline function, we may halt the compilation
too early and as a result the things Kaveh wants to check hevn't been
initialized.

I'm well aware of the halting problem :-)

jeff

Re: egcs, RFC: patch for possible -Wmissing-noreturn warning

[John Carr]

> There are three possible cases:
> ...
>   [3] Functions that contain jumps and don't fall into the previous category.
>       Since the Halting Problem is uncomputable, you cannot tell whether they
>       return or not which makes warnings of this case impossible.

The halting problem says that given an infinite system you can't write
a perfect theorem prover.  It doesn't say that you can't get most
cases right.  (It also doesn't apply to finite systems where it is
possible to enumerate all the possible states, but that's not an
important exception here.)

gcc needs to be careful with return/noreturn warnings because there
are three states, not two (like IEEE compares), but it should be
possible to arrange that:

1. every return/noreturn warning is correct
2. most functions which should have warnings get them

Re: egcs, RFC: patch for possible -Wmissing-noreturn warning

[Kaveh R. Ghazi]

 > From: Joe Buck <jbuck@Synopsys.COM>
 > 
 > [ determining whether a function will return ]
 > 
 > > 	Yes, but IMHO, this isn't the halting problem.  Its not
 > > whether it *will* return but whether its *possible* to return.  Ie,
 > > what are the exit points of the function?  Does it `return', fall off
 > > its end or call abort/exit/longjmp/{some other `noreturn' function}?
 > 
 > If we require an exact solution (given any function, can it return?), then
 > it is equivalent to the halting problem.  But we don't require an exact
 > solution (if we can't tell whether the function returns or not, we assume
 > that it returns).

	Exactly my point.  If your statement is true, (that in the
undetermined state we assume it *returns*) then gcc is giving false
information. 

	A one line static inline function that simply does one
assignment is being flagged as *not* returning.  Jeff has commented that
gcc may be prematurely short circuiting this calculation in the case of
static inlines.  I agree. 

		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Icon CMT Corp.

Re: egcs, RFC: patch for possible -Wmissing-noreturn warning

[Joe Buck]

[ determining whether a function will return ]

> 	Yes, but IMHO, this isn't the halting problem.  Its not
> whether it *will* return but whether its *possible* to return.  Ie,
> what are the exit points of the function?  Does it `return', fall off
> its end or call abort/exit/longjmp/{some other `noreturn' function}?

If we require an exact solution (given any function, can it return?), then
it is equivalent to the halting problem.  But we don't require an exact
solution (if we can't tell whether the function returns or not, we assume
that it returns).

Re: egcs, RFC: patch for possible -Wmissing-noreturn warning

[Kaveh R. Ghazi]

 > From: Martin Mares <mj@ucw.cz>
 > 
 > Hello,
 > 
 > > 	Looking at these functions, they clearly fall off their ends.  Eg:
 > > 
 > >  > static inline void
 > >  > fde_insert (fde_accumulator *accu, fde *this_fde)
 > >  > {
 > >  >   accu->linear.array[accu->linear.count++] = this_fde;
 > >  > }
 > > 
 > > 	So why is the test flagging them? Is there another bit I need to
 > > check besides:
 > > current_function_returns_null && current_function_returns_value ??
 > 
 > It is not as simple as it might look on the first sight. It's easy
 > to prove there exists _no_ algorithm distinguishing functions that
 > really return.
 > 
 > [...]
 > 
 > There are three possible cases:
 > 
 > [1] Functions that contain no jumps. They always return and there
 > is nothing to check.
 > -- 
 > Martin `MJ' Mares   <mj@ucw.cz>   http://atrey.karlin.mff.cuni.cz/~mj/

	Yes, but IMHO, this isn't the halting problem.  Its not
whether it *will* return but whether its *possible* to return.  Ie,
what are the exit points of the function?  Does it `return', fall off
its end or call abort/exit/longjmp/{some other `noreturn' function}?

	If its only the first two, or some combination of all three,
its *possible* to return.  If its only the third case, then its a
`noreturn' function.

	Gcc has code to detect this stuff and its failing in the above
case which is a one line assignment statement.  Clearly this static
inline function returns.  So I'm convinced its either a bug or I'm not
checking all the appropriate bits. 

		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Icon CMT Corp.