Undefined static functions

Undefined static functions

[Joseph S. Myers]

C requires a diagnostic if a static function is used but not defined in a 
translation unit.  The present implementation of this is a pedwarn in 
check_global_declarations in toplev.c.

(a) Which language front ends use this pedwarn, rather than avoiding 
getting that far with a translation unit using an undefined static 
function?

(b) Is there any reason this shouldn't be a hard error rather than just a 
pedwarn?

(c) This diagnostic is broken for C with -funit-at-a-time (implied by 
-O2), ": Search converges between 2004-07-08-trunk (#482) and 
2004-07-09-trunk (#483).".  Test (should be diagnosed, now isn't at -O2):

static void f0(void);
void g0(void) { f0(); }

I would guess TREE_SYMBOL_REFERENCED is being tested too early for 
unit-at-a-time mode; certainly it's the wrong way to implement this 
diagnostic.

(d) It also has always been broken with regard to calls that are optimised 
away; C90 and C99 are explicit that it is any use outside sizeof (sizeof 
whose result is an integer constant, in C99) that is erroneous.  For 
example,

static void f1(void);
void g1(void) { if (0) { f1(); } }

should be diagnosed.  Is there any reason this shouldn't be a hard error 
as well?

I propose to fix this C90 issue (which was missing from my original C90 
project proposal) in the front end, and there is nothing tricky about so 
doing (flag all used static function decls, ignoring uses inside 
__alignof__ and keeping lists inside sizeof/typeof so they can be flagged 
afterwards if the result turns out nonconstant).  It would however be 
convenient to be able to get rid of the checks in toplev.c (or make them 
internal_error if no errors have been given by the front end) by having 
all front ends give proper errors, to avoid the need to do anything 
special to avoid duplicate error messages.

(e) Consider the following C++ variation on the above program:

inline static void f1(void);
void g1(void) { if (0) { f1(); } }

Does "no diagnostic required" in C++03 [basic.def.odr] paragraph 3 apply 
to the whole paragraph, or only to the sentence it appears in?  That is, 
is "An inline function shall be defined in every translation unit in which 
it is used." a diagnosable rule?  If so, this program should be diagnosed 
by the C++ compiler (and the existing diagnostic for inline functions used 
but not defined should be a pedwarn or error not a warning).

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
  http://www.srcf.ucam.org/~jsm28/gcc/#c90status - status of C90 for GCC 3.5
    jsm@polyomino.org.uk (personal mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)

Re: Undefined static functions

[Richard Henderson]

On Tue, Aug 31, 2004 at 12:11:20AM +0000, Joseph S. Myers wrote:
> (b) Is there any reason this shouldn't be a hard error rather than just a 
> pedwarn?

The function implementation is contained within a file-scope asm.  I need
to tell the compiler that the symbol is locally defined (static as opposed
to extern), so that the proper set of relocations are selected.

I have used this often in kernel and libc code.



r~

Re: Undefined static functions

[Joseph S. Myers]

On Mon, 30 Aug 2004, Richard Henderson wrote:

> On Tue, Aug 31, 2004 at 12:11:20AM +0000, Joseph S. Myers wrote:
> > (b) Is there any reason this shouldn't be a hard error rather than just a 
> > pedwarn?
> 
> The function implementation is contained within a file-scope asm.  I need
> to tell the compiler that the symbol is locally defined (static as opposed
> to extern), so that the proper set of relocations are selected.
> 
> I have used this often in kernel and libc code.

This does rather make me wonder whether what check_global_declarations 
does after warning,

          /* This symbol is effectively an "extern" declaration now.  */
          TREE_PUBLIC (decl) = 1;

is particularly safe.  Certainly it looks like setting TREE_PUBLIC after 
pedwarning in the front end wouldn't be a particularly good way to stop 
check_global_declarations from warning - defining TREE_NO_WARNING to apply 
to decls as well as expressions looks safer.

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
  http://www.srcf.ucam.org/~jsm28/gcc/#c90status - status of C90 for GCC 3.5
    jsm@polyomino.org.uk (personal mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)

Re: Undefined static functions

[Richard Henderson]

On Tue, Aug 31, 2004 at 08:31:09AM +0000, Joseph S. Myers wrote:
> This does rather make me wonder whether what check_global_declarations 
> does after warning,
> 
>           /* This symbol is effectively an "extern" declaration now.  */
>           TREE_PUBLIC (decl) = 1;
> 
> is particularly safe.

Doesn't sound like it.  I guess the reason it works is that 
encode_section_info has already done its job, and we don't
call back into it now.

> ... defining TREE_NO_WARNING to apply 
> to decls as well as expressions looks safer.

TREE_NO_WARNING *does* apply to decls.  See warn_uninit in tree-ssa.c.


r~

Re: Undefined static functions

[Joseph S. Myers]

On Tue, 31 Aug 2004, Richard Henderson wrote:

> > ... defining TREE_NO_WARNING to apply 
> > to decls as well as expressions looks safer.
> 
> TREE_NO_WARNING *does* apply to decls.  See warn_uninit in tree-ssa.c.

So the documentation in tree.h of

   nowarning_flag:

       TREE_NO_WARNING in
           ... any expr node

and

/* In an expr node (usually a conversion) this means the node was made
   implicitly and should not lead to any sort of warning.  */

needs updating to reflect this.

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
  http://www.srcf.ucam.org/~jsm28/gcc/#c90status - status of C90 for GCC 3.5
    jsm@polyomino.org.uk (personal mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)

Re: Undefined static functions

[Richard Henderson]

Done.


r~

Re: Undefined static functions

[Mark Mitchell]

Joseph S. Myers wrote:

>C requires a diagnostic if a static function is used but not defined in a 
>translation unit.  The present implementation of this is a pedwarn in 
>check_global_declarations in toplev.c.
>
>(a) Which language front ends use this pedwarn, rather than avoiding 
>getting that far with a translation unit using an undefined static 
>function?
>
>(b) Is there any reason this shouldn't be a hard error rather than just a 
>pedwarn?
>
>(c) This diagnostic is broken for C with -funit-at-a-time (implied by 
>-O2), ": Search converges between 2004-07-08-trunk (#482) and 
>2004-07-09-trunk (#483).".  Test (should be diagnosed, now isn't at -O2):
>
>static void f0(void);
>void g0(void) { f0(); }
>
>I would guess TREE_SYMBOL_REFERENCED is being tested too early for 
>unit-at-a-time mode; certainly it's the wrong way to implement this 
>diagnostic.
>
>(d) It also has always been broken with regard to calls that are optimised 
>away; C90 and C99 are explicit that it is any use outside sizeof (sizeof 
>whose result is an integer constant, in C99) that is erroneous.  For 
>example,
>
>static void f1(void);
>void g1(void) { if (0) { f1(); } }
>
>should be diagnosed.  Is there any reason this shouldn't be a hard error 
>as well?
>
>I propose to fix this C90 issue (which was missing from my original C90 
>project proposal) in the front end, and there is nothing tricky about so 
>doing (flag all used static function decls, ignoring uses inside 
>__alignof__ and keeping lists inside sizeof/typeof so they can be flagged 
>afterwards if the result turns out nonconstant).  It would however be 
>convenient to be able to get rid of the checks in toplev.c (or make them 
>internal_error if no errors have been given by the front end) by having 
>all front ends give proper errors, to avoid the need to do anything 
>special to avoid duplicate error messages.
>
>(e) Consider the following C++ variation on the above program:
>
>inline static void f1(void);
>void g1(void) { if (0) { f1(); } }
>
>Does "no diagnostic required" in C++03 [basic.def.odr] paragraph 3 apply 
>to the whole paragraph, or only to the sentence it appears in?  That is, 
>is "An inline function shall be defined in every translation unit in which 
>it is used." a diagnosable rule?  
>
Yes.  (And there's nothing wrong with issuing diagnostics that are not 
strictly required.)

>If so, this program should be diagnosed 
>by the C++ compiler (and the existing diagnostic for inline functions used 
>but not defined should be a pedwarn or error not a warning).
>  
>
I agree.

-- 
Mark Mitchell
CodeSourcery, LLC
(916) 791-8304
mark@codesourcery.com

Re: Undefined static functions

[Joseph S. Myers]

On Tue, 31 Aug 2004, Mark Mitchell wrote:

> > (e) Consider the following C++ variation on the above program:
> > 
> > inline static void f1(void);
> > void g1(void) { if (0) { f1(); } }
> > 
> > Does "no diagnostic required" in C++03 [basic.def.odr] paragraph 3 apply to
> > the whole paragraph, or only to the sentence it appears in?  That is, is "An
> > inline function shall be defined in every translation unit in which it is
> > used." a diagnosable rule?  
> Yes.  (And there's nothing wrong with issuing diagnostics that are not
> strictly required.)

Thanks - I've filed bug 17256 to record this issue as I wouldn't expect to 
be looking at fixing it myself in the near future.

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
  http://www.srcf.ucam.org/~jsm28/gcc/#c90status - status of C90 for GCC 3.5
    jsm@polyomino.org.uk (personal mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)