delcypher created this revision.
delcypher added reviewers: arphaman, rapidsna, fcloutier, NoQ.
delcypher requested review of this revision.
Herald added a project: clang.

The warning is currently disabled by default but can be enabled with
`-Wstrict-calls-without-prototype`. Enabling it by default will be
handled by future patches.

The warning is intended to catch C code like this:

  int f(); // No prototype, accepts any number of arguments of any type.
  
  int call(void) {
    return f(1, 2, 3);
  }

While code like this is legal it can very easily invoke undefined behavior
by passing the wrong number of arguments or wrong types.

The warning only fires when arguments are passed to make it less noisy, i.e.
so that the warning does not fire on code like this:

  int g();
  int call(void) {
      return g();
  }

While the above code could invoke undefined behavior, if the definition of 
`g()` takes no
arguments then the lack of a prototype is benign.

This warning while similar to `-Wstrict-prototypes` is distinct because
it warns at call sites rather at declarations.

This patch currently warns on calls to K&R style function declarations where a
previous declaration has a prototype. It is currently not clear if this is the 
correct behavior
as noted in the included test case.

rdar://87118271


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116635

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/warn-calls-without-prototype.c

Index: clang/test/Sema/warn-calls-without-prototype.c
===================================================================
--- /dev/null
+++ clang/test/Sema/warn-calls-without-prototype.c
@@ -0,0 +1,131 @@
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-calls-without-prototype -std=c99 -verify %s
+
+//------------------------------------------------------------------------------
+// Expected -Wstrict-calls-without-prototype warnings
+//------------------------------------------------------------------------------
+
+unsigned not_a_prototype();
+
+unsigned call_to_function_without_prototype(void) {
+  // expected-warning@+2{{calling function 'not_a_prototype' with arguments when function has no prototype}}
+  // expected-note@-4{{'not_a_prototype' declared here}}
+  return not_a_prototype(1);
+}
+
+// K&R style function declaration has no proto-type
+unsigned not_a_prototype2(a, b)
+int a;
+int b;
+{
+  return a + b;
+}
+
+unsigned call_to_function_without_prototype2(void) {
+  // expected-warning@+2{{calling function 'not_a_prototype2' with arguments when function has no prototype}}
+  // expected-note@-9{{'not_a_prototype2' declared here}}
+  return not_a_prototype2(1, 2);
+}
+
+// K&R style function seems to "hide" the prototype because the merged
+// FunctionDecl seems to not have a prototype.
+// FIXME(dliew): Is this a bug in `Sema::MergeFunctionDecl`?
+unsigned not_a_prototype3(int a, int b, int c); // Start with a prototype
+unsigned not_a_prototype3(a, b, c )
+  int a;
+  int b;
+  int c;
+{
+  return a + b +c;
+}
+
+unsigned call_to_function_without_prototype3(void) {
+  // FIXME(dliew): Should we actually warn about this? There is a prototype it
+  // just gets hidden when the FunctionDecls get merged.
+  // expected-warning@+2{{calling function 'not_a_prototype3' with arguments when function has no prototype}}
+  // expected-note@-12{{'not_a_prototype3' declared here}}
+  return not_a_prototype3(1, 2, 3);
+}
+
+// Merging two FunctionDecls without prototypes should result in a FunctionDecl without a prototype.
+unsigned not_a_prototype4(); // Start with something that has no prototype.
+unsigned not_a_prototype4(a, b, c)
+  int a;
+  int b;
+  int c;
+{
+  return a + b +c;
+}
+
+unsigned call_to_function_without_prototype4(void) {
+  // expected-warning@+2{{calling function 'not_a_prototype4' with arguments when function has no prototype}}
+  // expected-note@-10{{'not_a_prototype4' declared here}}
+  return not_a_prototype4(1, 2, 3);
+}
+
+//------------------------------------------------------------------------------
+// No expected warnings from -Wstrict-calls-without-prototype
+//------------------------------------------------------------------------------
+
+unsigned call_to_function_without_declartion(void) {
+  // `-Wstrict-calls-without-prototype` doesn't emit a warning here because
+  // `-Wimplicit-function-declaration` cover this.
+  // expected-warning@+1{{implicit declaration of function 'function_without_declaration' is invalid in C99}}
+  return function_without_declaration(5);
+}
+
+unsigned call_builtins(void) {
+  // Builtin with a signature
+  int *alloc = (int *)__builtin_alloca(sizeof(int)); // no warning
+  *alloc = 5;
+
+  // Builtin without a meaningful signature
+  int gt = __builtin_isgreater(0.0, 0.1); // no warning
+
+  return *alloc + gt;
+}
+
+unsigned provides_prototype(int *a);
+unsigned provides_prototype_from_definition(int *a) {
+  return a[1];
+}
+unsigned call_prototypes(void) {
+  int a[] = {0, 1, 2};
+  int result = provides_prototype_from_definition(a); // no warning
+  int result2 = provides_prototype(a); // no warning
+  return result + result2;
+}
+
+unsigned will_be_refined();
+unsigned will_be_refined(void *); // refine the decl to be a prototype
+unsigned call_to_refined_function(void) {
+  int *a;
+  return will_be_refined(a); // no warning
+}
+
+unsigned will_be_refined2(void *);
+unsigned will_be_refined2();
+unsigned call_to_refined_function2(void) {
+  int *a;
+  return will_be_refined2(a); // no warning
+}
+
+unsigned provides_prototype_variadic(int, ...);
+
+unsigned call_to_prototype_variadic(void) {
+    return provides_prototype_variadic(5, "test"); // no warning
+}
+
+// Declaring a function prototype after a K&R style function causes the merged
+// decl to be marked as having a prototype.
+unsigned not_a_prototype5(a, b, c )
+  int a;
+  int b;
+  int c;
+{
+  return a + b +c;
+}
+unsigned not_a_prototype5(int a, int b, int c); // Declare prototype after decl
+
+unsigned call_to_function_without_prototype5(void) {
+  return not_a_prototype5(1, 2, 3); // no warning
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -6388,6 +6388,29 @@
     Call = ActOnOpenMPCall(Call, Scope, LParenLoc, ArgExprs, RParenLoc,
                            ExecConfig);
 
+  // Diagnose calls that pass arguments to functions without a prototype
+  if (!LangOpts.CPlusPlus) {
+    if (auto *DRE = dyn_cast<DeclRefExpr>(Fn)) {
+      if (auto *FD = dyn_cast<FunctionDecl>(DRE->getDecl())) {
+        if (auto *CE = dyn_cast<CallExpr>(Call.get())) {
+          // Skip emitting warning for calls to implicit functions for the
+          // following reasons:
+          // * Calls to generated implicit functions are handled by
+          // `-Wimplicit-function-declaration`.
+          // * We want to skip implicit builtin functions. Some do not have
+          // a meaningful type signature (treated as not having a prototype).
+          if (CE->getNumArgs() > 0 && !FD->hasPrototype() &&
+              !FD->isImplicit()) {
+            Diag(CE->getExprLoc(), diag::warn_call_function_without_prototype)
+                << FD << CE->getSourceRange();
+            Diag(FD->getLocation(), diag::note_previous_decl)
+                << FD << FD->getSourceRange();
+          }
+        }
+      }
+    }
+  }
+
   return Call;
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5525,6 +5525,9 @@
   "this %select{function declaration is not|block declaration is not|"
   "old-style function definition is not preceded by}0 a prototype">,
   InGroup<DiagGroup<"strict-prototypes">>, DefaultIgnore;
+def warn_call_function_without_prototype : Warning<
+  "calling function %0 with arguments when function has no prototype">, InGroup<
+  DiagGroup<"strict-calls-without-prototype">>, DefaultIgnore;
 def warn_missing_variable_declarations : Warning<
   "no previous extern declaration for non-static variable %0">,
   InGroup<DiagGroup<"missing-variable-declarations">>, DefaultIgnore;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to