CuriousGeorgiy updated this revision to Diff 554259.
CuriousGeorgiy marked 2 inline comments as done.
CuriousGeorgiy added a comment.
Herald added a subscriber: ormris.
[analyzer] Add check for null pointer passed to the %p of printf family
The result of passing a null pointer to the pointer conversion specifier of
printf family of functions is implementation defined — for instance, on
Windows zeros are printed, whilst on Linux '(nil)' is printed.
The problem described above is a minor portability issue, so we don't just
add a check for to the existing Unix API checker, since it would be
desirable to be able to disable it.
Instead, to collect such checks we add a new APIPortabilityMinor checker to
the alpha.unix package with a check for the problem described above.
The check assumes that a null pointer is only passed to a pointer
conversion specifier, otherwise it would be undefined behaviour. By making
this assumption we do not have to check the format string and match data
arguments to conversion specifiers.
Closes #43453
Differential Revision: https://reviews.llvm.org/D154838
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154838/new/
https://reviews.llvm.org/D154838
Files:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/UnixAPIPortabilityMinorChecker.cpp
clang/test/Analysis/unix-api-portability-minor.cpp
Index: clang/test/Analysis/unix-api-portability-minor.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/unix-api-portability-minor.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.APIPortabilityMinor -verify %s
+
+#define NULL ((void*)0)
+
+struct FILE_t;
+typedef struct FILE_t FILE;
+
+typedef __typeof(sizeof(int)) size_t;
+
+int printf(const char *, ... );
+int fprintf(FILE *, const char *, ...);
+int sprintf(char *, const char *, ...);
+int snprintf(char *, size_t, const char *, ...);
+
+// Test basic case for the whole print family.
+void test_printf_family_pointer_conversion_specifier_null(FILE *file, char *buf, size_t buf_size, char *format) {
+ printf(format, NULL); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+ fprintf(file, format, NULL); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+ sprintf(buf, format, NULL); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+ snprintf(buf, buf_size, format, NULL); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+}
+
+// Test builtin null pointer type is handled correctly.
+void test_printf_pointer_conversion_specifier_null_nullptr(char *format) {
+ printf(format, nullptr); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+}
+
+// Test various argument indexes.
+void test_printf_pointer_conversion_specifier_null_various_arguments(char *format) {
+ printf(format, NULL); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+ printf(format, 1, NULL); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+ printf(format, 1, NULL, 2); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+ printf(format, NULL, NULL); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+ printf(format, NULL, 1, NULL); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+ printf(format, 0); // no-warning
+}
+
+// Test pointer constraints.
+void printf_pointer_conversion_specifier_null_pointer_constraints(char *format, int *pointer1, int *pointer2) {
+ // Unknown pointer should not rase warning.
+ printf(format, pointer1); // no-warning
+ // Pointer argument should not get constrained after the check.
+ *pointer1 = 777; // no-warning
+
+ if (pointer2 != NULL) {
+ printf(format, pointer1); // no-warning
+ return;
+ }
+ printf(format, pointer2); // expected-warning{{The result of passing a null pointer to the pointer conversion specifier of the printf family of functions is implementation defined}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/UnixAPIPortabilityMinorChecker.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/UnixAPIPortabilityMinorChecker.cpp
@@ -0,0 +1,177 @@
+//= UnixAPIPortabilityMinorChecker.cpp --------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This files defines the UnixAPIPortabilityMinorChecker, which is a collection
+// of checks for minor portability issues in calls to various UNIX/Posix
+// functions.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class UnixAPIPortabilityMinorChecker : public Checker<check::PreCall> {
+public:
+ UnixAPIPortabilityMinorChecker();
+
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+
+private:
+ std::unique_ptr<BugType> PrintfPointerConversionSpecifierNULLBugType;
+ void CheckPrintfFamilyPointerConversionSpecifierNULLGeneric(
+ const CallEvent &Call, CheckerContext &C,
+ unsigned int data_args_index) const;
+ void CheckPrintfFamilyPointerConversionSpecifierNULLPrintf(
+ const CallEvent &Call, CheckerContext &C) const;
+ void CheckPrintfFamilyPointerConversionSpecifierNULLFprintf(
+ const CallEvent &Call, CheckerContext &C) const;
+ void CheckPrintfFamilyPointerConversionSpecifierNULLSprintf(
+ const CallEvent &Call, CheckerContext &C) const;
+ void CheckPrintfFamilyPointerConversionSpecifierNULLSnprintf(
+ const CallEvent &Call, CheckerContext &C) const;
+ void ReportPrintfPointerConversionSpecifierNULL(CheckerContext &C,
+ ProgramStateRef nullState,
+ const Expr *arg) const;
+ using CheckFn = void (UnixAPIPortabilityMinorChecker::*)(
+ const CallEvent &Call, CheckerContext &C) const;
+ const CallDescriptionMap<CheckFn> PrintfFamilyFunctions = {
+ {{{"printf"}},
+ &UnixAPIPortabilityMinorChecker::
+ CheckPrintfFamilyPointerConversionSpecifierNULLPrintf},
+ {{{"fprintf"}},
+ &UnixAPIPortabilityMinorChecker::
+ CheckPrintfFamilyPointerConversionSpecifierNULLFprintf},
+ {{{"sprintf"}},
+ &UnixAPIPortabilityMinorChecker::
+ CheckPrintfFamilyPointerConversionSpecifierNULLSprintf},
+ {{{"snprintf"}},
+ &UnixAPIPortabilityMinorChecker::
+ CheckPrintfFamilyPointerConversionSpecifierNULLSnprintf}};
+};
+} // end anonymous namespace
+
+UnixAPIPortabilityMinorChecker::UnixAPIPortabilityMinorChecker() {
+ PrintfPointerConversionSpecifierNULLBugType.reset(
+ new BugType(this,
+ "Passing a null pointer to the pointer conversion "
+ "specifier of ",
+ categories::UnixAPI));
+}
+
+//===----------------------------------------------------------------------===//
+// printf family of functions with null pointer passed to pointer
+// conversion specifier
+//===----------------------------------------------------------------------===//
+
+// Generates an error report, indicating that the result of passing a null
+// pointer to pointer conversion specifier of printf family of functions is
+// implementation defined.
+void UnixAPIPortabilityMinorChecker::ReportPrintfPointerConversionSpecifierNULL(
+ CheckerContext &C, ProgramStateRef nullState, const Expr *arg) const {
+ ExplodedNode *N =
+ C.generateNonFatalErrorNode(nullState ? nullState : C.getState());
+ if (!N)
+ return;
+ auto report = std::make_unique<PathSensitiveBugReport>(
+ *PrintfPointerConversionSpecifierNULLBugType,
+ "The result of passing a null pointer to the pointer conversion "
+ "specifier of the printf family of functions is implementation defined",
+ N);
+ report->addRange(arg->getSourceRange());
+ bugreporter::trackExpressionValue(N, arg, *report);
+ C.emitReport(std::move(report));
+}
+
+// Checks data arguments of printf family of functions for a null pointer,
+// assuming it is passed to a pointer conversion specifier (%p), i.e. without
+// checking the format string.
+void UnixAPIPortabilityMinorChecker::
+ CheckPrintfFamilyPointerConversionSpecifierNULLGeneric(
+ const CallEvent &Call, CheckerContext &C,
+ unsigned int data_args_index) const {
+ ProgramStateRef state = C.getState();
+ ConstraintManager &CM = state->getConstraintManager();
+
+ for (unsigned int i = data_args_index; i < Call.getNumArgs(); i++) {
+ const Expr *arg = Call.getArgExpr(i);
+ if (!arg)
+ continue;
+
+ const QualType type = arg->getType();
+ if (!type->isPointerType() && !type->isNullPtrType())
+ continue;
+
+ SVal argVal = Call.getArgSVal(i);
+ if (argVal.isUnknownOrUndef())
+ continue;
+
+ auto argDefinedVal = argVal.getAs<DefinedSVal>();
+
+ ProgramStateRef notNullState, nullState;
+ std::tie(notNullState, nullState) =
+ CM.assumeDual(C.getState(), *argDefinedVal);
+ if (!notNullState && nullState) {
+ ReportPrintfPointerConversionSpecifierNULL(C, nullState, arg);
+ return;
+ }
+ }
+}
+
+void UnixAPIPortabilityMinorChecker::
+ CheckPrintfFamilyPointerConversionSpecifierNULLPrintf(
+ const CallEvent &Call, CheckerContext &C) const {
+ CheckPrintfFamilyPointerConversionSpecifierNULLGeneric(Call, C, 1);
+}
+
+void UnixAPIPortabilityMinorChecker::
+ CheckPrintfFamilyPointerConversionSpecifierNULLFprintf(
+ const CallEvent &Call, CheckerContext &C) const {
+ CheckPrintfFamilyPointerConversionSpecifierNULLGeneric(Call, C, 2);
+}
+
+void UnixAPIPortabilityMinorChecker::
+ CheckPrintfFamilyPointerConversionSpecifierNULLSprintf(
+ const CallEvent &Call, CheckerContext &C) const {
+ CheckPrintfFamilyPointerConversionSpecifierNULLGeneric(Call, C, 2);
+}
+
+void UnixAPIPortabilityMinorChecker::
+ CheckPrintfFamilyPointerConversionSpecifierNULLSnprintf(
+ const CallEvent &Call, CheckerContext &C) const {
+ CheckPrintfFamilyPointerConversionSpecifierNULLGeneric(Call, C, 3);
+}
+
+void UnixAPIPortabilityMinorChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (!Call.isGlobalCFunction())
+ return;
+
+ if (const auto *Checker = PrintfFamilyFunctions.lookup(Call))
+ (this->**Checker)(Call, C);
+}
+
+//===----------------------------------------------------------------------===//
+// Checker registration.
+//===----------------------------------------------------------------------===//
+
+void ento::registerUnixAPIPortabilityMinorChecker(CheckerManager &mgr) {
+ mgr.registerChecker<UnixAPIPortabilityMinorChecker>();
+}
+bool ento::shouldRegisterUnixAPIPortabilityMinorChecker(
+ const CheckerManager &mgr) {
+ return true;
+}
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -125,6 +125,7 @@
UninitializedObject/UninitializedObjectChecker.cpp
UninitializedObject/UninitializedPointee.cpp
UnixAPIChecker.cpp
+ UnixAPIPortabilityMinorChecker.cpp
UnreachableCodeChecker.cpp
VforkChecker.cpp
VLASizeChecker.cpp
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -582,6 +582,10 @@
WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, StreamChecker]>,
Documentation<HasDocumentation>;
+def UnixAPIPortabilityMinorChecker : Checker<"APIPortabilityMinor">,
+ HelpText<"Checks for minor portability issues in calls to various UNIX/Posix functions">,
+ Documentation<HasDocumentation>;
+
} // end "alpha.unix"
//===----------------------------------------------------------------------===//
Index: clang/docs/analyzer/checkers.rst
===================================================================
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -949,6 +949,32 @@
while(1);
}
+unix.APIPortabilityMinor (C)
+""""""""""""""""""""""""""""
+Checks for minor portability issues in calls to various UNIX/Posix
+functions.
+
+These are the checks included:
+ - Null pointer being passed to the pointer conversion specifier (%p) of the
+ printf family of functions. Passing null pointers to such functions is
+ implementation defined, thus non-portable. For instance, on Windows zeros
+ are printed, whilst on Linux '(nil)' is printed. The following functions are
+ considered: printf, fprintf, sprintf, snprintf.
+
+ .. code-block:: c
+
+ void test(void *p) {
+ if (p == NULL) {
+ printf("WARNING: null pointer passed");
+ }
+
+ printf("%p", p); // warn
+
+ if (p != NULL) {
+ printf("%p", p); // no warning
+ }
+ }
+
.. _unix-cstring-BadSizeArg:
unix.cstring.BadSizeArg (C)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
