Re: More C type errors by default for GCC 14

[Florian Weimer <fw@deneb.enyo.de>]

* Joseph Myers:

> On Wed, 10 May 2023, Eli Zaretskii via Gcc wrote:
>
>> That is not the case we are discussing, AFAIU.  Or at least no one has
>> yet explained why accepting those old K&R programs will adversely
>> affect the ability of GCC to compile C2x programs.
>
> At block scope,
>
>   auto x = 1.5;
>
> declares x to have type double in C2x (C++-style auto), but type int in 
> C89 (and is invalid for versions in between).  In this case, there is an 
> incompatible semantic change between implicit int and C++-style auto.  
> Giving an error before we make -std=gnu2x the default seems like a 
> particularly good idea, to further alert anyone who has been ignoring the 
> warnings about implicit int that semantics will change incompatibly.

Obviously makes sense to me.

> In cases where the standard requires a diagnostic, some are errors, some 
> are pedwarns-by-default or unconditional pedwarns, some are 
> pedwarns-if-pedantic - the choice depending on how suspicious the 
> construct in question is and whether it corresponds to a meaningful 
> extension (this is not making an automatic choice for every such situation 
> in the standard, it's a case-by-case judgement by maintainers).  By now, 
> the cases discussed in this thread are sufficiently suspicious - 
> sufficiently likely to result in unintended execution at runtime (not, of 
> course, reliably detected because programs with such dodgy code are very 
> unlikely to have thorough automated tests covering all their code) - that 
> is it in the interests of users for them to be errors by default (for C99 
> and later modes, in the cases that were valid in C89).

Just to recap, those are controlled by
-Wimplicit-function-declaration, -Wimplicit-int, -Wint-conversion, and
-Wincompatible-pointer-types, roughly in increasing order of
compatibility impact with old sources.

> It might also make sense to review other pedwarns-by-default and 
> unconditional pedwarns to consider if any of those should be errors by 
> default, though I suspect most of those are less significant.

I went through the pedwarn calls in the C front end.

First, these two appear to be genuine bugs:

  Incompatible pointer types in ?: not covered by -Wincompatible-pointer-types
  <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109826>

  Pointer/integer mismatch in ?: not covered by -Wint-conversion
  <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109827

Maybe the latter more so than the former.  These create pointer
values, so they seem problematic.

There are related coverage gaps for comparison operators for the more
specific -W… options:

  pedwarn (location, 0, "comparison of distinct pointer types lacks a cast");
  pedwarn (location, OPT_Wpedantic, 
           "ordered comparison of pointer with integer zero");
  pedwarn (location, 0, "comparison between pointer and integer");

These expressions do not create pointer values, so maybe they are less
likely to introduce bugs?  But they are type errors in standard C.
(Clang has separate warnings for those, -Wcompare-distinct-pointer-types.)


Moving on, this seems to be a good candidate for an error:

  pedwarn (input_location, 0, "parameter names (without types) in "
           "function declaration");

The reason is that

  void f (uint3_t);

is a function declaration without a prototype, which is very likely
not what is intended.  The error diagnostic could also provide a
spelling hint for the type.


We may not need to do anything for this one (except removing cascading
errors) because I think this is only reachable when an implicit int is
involved:

  pedwarn (here, 0, "data definition has no type or storage class");


(Slightly unrelated because it's a purely syntactic issue.  It's about
designated initaliziers of the form “{ a[0] 1 }”:

  pedwarn (c_parser_peek_token (parser)->location, OPT_Wpedantic,
           "obsolete use of designated initializer without %<=%>");

Unclear whether we still need to support that, but also harmless, I
guess.)


This one seems to be hack to support obsolete wait function usage.  We
probably don't need it any more because the union wait was removed
from glibc 2.24, and the function prototypes in glibc are now more
standard.  The union wait type was deprecated in the early 90s.

  /* Given  wait (union {union wait *u; int *i} *)
     and  wait (union wait *),
     prefer  union wait *  as type of parm.  */
   pedwarn (input_location, OPT_Wpedantic,
            "function types not truly compatible in ISO C")

And furher below in c-typeck.cc:

  /* Allow  wait (union {union wait *u; int *i} *)
     and  wait (union wait *)  to be compatible.  */

I think it should be safe to error for these by default.


I couldn't figure out what these warnings are about:

  pedwarn (input_location, OPT_Wpedantic,
           "function types not truly compatible in ISO C");
  pedwarn (location, OPT_Wpedantic, "types are not quite compatible");


This seems to be mainly for &*p if p is of type void *:

  warning_at (loc, 0, "dereferencing %<void *%> pointer");
  pedwarn (location, 0, "taking address of expression of type %<void%>");

Rather hideous, but maybe harmless in the grand scheme of things?


One rather large set of pedwarns concerns qualifier mismatches.  I
believe this is about technically undefined behavior, so maybe the
warning is not about things that are entirely harmless.  Implicitly
dropping const-ness from pointers could cause crashes at a later
stage, so more rigorous errors could point at the location of the
actual mistake.  On the other hand, C programs aiming for full
const-correctness are quite rare.  Adding all those casts for things
like

  volatile int x[4];
  memset (x, 0, sizeof (x));

would be quite cumbersome, too.  So I'm not sure what to do about
these.

Then there is -Wpointer-sign.  The Linux kernel really dislikes that
warning, so we would want a mechanism to control that separately.
Again not sure what to do about those.


This sone seems to be a good candidate for additional errors, though:

  warned_here = pedwarn
    (loc, warn_return_type >= 0 ? OPT_Wreturn_type : 0,
     "%<return%> with no value, in function returning non-void");

It's a clear type volation that can lead to obscure bugs.  Maybe the
converse as well.


In summary, all these seems to be good candidates for errors by default:

* int-conversion as errors (already raised separately
* -Wint-conversion for ?:
* parameter names in non-prototype function declarations
* the union wait function pointer compatibility kludge
* return-with-out-value for non-void functions
* -Wincomatible-pointer-types warning for ?: (but no error yet, see below)

This are more “maybe“:

* incompatible-pointer-types as errors (already raised separately)
* int-conversion and incompatible-pointer-types in comparisons
* return with value in a function returning void
* dereferencing void *
* taking the address of void
* "function types not truly compatible in ISO C"
  and "types are not quite compatible" (depending on what they actually mean)
* qualifier mismatches (may need separate opt-out)
* sign mismatches in pointers (definitely needs separate opt-out)

I can do experiments on the Fedora code base in the coming weeks for
at least a subset of those.  The second list likely has a few with
quite high cost for us.  I suspect even incompatible-pointer-types is
problematic in that regard.  The first list seems more manageable,
although int-conversion is also a fairly big work item (and I need to
find a way to cut down the number of false positives from the tester).