[RFC] Enabling -Wstrict-prototypes by default in C

Clang Frontend
1

I would like to enable -Wstrict-prototypes by default in all C language modes, and I would like to surface it as a warning which defaults to an error instead of a typical warning.

Background

C has the notion of functions with prototypes and functions without prototypes. A function with a prototype is one which specifies its parameter type list. A function without a prototype either uses an identifier list or empty parentheses. e.g.,

void func1(int a, int b); // Prototype
void func2(a, b) int a, b; {} // No prototype
void func3(); // No prototype, can be called with any number and type of arguments

Functions without prototypes have never been a recommended practice in any standard version of C. In fact, they were introduced into C89 as already being deprecated (see C89 3.9.4 and 3.9.5). After a 35+ year deprecation period, C2x will be removing support for functions with identifier lists (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2432.pdf) and will be changing the behavior of prototypeless functions with empty parentheses to match the behavior of C++ (N 2841: No function declarators without prototypes).

void f1(a, b) int a, b; {} // No longer syntactically valid in C2x
void f2(); // Changes meaning in C2x to be void f2(void);
void use(void) {
  f2(1, 2); // Changes meaning in C2x, no longer valid
}

Justification

I would like to be aggressive about users migrating their code to functions with prototypes for several reasons:

  • There are substantial changes of behavior that users should be alerted to. void f(); goes from accepting any number and type of arguments at the call site to accepting exactly zero arguments at the call site; certain syntactic constructs are now invalid

  • Functions with prototypes are more secure than functions without prototypes due to the extra type information and type checking the prototype enables (EXP37-C. Call functions with the correct number and type of arguments - SEI CERT C Coding Standard - Confluence, CVE - CVE-2006-1174, MISRA C:2004, 8.1 - Functions shall have prototype declarations and the prototype shall be visible at both the function definition and call).

  • Ultimately, I want to remove FunctionNoPrototype from Clang. This type adds maintenance costs that are not born out from the functionality provided. To be clear, I donā€™t expect to remove FunctionNoPrototype anytime soon, this is more of a 5-10 year plan.

By enabling the diagnostic as a warning which defaults to an error, I think we can take the aggressive stance while still giving users a reasonable migration path. Users who want to know about the prototype issues in their code base but not able to fix them all yet can use -Wno-error=strict-prototypes to downgrade the error back into a warning. Users who have no interest in updating their code base case use -Wno-strict-prototypes to disable the diagnostic entirely.

