From 877d2dbd21d548a853b34faff4484c85ba7e8583 Mon Sep 17 00:00:00 2001 From: Daniel Krupp Date: Thu, 10 Nov 2022 11:41:07 +0100 Subject: [PATCH 1/3] Function Summary Config Option A new yaml config file is introduced for the stdlibrary checker, so the user can define argument constraints and function summaries in runtime. --- .../Checkers/StdLibraryFunctionsChecker.cpp | 173 +++++++++++++++++- clang/test/Analysis/Inputs/fread-summary.yaml | 25 +++ .../test/Analysis/Inputs/isalnum-summary.yaml | 37 ++++ .../Analysis/std-c-library-functions-config.c | 70 +++++++ 4 files changed, 303 insertions(+), 2 deletions(-) create mode 100644 clang/test/Analysis/Inputs/fread-summary.yaml create mode 100644 clang/test/Analysis/Inputs/isalnum-summary.yaml create mode 100644 clang/test/Analysis/std-c-library-functions-config.c diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index c5fa43ac23d1e..f397070e652b0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -52,11 +52,102 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" +#include "Yaml.h" +#include "llvm/Support/YAMLTraits.h" + #include using namespace clang; using namespace clang::ento; + + +//SUMMARY CONFIGURATION PARSING + + +struct SummaryConfiguration{ + enum class ArgConstraintType{ + NotNull, + BufferSize + }; + struct ArgConstraint{ + ArgConstraintType type; + int arg; //Arg count in case of NotNull constraint type + int bufferArg; // Arg count of the destination buffer + int sizeArg; //arg count of the size argument + int countArg; //arg count of the element count argument + }; + struct Signature{ + std::vector argTypes; + std::string returnType; + }; + enum class EvaluationType{ + NoEvalCall, + EvalCallAsPure + }; + struct Summary{ + std::string name; + EvaluationType evaluationType; + Signature signature; + std::vector argConstraints; + + }; + std::vector summaries; +}; + +LLVM_YAML_IS_SEQUENCE_VECTOR(SummaryConfiguration::Summary) +LLVM_YAML_IS_SEQUENCE_VECTOR(SummaryConfiguration::ArgConstraint) + +// YAML CONFIG PARSING +namespace llvm { +namespace yaml { +template <> struct MappingTraits { + static void mapping(IO &IO, SummaryConfiguration &Config) { + IO.mapRequired("Summaries", Config.summaries); + } +}; +template <> struct MappingTraits { + static void mapping(IO &IO, SummaryConfiguration::Summary &Config) { + IO.mapRequired("Name", Config.name); + IO.mapRequired("EvaluationType", Config.evaluationType); + IO.mapRequired("Signature", Config.signature); + IO.mapRequired("ArgConstraints", Config.argConstraints); + } +}; +template <> struct ScalarEnumerationTraits { + static void enumeration(IO &IO, SummaryConfiguration::EvaluationType &Config) { + IO.enumCase(Config, "NoEvalCall", SummaryConfiguration::EvaluationType::NoEvalCall); + IO.enumCase(Config, "EvalCallAsPure", SummaryConfiguration::EvaluationType::EvalCallAsPure); + } +}; +template <> struct MappingTraits { + static void mapping(IO &IO, SummaryConfiguration::Signature &Config) { + IO.mapRequired("ArgTypes", Config.argTypes); + IO.mapRequired("RetType", Config.returnType); + } +}; + +template <> struct ScalarEnumerationTraits { + static void enumeration(IO &IO, SummaryConfiguration::ArgConstraintType &Config) { + IO.enumCase(Config, "NotNull", SummaryConfiguration::ArgConstraintType::NotNull); + IO.enumCase(Config, "BufferSize", SummaryConfiguration::ArgConstraintType::BufferSize); + } +}; + +template <> struct MappingTraits { + static void mapping(IO &IO, SummaryConfiguration::ArgConstraint &Config) { + IO.mapRequired("type", Config.type); + IO.mapOptional("arg", Config.arg); + IO.mapOptional("bufferArg", Config.bufferArg); + IO.mapOptional("sizeArg", Config.sizeArg); + IO.mapOptional("countArg", Config.countArg); + } +}; + +} // namespace yaml +} // namespace llvm + + namespace { class StdLibraryFunctionsChecker : public Checker { @@ -1357,6 +1448,84 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( Optional FilePtrTy = getPointerTy(FileTy); Optional FilePtrRestrictTy = getRestrictTy(FilePtrTy); +////////READING SUMMARY CONFIG///////////////////// + +// User-provided summary configuration. +CheckerManager *Mgr = C.getAnalysisManager().getCheckerManager(); +std::string Option{"Config"}; +/*StringRef ConfigFile = + Mgr->getAnalyzerOptions().getCheckerStringOption(this, Option);*/ +StringRef ConfigFile = "/local/workspace/llvm-project/clang/test/Analysis/Inputs/fread-summary.yaml"; +llvm::Optional Config = + getConfiguration(*Mgr, this, Option, ConfigFile); +llvm::errs()<<"Config :"<summaries){ + llvm::errs()<<"Config :"<dump(); + args.push_back(lookupTy(t)); + } + } + RetType rt = lookupTy(s.signature.returnType); + auto GetSummary = [s]() { + switch (s.evaluationType) { + case SummaryConfiguration::EvaluationType::NoEvalCall: + return Summary(NoEvalCall); + case SummaryConfiguration::EvaluationType::EvalCallAsPure: + return Summary(EvalCallAsPure); + } + }; + Summary summary = GetSummary(); + + for (const SummaryConfiguration::ArgConstraint &ac: s.argConstraints){ + switch (ac.type){ + case SummaryConfiguration::ArgConstraintType::NotNull: + summary.ArgConstraint(NotNull(ac.arg)); + break; + case SummaryConfiguration::ArgConstraintType::BufferSize: + summary.ArgConstraint(BufferSize(ac.bufferArg,ac.sizeArg, ac.countArg)); + break; + } + } + + addToFunctionSummaryMap( + s.name, + Signature(args, + rt), + summary); + } +} + /* + WE want to add this + auto FreadSummary = + Summary(NoEvalCall) + .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)), + ReturnValueCondition(WithinRange, Range(0, SizeMax))}, + ErrnoIrrelevant) + .ArgConstraint(NotNull(ArgNo(0))) + .ArgConstraint(NotNull(ArgNo(3))) + .ArgConstraint(BufferSize(ArgNo(0), ArgNo(1), + ArgNo(2))); + */ + // size_t fread(void *restrict ptr, size_t size, size_t nitems, + // FILE *restrict stream); + /*addToFunctionSummaryMap( + "fread", + Signature(ArgTypes{VoidPtrRestrictTy, SizeTy, SizeTy, FilePtrRestrictTy}, + RetType{SizeTy}), + FreadSummary);*/ + + +//////////////////////////////////////////////////// + + + // We are finally ready to define specifications for all supported functions. // // Argument ranges should always cover all variants. If return value @@ -1591,11 +1760,11 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( // size_t fread(void *restrict ptr, size_t size, size_t nitems, // FILE *restrict stream); - addToFunctionSummaryMap( + /*addToFunctionSummaryMap( "fread", Signature(ArgTypes{VoidPtrRestrictTy, SizeTy, SizeTy, FilePtrRestrictTy}, RetType{SizeTy}), - FreadSummary); + FreadSummary);*/ // size_t fwrite(const void *restrict ptr, size_t size, size_t nitems, // FILE *restrict stream); addToFunctionSummaryMap("fwrite", diff --git a/clang/test/Analysis/Inputs/fread-summary.yaml b/clang/test/Analysis/Inputs/fread-summary.yaml new file mode 100644 index 0000000000000..02a880a1078f5 --- /dev/null +++ b/clang/test/Analysis/Inputs/fread-summary.yaml @@ -0,0 +1,25 @@ +# size_t fread(void *restrict ptr, size_t size, size_t nitems, +# FILE *restrict stream); +Summaries: + - Name: "fread" + Signature: + ArgTypes: + - "size_t" + - "size_t" + - "size_t" + - "FILE *restrict" + RetType: "size_t" + EvaluationType: "NoEvalCall" + ArgConstraints: # We give an error if this is violated + - + type: "NotNull" + arg: 0 + + - + type: "NotNull" + arg: 3 + - + type: "BufferSize" + bufferArg: 0 + sizeArg: 1 + countArg: 2 diff --git a/clang/test/Analysis/Inputs/isalnum-summary.yaml b/clang/test/Analysis/Inputs/isalnum-summary.yaml new file mode 100644 index 0000000000000..9098f1f480062 --- /dev/null +++ b/clang/test/Analysis/Inputs/isalnum-summary.yaml @@ -0,0 +1,37 @@ +#list of summaries +- Name: "isalnum" #int isalnum(int) + Signature: + ArgTypes: + - "int" + RetType: "int" + EvaluationType: "EvalCallAsPure" # or NoEvalCall + #case1 + Summary: #This models the function behaviour + - ArgumentCondition: + arg: 0 + type: "WithinRange" + ranges: [['0','9'],['A', 'Z'], ['a', 'z']] + ReturnValueCondition: + type: "OutOfRange" + ranges: [0,0] + Errno: "ErrnoIrrelevant" + AssumptionNote: "Assuming the character is alphanumeric" + - ArgumentCondition: + arg: 0 + type: "WithinRange" + ranges: [128,"UCharRangeMax"] + Errno: "ErrnoIrrelevant" + - ArgumentCondition: + arg: 0 + type: "OutOfRange" + ranges: [['0','9'],['A', 'Z'], ['a', 'z'],[128,"UCharRangeMax"]] + ReturnValueCondition: + type: "WithinRange" + ranges: [0,0] + Errno: "ErrnoIrrelevant" + AssumptionNote: "Assuming the character is non-alphanumeric" + ArgConstraint: # We give an error if this is violated + - ArgumentCondition: + arg: 0 + type: "WithinRange" + ranges: [["EOFv","EOFv"],[0,"UCharRangeMax"]] diff --git a/clang/test/Analysis/std-c-library-functions-config.c b/clang/test/Analysis/std-c-library-functions-config.c new file mode 100644 index 0000000000000..5e0534bcd88d7 --- /dev/null +++ b/clang/test/Analysis/std-c-library-functions-config.c @@ -0,0 +1,70 @@ +// Check the basic reporting/warning and the application of constraints. +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \ +// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \ +// RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -triple x86_64-unknown-linux-gnu \ +// RUN: -verify=report + +// Check the bugpath related to the reports. +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \ +// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \ +// RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -triple x86_64-unknown-linux-gnu \ +// RUN: -analyzer-output=text \ +// RUN: -verify=bugpath + +void clang_analyzer_eval(int); + +typedef struct FILE FILE; +typedef typeof(sizeof(int)) size_t; +size_t fread(void *restrict, size_t, size_t, FILE *restrict); +void test_notnull_concrete(FILE *fp) { + fread(0, sizeof(int), 10, fp); // \ + // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ + // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ + // bugpath-note{{Function argument constraint is not satisfied}} +} +void test_notnull_symbolic(FILE *fp, int *buf) { + fread(buf, sizeof(int), 10, fp); + clang_analyzer_eval(buf != 0); // \ + // report-warning{{TRUE}} \ + // bugpath-warning{{TRUE}} \ + // bugpath-note{{TRUE}} \ + // bugpath-note{{'buf' is not equal to null}} +} +void test_notnull_symbolic2(FILE *fp, int *buf) { + if (!buf) // bugpath-note{{Assuming 'buf' is null}} \ + // bugpath-note{{Taking true branch}} + fread(buf, sizeof(int), 10, fp); // \ + // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ + // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ + // bugpath-note{{Function argument constraint is not satisfied}} +} +typedef __WCHAR_TYPE__ wchar_t; +// This is one test case for the ARR38-C SEI-CERT rule. +void ARR38_C_F(FILE *file) { + enum { BUFFER_SIZE = 1024 }; + wchar_t wbuf[BUFFER_SIZE]; // bugpath-note{{'wbuf' initialized here}} + + const size_t size = sizeof(*wbuf); // bugpath-note{{'size' initialized to}} + const size_t nitems = sizeof(wbuf); // bugpath-note{{'nitems' initialized to}} + + // The 3rd parameter should be the number of elements to read, not + // the size in bytes. + fread(wbuf, size, nitems, file); // \ + // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ + // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ + // bugpath-note{{Function argument constraint is not satisfied}} +} From d256712143eb1181cc472a431707424eabb62792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= Date: Thu, 10 Nov 2022 17:04:16 +0100 Subject: [PATCH 2/3] Implement basic type lookup --- .../Checkers/StdLibraryFunctionsChecker.cpp | 72 +++++++++++-------- clang/test/Analysis/Inputs/fread-summary.yaml | 4 +- 2 files changed, 45 insertions(+), 31 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index f397070e652b0..92315b1b6b3bd 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -1457,23 +1457,39 @@ std::string Option{"Config"}; Mgr->getAnalyzerOptions().getCheckerStringOption(this, Option);*/ StringRef ConfigFile = "/local/workspace/llvm-project/clang/test/Analysis/Inputs/fread-summary.yaml"; llvm::Optional Config = - getConfiguration(*Mgr, this, Option, ConfigFile); -llvm::errs()<<"Config :"<summaries){ - llvm::errs()<<"Config :"<dump(); - args.push_back(lookupTy(t)); - } + getConfiguration(*Mgr, this, Option, ConfigFile); +llvm::errs() << "Config :" << Config.has_value() << "\n"; + +auto GetTypeFromStr = [&](StringRef TypeName) { + Optional LType = lookupTy(TypeName); + if (LType) + return *LType; + + return llvm::StringSwitch(TypeName) + .Case("void *", VoidPtrTy) + .Case("void * restrict", VoidPtrRestrictTy) + .Default(Irrelevant); +}; + +if (Config.has_value()) { + for (const SummaryConfiguration::Summary &s : Config->summaries) { + ArgTypes Args; + for (const std::string &TypeName : s.signature.argTypes) { + llvm::errs() << "arg type string:" << TypeName << "\n"; + QualType Type = GetTypeFromStr(TypeName); + llvm::errs() << "arg type dump:"; + Type.dump(); + llvm::errs() << "\n"; + + Args.push_back(Type); } - RetType rt = lookupTy(s.signature.returnType); - auto GetSummary = [s]() { + + const std::string &RetTypeName = s.signature.returnType; + llvm::errs() << "ret type string:" << RetTypeName << "\n"; + QualType RetType = GetTypeFromStr(RetTypeName); + llvm::errs() << "ret type dump:"; + + auto GetSummary = [&s]() { switch (s.evaluationType) { case SummaryConfiguration::EvaluationType::NoEvalCall: return Summary(NoEvalCall); @@ -1481,24 +1497,22 @@ if (Config.has_value()){ return Summary(EvalCallAsPure); } }; + Summary summary = GetSummary(); - for (const SummaryConfiguration::ArgConstraint &ac: s.argConstraints){ - switch (ac.type){ - case SummaryConfiguration::ArgConstraintType::NotNull: - summary.ArgConstraint(NotNull(ac.arg)); - break; - case SummaryConfiguration::ArgConstraintType::BufferSize: - summary.ArgConstraint(BufferSize(ac.bufferArg,ac.sizeArg, ac.countArg)); - break; + for (const SummaryConfiguration::ArgConstraint &AC : s.argConstraints) { + switch (AC.type) { + case SummaryConfiguration::ArgConstraintType::NotNull: + summary.ArgConstraint(NotNull(AC.arg)); + break; + case SummaryConfiguration::ArgConstraintType::BufferSize: + summary.ArgConstraint( + BufferSize(AC.bufferArg, AC.sizeArg, AC.countArg)); + break; } } - addToFunctionSummaryMap( - s.name, - Signature(args, - rt), - summary); + addToFunctionSummaryMap(s.name, Signature(Args, RetType), summary); } } /* diff --git a/clang/test/Analysis/Inputs/fread-summary.yaml b/clang/test/Analysis/Inputs/fread-summary.yaml index 02a880a1078f5..0c7b582e886a5 100644 --- a/clang/test/Analysis/Inputs/fread-summary.yaml +++ b/clang/test/Analysis/Inputs/fread-summary.yaml @@ -4,10 +4,10 @@ Summaries: - Name: "fread" Signature: ArgTypes: + - "void*" - "size_t" - "size_t" - - "size_t" - - "FILE *restrict" + - "FILE*" RetType: "size_t" EvaluationType: "NoEvalCall" ArgConstraints: # We give an error if this is violated From b76fceaa9def315ae56d0c6d5b8b281538443ead Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= Date: Thu, 10 Nov 2022 18:18:38 +0100 Subject: [PATCH 3/3] Implement checker config for summary configuration --- .../include/clang/StaticAnalyzer/Checkers/Checkers.td | 5 +++++ .../Checkers/StdLibraryFunctionsChecker.cpp | 11 ++++++----- clang/test/Analysis/std-c-library-functions-config.c | 2 ++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 3dd2c7c1606c7..518bffcd6f965 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -377,6 +377,11 @@ def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">, "If set to true, the checker models functions from the " "POSIX standard.", "false", + InAlpha>, + CmdLineOption ]>, Documentation, diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index 92315b1b6b3bd..b1077b2b3aa9b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -774,6 +774,7 @@ class StdLibraryFunctionsChecker bool DisplayLoadedSummaries = false; bool ModelPOSIX = false; bool ShouldAssumeControlledEnvironment = false; + std::string SummaryConfigPath; private: Optional findFunctionSummary(const FunctionDecl *FD, @@ -1452,12 +1453,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( // User-provided summary configuration. CheckerManager *Mgr = C.getAnalysisManager().getCheckerManager(); -std::string Option{"Config"}; -/*StringRef ConfigFile = - Mgr->getAnalyzerOptions().getCheckerStringOption(this, Option);*/ -StringRef ConfigFile = "/local/workspace/llvm-project/clang/test/Analysis/Inputs/fread-summary.yaml"; +std::string Option{"SummaryConfigPath"}; + Mgr->getAnalyzerOptions().getCheckerStringOption(this, Option); llvm::Optional Config = - getConfiguration(*Mgr, this, Option, ConfigFile); + getConfiguration(*Mgr, this, Option, SummaryConfigPath); llvm::errs() << "Config :" << Config.has_value() << "\n"; auto GetTypeFromStr = [&](StringRef TypeName) { @@ -3178,6 +3177,8 @@ void ento::registerStdCLibraryFunctionsChecker(CheckerManager &mgr) { Checker->ModelPOSIX = Opts.getCheckerBooleanOption(Checker, "ModelPOSIX"); Checker->ShouldAssumeControlledEnvironment = Opts.ShouldAssumeControlledEnvironment; + Checker->SummaryConfigPath = + Opts.getCheckerStringOption(Checker, "SummaryConfigPath"); } bool ento::shouldRegisterStdCLibraryFunctionsChecker( diff --git a/clang/test/Analysis/std-c-library-functions-config.c b/clang/test/Analysis/std-c-library-functions-config.c index 5e0534bcd88d7..5a6557ca95b97 100644 --- a/clang/test/Analysis/std-c-library-functions-config.c +++ b/clang/test/Analysis/std-c-library-functions-config.c @@ -2,6 +2,7 @@ // RUN: %clang_analyze_cc1 %s \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \ +// RUN: -analyzer-config apiModeling.StdCLibraryFunctions:SummaryConfigPath="%S/Inputs/fread-summary.yaml" \ // RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \ // RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \ // RUN: -analyzer-checker=debug.ExprInspection \ @@ -12,6 +13,7 @@ // RUN: %clang_analyze_cc1 %s \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \ +// RUN: -analyzer-config apiModeling.StdCLibraryFunctions:SummaryConfigPath="%S/Inputs/fread-summary.yaml" \ // RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \ // RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \ // RUN: -analyzer-checker=debug.ExprInspection \