Feedback fixes

Refactor `isNonNull` parsing and add C++ test for GNU `__null`

Author: bozicrHT

clang/include/clang/Basic/Builtins.h

@@ -392,9 +392,9 @@ class Context {
   bool performsCallback(unsigned ID,
                         llvm::SmallVectorImpl<int> &Encoding) const;
 
-  /// Return true if this builtin has parameters at fixed positions
-  /// that must be non-null.
-  bool IsNonNull(unsigned ID, llvm::SmallVectorImpl<int> &Indxs) const;
+  /// Return true if this builtin has parameters that must be non-null.
+  /// The parameter indices are appended into 'Indxs'.
+  bool isNonNull(unsigned ID, llvm::SmallVectorImpl<int> &Indxs) const;
 
   /// Return true if this function has no side effects and doesn't
   /// read memory, except for possibly errno or raising FP exceptions.

clang/lib/Basic/Builtins.cpp

@@ -293,22 +293,15 @@ bool Builtin::Context::isScanfLike(unsigned ID, unsigned &FormatIdx,
   return isLike(ID, FormatIdx, HasVAListArg, "sS");
 }
 
-bool Builtin::Context::IsNonNull(unsigned ID,
-                                 llvm::SmallVectorImpl<int> &Indxs) const {
-
-  const char *CalleePos = ::strchr(getAttributesString(ID), 'N');
-  if (!CalleePos)
-    return false;
-
-  ++CalleePos;
-  assert(*CalleePos == '<' &&
-         "Callback callee specifier must be followed by a '<'");
-  ++CalleePos;
+static void parseCommaSeparatedIndices(const char *CurrPos,
+                                       llvm::SmallVectorImpl<int> &Indxs) {
+  assert(*CurrPos == '<' && "Expected '<' to start index list");
+  ++CurrPos;
 
   char *EndPos;
-  int CalleeIdx = ::strtol(CalleePos, &EndPos, 10);
-  assert(CalleeIdx >= 0 && "Callee index is supposed to be positive!");
-  Indxs.push_back(CalleeIdx);
+  int PosIdx = ::strtol(CurrPos, &EndPos, 10);
+  assert(PosIdx >= 0 && "Index is supposed to be positive!");
+  Indxs.push_back(PosIdx);
 
   while (*EndPos == ',') {
     const char *PayloadPos = EndPos + 1;
@@ -317,7 +310,19 @@ bool Builtin::Context::IsNonNull(unsigned ID,
     Indxs.push_back(PayloadIdx);
   }
 
-  assert(*EndPos == '>' && "Callback callee specifier must end with a '>'");
+  assert(*EndPos == '>' && "Index list must end with '>'");
+}
+
+bool Builtin::Context::isNonNull(unsigned ID,
+                                 llvm::SmallVectorImpl<int> &Indxs) const {
+
+  const char *AttrPos = ::strchr(getAttributesString(ID), 'N');
+  if (!AttrPos)
+    return false;
+
+  ++AttrPos;
+  parseCommaSeparatedIndices(AttrPos, Indxs);
+
   return true;
 }
 
@@ -328,23 +333,8 @@ bool Builtin::Context::performsCallback(unsigned ID,
     return false;
 
   ++CalleePos;
-  assert(*CalleePos == '<' &&
-         "Callback callee specifier must be followed by a '<'");
-  ++CalleePos;
-
-  char *EndPos;
-  int CalleeIdx = ::strtol(CalleePos, &EndPos, 10);
-  assert(CalleeIdx >= 0 && "Callee index is supposed to be positive!");
-  Encoding.push_back(CalleeIdx);
-
-  while (*EndPos == ',') {
-    const char *PayloadPos = EndPos + 1;
-
-    int PayloadIdx = ::strtol(PayloadPos, &EndPos, 10);
-    Encoding.push_back(PayloadIdx);
-  }
+  parseCommaSeparatedIndices(CalleePos, Encoding);
 
-  assert(*EndPos == '>' && "Callback callee specifier must end with a '>'");
   return true;
 }
 

clang/lib/Sema/SemaDecl.cpp

@@ -17142,7 +17142,7 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) {
     }
 
     SmallVector<int, 4> Indxs;
-    if (Context.BuiltinInfo.IsNonNull(BuiltinID, Indxs) &&
+    if (Context.BuiltinInfo.isNonNull(BuiltinID, Indxs) &&
         !FD->hasAttr<NonNullAttr>()) {
       llvm::SmallVector<ParamIdx, 4> ParamIndxs;
       for (int I : Indxs)

clang/test/Sema/format-strings-nonnull.c

@@ -1,10 +1,10 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wnonnull -Wno-format-security %s
+// RUN: %clang_cc1 -fsyntax-only --std=c23 -verify -Wnonnull -Wno-format-security %s
 
-#include <stdarg.h>
-#include <stddef.h>
+#define NULL  (void*)0
 
 typedef struct _FILE FILE;
-
+typedef __SIZE_TYPE__ size_t;
+typedef __builtin_va_list va_list;
 int printf(char const* restrict, ...);
 int __builtin_printf(char const* restrict, ...);
 int fprintf(FILE* restrict, char const* restrict, ...);
@@ -48,7 +48,7 @@ void check_format_string(FILE *fp, va_list ap) {
     vfprintf(fp, 0, ap);
     // expected-warning@-1{{null passed to a callee that requires a non-null argument}}
 
-    vsprintf(buf, NULL, ap);
+    vsprintf(buf, nullptr, ap);
     // expected-warning@-1{{null passed to a callee that requires a non-null argument}}
 
     vsnprintf(buf, 10, fmt, ap);
@@ -57,7 +57,7 @@ void check_format_string(FILE *fp, va_list ap) {
     scanf(NULL);
     // expected-warning@-1{{null passed to a callee that requires a non-null argument}}
 
-    fscanf(fp, NULL);
+    fscanf(fp, nullptr);
     // expected-warning@-1{{null passed to a callee that requires a non-null argument}}
 
     sscanf(buf, fmt);

clang/test/Sema/format-strings-nonnull.cpp

@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wnonnull %s
+
+#ifdef __cplusplus
+# define EXTERN_C extern "C"
+#else
+# define EXTERN_C extern
+#endif
+
+typedef struct _FILE FILE;
+typedef __SIZE_TYPE__ size_t;
+typedef __builtin_va_list va_list;
+
+EXTERN_C int printf(const char *, ...);
+EXTERN_C int fprintf(FILE *, const char *restrict, ...);
+EXTERN_C int sprintf(char* restrict, char const* res, ...);
+
+EXTERN_C int scanf(char const *restrict, ...);
+EXTERN_C int fscanf(FILE* restrict, char const* res, ...);
+
+void test(FILE *fp) {
+  char buf[256];
+
+  __builtin_printf(__null, "x");
+  // expected-warning@-1 {{null passed to a callee that requires a non-null argument}}
+
+  printf(__null, "xxd");
+  // expected-warning@-1 {{null passed to a callee that requires a non-null argument}}
+
+  fprintf(fp, __null, 42);
+  // expected-warning@-1 {{null passed to a callee that requires a non-null argument}}
+
+  sprintf(buf, __null);
+  // expected-warning@-1 {{null passed to a callee that requires a non-null argument}}
+
+  scanf(__null);
+  // expected-warning@-1 {{null passed to a callee that requires a non-null argument}}
+
+  fscanf(fp, __null);
+  // expected-warning@-1 {{null passed to a callee that requires a non-null argument}}
+}