I have taken steps to prepare the clang and clang-tools-extra test suites to enable this diagnostic by default, which involved changing around 3000 test files. The vast majority of those changes were to give a function with an empty parameter list an explicit parameter of (void) because the lack of a prototype is not whatā€™s being tested. The rest of the changes were to add -Wno-strict-prototypes to test files that were incidentally testing functions without prototypes (e.g., when testing attribute functionality, testing whether the attribute applies to a K&R C function the same as a function with a prototype is ā€œincidentalā€). This leaves roughly 60 test files that are almost exclusively testing functions without prototypes. My intention is to downgrade the error diagnostic into a warning for the majority of these test files (and perhaps add #if __STDC_VERSION__ guards around the code blocks that wonā€™t work in C2x mode any longer) and add new expected warning flags where appropriate.

Fallout

I would like to hear folksā€™ thoughts on this. As a WG14 representative for the Clang community, I donā€™t believe itā€™s reasonable to continue to leave -Wstrict-prototypes disabled by default. However, I still expect there to be fallout from enabling this diagnostic even as a warning. By failing to warn users about this for so long, thereā€™s likely to be a fair amount of accidental uses of functions without prototypes in the wild that this will catch. The issues we had in our own test suite are likely to also hit downstream folks with their test suites, for instance. However, I do not think this is a case where we want to allow ā€œthis is disruptive but I can migrate my codeā€ hold us back from making the change. The situations I think should hold us back from surfacing this as an error are more along the lines of ā€œthere is no way to migrate my code to use a prototype because <specific reasons> and I need more time/features to solve that.ā€

7 Likes
2

I agree with your reasoning, especially given the future direction of C2x aligning with C++ on empty function prototypes.

However, I wonder if weā€™re asking people to churn their code too much. As you say, the vast majority of code changes required by this warning are to change () to (void). Should we just give people a language feature flag to just say, ā€œoh yeah, I never wanted prototype-less functions, please opt me into the C2x and C++ behavior now with no other semantic changesā€. This behavior is consistent with C++ and will be the default from C in the future, so in the end it may save folks time and effort. I imagine there are people out there building legacy packages from source who can modify the build system more easily than they can modify the source. People may also prefer empty parentheses for aesthetic reasons, and would prefer to pre-deploy the new semantics. I suggest -fc2x-prototypes or -fstrict-prototypes as possible flag names. What do you think?

The warning diagnostic for empty parameter lists can direct people to try one of three things: add void (with a fixit), enable -std=c2x, or enable -fstrict-prototypes. These are all semantic changes, but we can be pretty confident that itā€™s what users want. The warning for untyped parameter lists would suggest adding types (int, since thatā€™s the default? hard to say), of course.

8 Likes
3

Should we just give people a language feature flag to just say, ā€œoh yeah, I never wanted prototype-less functions, please opt me into the C2x and C++ behavior now with no other semantic changesā€.

Thatā€™s a pretty neat idea, I think I like it. So the idea there is to treat all () function parameter lists as being (void) and if that causes a compile error with a subsequent definition, so be it? e.g.,

// Compiled with -fstrict-prototypes
void f(); // Treated as void f(void)
void f(a, b) int a, b; {} // Compile error due to conflicting types

void g(); // Treated as void g(void)
void g() {} // Works just fine, treated as void g(void) {}

void func(void) {
  extern void h(); // Treated as extern void h(void);
  h(1, 2, 3); // Compile error due to too many arguments
}

The warning diagnostic for empty parameter lists can direct people to try one of three things:

My plan on the diagnostic is not to focus on the solution, but focus on the problem. The current wording Iā€™ve been using is ā€œa function declaration without a prototype is deprecated in all versions of Cā€ and then we attach the fix-it to a note when we find something we think can potentially be modified (so that the fix-it isnā€™t automatically applied due to the potential for changing semantics).

The warning for untyped parameter lists would suggest adding types (int, since thatā€™s the default? hard to say), of course.

This situation is more challenging because int isnā€™t necessarily the type used by the arguments. e.g., Compiler Explorer

4

Iā€™m concerned this is going to cause a bunch of churn for people using ancient C codebases in benchmarks, where the code is basically frozen in the same state it was many years ago. Maybe we can keep the warning disabled by default for anyone explicitly using -ansi/-std=c89?

5

Maybe we can keep the warning disabled by default for anyone explicitly using -ansi/-std=c89?

I had considered this idea and it may still be worth thinking about. Iā€™d be opposed to silencing the diagnostic in -std=c89 mode because the functionality is deprecated in C89 same as any other version of ISO C. However, -ansi is a potentially viable option. Weā€™ve never distinguished between -ansi and -std=c89 before and I wasnā€™t certain whether it was worth it for this case or not. https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L5660 is where the driver happily converts -ansi into a specific language standard, so weā€™d need to thread some sort of option through to -cc1 invocations.

6

However, I wonder if weā€™re asking people to churn their code too much. As you say, the vast majority of code changes required by this warning are to change () to (void). Should we just give people a language feature flag to just say, ā€œoh yeah, I never wanted prototype-less functions, please opt me into the C2x and C++ behavior now with no other semantic changesā€. This behavior is consistent with C++ and will be the default from C in the future, so in the end it may save folks time and effort. I imagine there are people out there building legacy packages from source who can modify the build system more easily than they can modify the source. People may also prefer empty parentheses for aesthetic reasons, and would prefer to pre-deploy the new semantics. I suggest -fc2x-prototypes or -fstrict-prototypes as possible flag names. What do you think?

One of the things Iā€™ve noticed when talking to a lot of general programmers is that thereā€™s not a lot of awareness of the weird corner cases of the C language. I suspect that something like 80% of C programmers would be surprised that -fstrict-prototypes wasnā€™t how C already worked, especially programmers who are only dabbling in C. If thatā€™s the case, then it may ultimately be less disruptive to just make -fstrict-prototypes the default, and fix the (hopefully relatively few) remaining software that actually uses this deprecated feature to add in another command-line flag to keep using it, especially if we can also get gcc to go along with a standard command-line flag for this.

Is there any way we could do an experiment like building all of Debian with a so-modified clang and see how much stuff actually breaks?

2 Likes
7

From experience, a lot of people incorrectly assume that behavior, and having a way for their expectations to be matched would be great, i really like that direction.

Agreed, an automatic fix it here would terrify me.

Maybe a warning in C89 and an error after C99/C11?
The logic being that people on C89 are less likely to migrate to C2x anytime soon and maybe they consider prototypes a features.
On the other hand, by treating C89 differently, we may discourage upgrades even more which we should probably not.

8

Agreed, an automatic fix it here would terrify me.

ā€œFixItā€ still results in an error, its just a feature of our diagnostics engine that prints the ā€˜expectedā€™ thing and makes the compiler pretend that it was like that before.

Maybe a warning in C89 and an error after C99/C11?

This seems like a sensible split to me. Anyone who explicitly chooses C89 is probably never going to upgrade.

That said, this is something we should have been default-warning on 30 years ago. This is something that should have been an error 20 years ago. We are now at a point where we are only a few years away from someone switching compilers and having their codeā€™s meaning change subtly.

We should make this as obvious as possible as soon as possible, and the way we do that is an error. Warning-as-default-error is about as flexible as we can get, IMO, since it means a -Wno-error=whatever is a simple way ā€˜outā€™ of this.

1 Like
9

Iā€™m not in favor of this ā€“ Iā€™d expect this to break a lot of code which uses empty parens intending to declare a no-arg function. Code which, importantly, will be compatible with ā€“ and improved by ā€“ the change in semantics in C2x, with no code change needed. I donā€™t see why itā€™s a good idea to preemptively break such code that otherwise would need to make no changes.

Iā€™d be a lot happier with a warning/error which triggers only when you use such a function declaration in such a way as will become invalid in the future, e.g. calling it with arguments, or introducing a subsequent definition with arguments.

2 Likes
10

This makes sense to me. void f(); instead of void f(void); is a common C programmer mistake, so warning about it seems useful.

This seems more problematic. The warning makes sense because ā€œthis code probably doesnā€™t do what you wanted it toā€, but as far as I understand itā€™s not technically an error, so how would we justify flagging it as such?

11

The proposal is not completely clear to me. What about old-style function definitions with a prototype? Would they still be accepted by default (without warning)? Depending on the language standard, that was not really defined, but I think GCC always treated the old-style definition as a purely syntactical feature (without the usual ABI changes that happen for functions without prototypes).

One thing that will break is old configure (autoconf) tests. Worst case, they will simply produce different results, leading to features silently getting left out from the build.

1 Like
12

If that kind of signature is deprecated, why not just adding a -Wdeprecated-prototype and turn this on (without error) for the version where itā€™s deprecated?

13

This would be very bad, indeed.

14

One thing that will break is old configure (autoconf) tests. Worst case, they will simply produce different results, leading to features silently getting left out from the build.

Do you have an example of this? Is it part of autoconf itself, or some common idiom commonly used in autoconf tests? And what does ā€œoldā€ mean? Any change whatsoever might potentially silently break some autoconf test. But, with more details of what the issue here is, itā€™d be possible to evaluate each of the alternative options being discussed on this thread for their impact, which might motivate making one choice vs another.

15

If there is a declaration with a prototype before a K&R-style definition, the definition just silently inherits it. Otherwise, -Wstrict-prototypes triggers. This includes definitions without any arguments, e.g. void f() {}.

The argument list in a K&R style definition doesnā€™t affect the declared type of the function.

Not sure what you mean by the ABI changes. The standard requires that functions with and without prototypes have exactly the same ABI. The only unusual thing about a call to a function with no prototype is a non-standard backwards-compatibility hack: on some targets, the compiler emits extra instructions to ensure itā€™s possible to call a variadic function without a prototype. For example, on x86-64 Linux we set the ā€œalā€ register. (The standard never allowed this, and itā€™s impossible on some targets.)

16

Current autoconf still uses

char shm_open ();

to probe for function availability. Consider this:

AC_INIT(foo, 1.0)
AC_CHECK_FUNC(shm_open)

Build it with autoconf 2.69 or 2.71 (same difference), and you get:

CC="clang -Werror=strict-prototypes" ./configure 
checking for gcc... clang -Werror=strict-prototypes
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether the compiler supports GNU C... yes
checking whether clang -Werror=strict-prototypes accepts -g... yes
checking for clang -Werror=strict-prototypes option to enable C11 features... none needed
checking for shm_open... no

Without -Werror=strict-prototypes, the result is (with glibc 2.34 and later at least):

checking for shm_open... yes

This is actually much more breakage than I had expected. Itā€™s far less subtle than the similar issue around -Werror=implicit-function-declarations.

On top of the auto-generated tests from AC_CHECK_FUNC and similar autoconf facilities, all the manually crafted tests that follow this style would need fixing as well.

I still plan to work on making the world ready for -Werror=implicit-function-declarations. It seems quite doable at this point, although itā€™s a bit of work. But -Werror=strict-prototypes is in a complete different league and does not seem feasible at this point, not without kludges to in the compiler to turn it off for autoconf tests.

17

I donā€™t think the standard has ABI requirements. There are of course practical considerations if you want to support old code which calls core run-time functions without including the appropriate headers, but the standard does not seem to require support for this. I think the recent Darwin aarch64 ABI has different argument passing conventions for arguments not passed in registers, depending on whether there is a prototype or not (the stack slot sizes for arguments less than 8 bytes large are different).

Previous ABIs were carefully designed to support prototype-less calls to functions defined with a prototype. But the converse is not actually true. You already gave the x86-64 example, but there is also a difference in parameter save area handling on powerpc64le. Given the way the GNU toolchain encourages compatibility, my focus has been on eliminating implicit function declarations because they cause much more hard-to-diagnose breakage for us. Implicit function declarations also lack a prototype, so there is some overlap with Aaronā€™s proposal, but on top of that, they also get the return type wrong.

18

Thanks.

This is a great example of an idiom that will not be broken by the C2x change to treat char shm_open() as if it was char shm_open(void), but would be broken by -Werror=strict-prototypes.

Since autoconf goes out of its way to avoid seeing any other incompatible prototypes, the (void) interpretation is perfectly fine for it. So, if we make Clang emit a warning (with default-Werror) only for subsequent uses which are incompatible with the interpretation of this declaration as a zero-arg prototyped function (as Iā€™d suggested above), this should be OK.

The test code generated from the above autoconf snippet is:

/* confdefs.h */
#define PACKAGE_NAME "foo"
#define PACKAGE_TARNAME "foo"
#define PACKAGE_VERSION "1.0"
#define PACKAGE_STRING "foo 1.0"
#define PACKAGE_BUGREPORT ""
#define PACKAGE_URL ""
/* end confdefs.h. */
/* Define shm_open to an innocuous variant, in case <limits.h> declares shm_open.
For example, HP-UX 11i <limits.h> declares gettimeofday. */
#define shm_open innocuous_shm_open

/* System header to define __stub macros and hopefully few prototypes,
which can conflict with char shm_open (); below. */

#include <limits.h>
#undef shm_open

/* Override any GCC internal prototype to avoid an error.
Use char because int might match the return type of a GCC
builtin and then its argument prototype would still apply. */
#ifdef __cplusplus
extern "C"
#endif
char shm_open ();
/* The GNU C library defines this for functions which it implements
to always fail with ENOSYS. Some functions are actually named
something starting with __ and the normal name is an alias. */
#if defined __stub_shm_open || defined __stub___shm_open
choke me
#endif

int
main (void)
{
return shm_open ();
;
return 0;
}
19

Thank you everyone for the good feedback on this!

From what Iā€™m hearing, it sounds like there is not consensus to move forward with -Werror=strict-prototypes as the default behavior. Iā€™m okay with that outcome; I thought it was a bit of a stretch to get away with this aggressive of an approach.

It sounds like there is appetite for allowing the C2x behavior in earlier language modes via something like -fstrict-prototypes (but without -fno-strict-prototypes, as we donā€™t want to allow people to use K&R C functions in C2x or later). This is something I can tackle when I go to implement the C2x functionality. There were some requests to perhaps make this option be on by default in older C modes, but I donā€™t think we can take that approach as it will break code that is intentionally using K&R C functions, so thereā€™s conformance questions with that approach.

This still leaves the question of what to do about warnings. I am planning on rewording -Wstrict-prototypes to be more clear as to what itā€™s warning about and why: a function declaration without a prototype is deprecated in all versions of C and a block declaration without a prototype is deprecated. It does not sound like thereā€™s objection to enabling -Wstrict-prototypes by default in pre-C2x modes as a regular warning (with this rewording in mind), but some folks might like to see behavioral changes for when the warning triggers. Iā€™m not convinced that behavior changes to this diagnostic are worth the effort, but if someone has some real world examples (like the autoconf behavior change) where thereā€™s a demonstrable need, that would be really helpful. My current thinking is that when -fstrict-prototypes is enabled (implied by C2x mode), the warning wonā€™t trigger (theyā€™ll get errors in appropriate situations that are no longer supported, and () meaning (void) will happen automatically). When -fstrict-prototypes is not enabled, -Wstrict-prototypes will fire on precisely all of the locations where there is a potential change of behavior in later C modes and alerts the users to the deprecations that we werenā€™t previously warning about. Users not intending to upgrade to later language standards because they need these signatures can use -Wno-strict-prototypes. Users intending to upgrade later can either fix all signatures or enable -fstrict-prototypes to fix only a limited subset of signatures; either approach silences the diagnostics.

What do folks think of this revised approach?

20

The requirement Iā€™m referring to is that itā€™s legal in some cases to call a prototype-less function using a function pointer with a compatible prototype, and itā€™s legal in some cases to call a function with a prototype using a function pointer without a prototype. This is outlined in 6.5.2.2 and 6.7.6.3. This effectively means the ABIs must be compatible to some extent.

But this is getting off topic; in any case, we should discourage code that depends on this.