llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-driver Author: Thurston Dang (thurstond) <details> <summary>Changes</summary> This adds a function to parse weighted sanitizer flags (e.g., -fsanitize-blah=undefined=0.5,null=0.3) and adds the plumbing to apply that to a new flag, -fno-sanitize-top-hot, from the frontend to backend. -fno-sanitize-top-hot currently has no effect; future work will use it to generalize ubsan-guard-checks (originaly introduced in 5f9ed2ff8364ff3e4fac410472f421299dafa793). --- Patch is 22.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/121619.diff 9 Files Affected: - (modified) clang/include/clang/Basic/CodeGenOptions.h (+8) - (modified) clang/include/clang/Basic/Sanitizers.h (+16) - (modified) clang/include/clang/Driver/Options.td (+7) - (modified) clang/include/clang/Driver/SanitizerArgs.h (+2) - (modified) clang/lib/Basic/Sanitizers.cpp (+40) - (modified) clang/lib/Driver/SanitizerArgs.cpp (+72-23) - (modified) clang/lib/Frontend/CompilerInvocation.cpp (+24) - (modified) clang/test/CodeGen/allow-ubsan-check.c (-1) - (modified) clang/test/Driver/fsanitize.c (+19) ``````````diff diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h index 8097c9ef772bc7..227bddf831002e 100644 --- a/clang/include/clang/Basic/CodeGenOptions.h +++ b/clang/include/clang/Basic/CodeGenOptions.h @@ -384,6 +384,14 @@ class CodeGenOptions : public CodeGenOptionsBase { /// the expense of debuggability). SanitizerSet SanitizeMergeHandlers; + /// Set of thresholds: the top hottest code responsible for the given + /// fraction of PGO counters will be excluded from sanitization + /// (0.0 [default] = skip none, 1.0 = skip all). + SanitizerSet NoSanitizeTopHot; + /// N.B. The cutoffs contain strictly more information than the SanitizerSet, + /// but the SanitizerSet is more efficient for some calculations. + SanitizerMaskCutoffs NoSanitizeTopHotCutoffs = {0}; + /// List of backend command-line options for -fembed-bitcode. std::vector<uint8_t> CmdArgs; diff --git a/clang/include/clang/Basic/Sanitizers.h b/clang/include/clang/Basic/Sanitizers.h index c890242269b334..12c2c93a7f89f6 100644 --- a/clang/include/clang/Basic/Sanitizers.h +++ b/clang/include/clang/Basic/Sanitizers.h @@ -154,6 +154,8 @@ struct SanitizerKind { #include "clang/Basic/Sanitizers.def" }; // SanitizerKind +using SanitizerMaskCutoffs = std::array<float, SanitizerKind::SO_Count>; + struct SanitizerSet { /// Check if a certain (single) sanitizer is enabled. bool has(SanitizerMask K) const { @@ -186,10 +188,24 @@ struct SanitizerSet { /// Returns a non-zero SanitizerMask, or \c 0 if \p Value is not known. SanitizerMask parseSanitizerValue(StringRef Value, bool AllowGroups); +/// Parse a single weighted value (e.g., 'undefined=0.05') from a -fsanitize= or +/// -fno-sanitize= value list. +/// Returns a non-zero SanitizerMask, or \c 0 if \p Value is not known. +/// The relevant weight(s) are updated in the passed array. +/// Individual Cutoffs are never reset to zero unless explicitly set +/// (e.g., 'null=0.0'). +SanitizerMask parseSanitizerWeightedValue(StringRef Value, bool AllowGroups, + SanitizerMaskCutoffs *Cutoffs); + /// Serialize a SanitizerSet into values for -fsanitize= or -fno-sanitize=. void serializeSanitizerSet(SanitizerSet Set, SmallVectorImpl<StringRef> &Values); +/// Serialize a SanitizerMaskCutoffs into values for -fsanitize= or +/// -fno-sanitize=. +void serializeSanitizerMaskCutoffs(const SanitizerMaskCutoffs Cutoffs, + SmallVectorImpl<std::string> &Values); + /// For each sanitizer group bit set in \p Kinds, set the bits for sanitizers /// this group enables. SanitizerMask expandSanitizerGroups(SanitizerMask Kinds); diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index d922709db17786..027093157d4c73 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2649,6 +2649,13 @@ def fsanitize_undefined_strip_path_components_EQ : Joined<["-"], "fsanitize-unde HelpText<"Strip (or keep only, if negative) a given number of path components " "when emitting check metadata.">, MarshallingInfoInt<CodeGenOpts<"EmitCheckPathComponentsToStrip">, "0", "int">; +def fno_sanitize_top_hot_EQ + : CommaJoined<["-"], "fno-sanitize-top-hot=">, + Group<f_clang_Group>, + HelpText<"Exclude sanitization for the top hottest code responsible for " + "the given fraction of PGO counters " + "(0.0 [default] = skip none; 1.0 = skip all). " + "Argument format: <sanitizer1>=,<sanitizer2>=,...">; } // end -f[no-]sanitize* flags diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h index 3b275092bbbe86..2462228f533746 100644 --- a/clang/include/clang/Driver/SanitizerArgs.h +++ b/clang/include/clang/Driver/SanitizerArgs.h @@ -26,6 +26,8 @@ class SanitizerArgs { SanitizerSet RecoverableSanitizers; SanitizerSet TrapSanitizers; SanitizerSet MergeHandlers; + SanitizerSet TopHot; + SanitizerMaskCutoffs TopHotCutoffs = {0}; std::vector<std::string> UserIgnorelistFiles; std::vector<std::string> SystemIgnorelistFiles; diff --git a/clang/lib/Basic/Sanitizers.cpp b/clang/lib/Basic/Sanitizers.cpp index 62ccdf8e9bbf28..6711b05c4539dd 100644 --- a/clang/lib/Basic/Sanitizers.cpp +++ b/clang/lib/Basic/Sanitizers.cpp @@ -36,6 +36,37 @@ SanitizerMask clang::parseSanitizerValue(StringRef Value, bool AllowGroups) { return ParsedKind; } +SanitizerMask +clang::parseSanitizerWeightedValue(StringRef Value, bool AllowGroups, + SanitizerMaskCutoffs *Cutoffs) { + SanitizerMask ParsedKind = llvm::StringSwitch<SanitizerMask>(Value) +#define SANITIZER(NAME, ID) .StartsWith(NAME "=", SanitizerKind::ID) +#define SANITIZER_GROUP(NAME, ID, ALIAS) \ + .StartsWith(NAME "=", \ + AllowGroups ? SanitizerKind::ID##Group : SanitizerMask()) +#include "clang/Basic/Sanitizers.def" + .Default(SanitizerMask()); + + if (ParsedKind && Cutoffs) { + size_t equalsIndex = Value.find_first_of('='); + if (equalsIndex != llvm::StringLiteral::npos) { + double arg; + if ((Value.size() > (equalsIndex + 1)) && + !Value.substr(equalsIndex + 1).getAsDouble(arg)) { + // AllowGroups is already taken into account for ParsedKind, + // hence we unconditionally expandSanitizerGroups. + SanitizerMask ExpandedKind = expandSanitizerGroups(ParsedKind); + + for (unsigned int i = 0; i < SanitizerKind::SO_Count; i++) + if (ExpandedKind & SanitizerMask::bitPosToMask(i)) + (*Cutoffs)[i] = arg; + } + } + } + + return ParsedKind; +} + void clang::serializeSanitizerSet(SanitizerSet Set, SmallVectorImpl<StringRef> &Values) { #define SANITIZER(NAME, ID) \ @@ -44,6 +75,15 @@ void clang::serializeSanitizerSet(SanitizerSet Set, #include "clang/Basic/Sanitizers.def" } +void clang::serializeSanitizerMaskCutoffs( + const SanitizerMaskCutoffs Cutoffs, SmallVectorImpl<std::string> &Values) { +#define SANITIZER(NAME, ID) \ + if (Cutoffs[SanitizerKind::SO_##ID]) \ + Values.push_back(std::string(NAME "=") + \ + std::to_string(Cutoffs[SanitizerKind::SO_##ID])); +#include "clang/Basic/Sanitizers.def" +} + SanitizerMask clang::expandSanitizerGroups(SanitizerMask Kinds) { #define SANITIZER(NAME, ID) #define SANITIZER_GROUP(NAME, ID, ALIAS) \ diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index 98116e2c8336b8..f7db3e5032ce1a 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -111,7 +111,8 @@ enum BinaryMetadataFeature { /// Parse a -fsanitize= or -fno-sanitize= argument's values, diagnosing any /// invalid components. Returns a SanitizerMask. static SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A, - bool DiagnoseErrors); + bool DiagnoseErrors, + SanitizerMaskCutoffs *Cutoffs); /// Parse -f(no-)?sanitize-coverage= flag values, diagnosing any invalid /// components. Returns OR of members of \c CoverageFeature enumeration. @@ -260,7 +261,7 @@ static SanitizerMask parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args, bool DiagnoseErrors, SanitizerMask Default, SanitizerMask AlwaysIn, SanitizerMask AlwaysOut, int OptInID, - int OptOutID) { + int OptOutID, SanitizerMaskCutoffs *Cutoffs) { assert(!(AlwaysIn & AlwaysOut) && "parseSanitizeArgs called with contradictory in/out requirements"); @@ -271,7 +272,7 @@ parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args, SanitizerMask DiagnosedAlwaysOutViolations; for (const auto *Arg : Args) { if (Arg->getOption().matches(OptInID)) { - SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors); + SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors, Cutoffs); // Report error if user explicitly tries to opt-in to an always-out // sanitizer. if (SanitizerMask KindsToDiagnose = @@ -287,7 +288,7 @@ parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args, Output |= expandSanitizerGroups(Add); Arg->claim(); } else if (Arg->getOption().matches(OptOutID)) { - SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors); + SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors, Cutoffs); // Report error if user explicitly tries to opt-out of an always-in // sanitizer. if (SanitizerMask KindsToDiagnose = @@ -320,7 +321,15 @@ static SanitizerMask parseSanitizeTrapArgs(const Driver &D, // (not even in recover mode) in order to avoid the need for a ubsan runtime. return parseSanitizeArgs(D, Args, DiagnoseErrors, TrappingDefault, AlwaysTrap, NeverTrap, options::OPT_fsanitize_trap_EQ, - options::OPT_fno_sanitize_trap_EQ); + options::OPT_fno_sanitize_trap_EQ, nullptr); +} + +static SanitizerMask parseNoSanitizeHotArgs(const Driver &D, + const llvm::opt::ArgList &Args, + bool DiagnoseErrors, + SanitizerMaskCutoffs *Cutoffs) { + return parseSanitizeArgs(D, Args, DiagnoseErrors, {}, {}, {}, + options::OPT_fno_sanitize_top_hot_EQ, -1, Cutoffs); } bool SanitizerArgs::needsFuzzerInterceptors() const { @@ -403,7 +412,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, for (const llvm::opt::Arg *Arg : llvm::reverse(Args)) { if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) { Arg->claim(); - SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors); + SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors, nullptr); if (RemoveObjectSizeAtO0) { AllRemove |= SanitizerKind::ObjectSize; @@ -573,7 +582,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, Kinds |= Add; } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) { Arg->claim(); - SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors); + SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors, nullptr); AllRemove |= expandSanitizerGroups(Remove); } } @@ -698,7 +707,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, SanitizerMask RecoverableKinds = parseSanitizeArgs( D, Args, DiagnoseErrors, RecoverableByDefault, AlwaysRecoverable, Unrecoverable, options::OPT_fsanitize_recover_EQ, - options::OPT_fno_sanitize_recover_EQ); + options::OPT_fno_sanitize_recover_EQ, nullptr); RecoverableKinds |= AlwaysRecoverable; RecoverableKinds &= ~Unrecoverable; RecoverableKinds &= Kinds; @@ -710,9 +719,14 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, SanitizerMask MergeKinds = parseSanitizeArgs(D, Args, DiagnoseErrors, MergeDefault, {}, {}, options::OPT_fsanitize_merge_handlers_EQ, - options::OPT_fno_sanitize_merge_handlers_EQ); + options::OPT_fno_sanitize_merge_handlers_EQ, nullptr); MergeKinds &= Kinds; + // Parse -fno-sanitize-top-hot flags + SanitizerMask TopHotMask = + parseNoSanitizeHotArgs(D, Args, DiagnoseErrors, &TopHotCutoffs); + (void)TopHotMask; + // Setup ignorelist files. // Add default ignorelist from resource directory for activated sanitizers, // and validate special case lists format. @@ -1132,6 +1146,15 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, "Overlap between recoverable and trapping sanitizers"); MergeHandlers.Mask |= MergeKinds; + + TopHotMask &= Sanitizers.Mask; + TopHot.Mask = TopHotMask; + + // Zero out TopHot for unused sanitizers + for (unsigned int i = 0; i < SanitizerKind::SO_Count; i++) { + if (!(Sanitizers.Mask & SanitizerMask::bitPosToMask(i))) + TopHotCutoffs[i] = 0; + } } static std::string toString(const clang::SanitizerSet &Sanitizers) { @@ -1146,6 +1169,19 @@ static std::string toString(const clang::SanitizerSet &Sanitizers) { return Res; } +static std::string toString(const clang::SanitizerMaskCutoffs &Cutoffs) { + std::string Res; +#define SANITIZER(NAME, ID) \ + if (Cutoffs[SanitizerKind::SO_##ID]) { \ + if (!Res.empty()) \ + Res += ","; \ + Res += std::string(NAME) + "=" + \ + std::to_string(Cutoffs[SanitizerKind::SO_##ID]); \ + } +#include "clang/Basic/Sanitizers.def" + return Res; +} + static void addSpecialCaseListOpt(const llvm::opt::ArgList &Args, llvm::opt::ArgStringList &CmdArgs, const char *SCLOptFlag, @@ -1297,6 +1333,11 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args, CmdArgs.push_back( Args.MakeArgString("-fsanitize-merge=" + toString(MergeHandlers))); + std::string TopHotCutoffsStr = toString(TopHotCutoffs); + if (TopHotCutoffsStr != "") + CmdArgs.push_back( + Args.MakeArgString("-fno-sanitize-top-hot=" + TopHotCutoffsStr)); + addSpecialCaseListOpt(Args, CmdArgs, "-fsanitize-ignorelist=", UserIgnorelistFiles); addSpecialCaseListOpt(Args, CmdArgs, @@ -1463,17 +1504,18 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args, } SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A, - bool DiagnoseErrors) { - assert( - (A->getOption().matches(options::OPT_fsanitize_EQ) || - A->getOption().matches(options::OPT_fno_sanitize_EQ) || - A->getOption().matches(options::OPT_fsanitize_recover_EQ) || - A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) || - A->getOption().matches(options::OPT_fsanitize_trap_EQ) || - A->getOption().matches(options::OPT_fno_sanitize_trap_EQ) || - A->getOption().matches(options::OPT_fsanitize_merge_handlers_EQ) || - A->getOption().matches(options::OPT_fno_sanitize_merge_handlers_EQ)) && - "Invalid argument in parseArgValues!"); + bool DiagnoseErrors, + SanitizerMaskCutoffs *Cutoffs) { + assert((A->getOption().matches(options::OPT_fsanitize_EQ) || + A->getOption().matches(options::OPT_fno_sanitize_EQ) || + A->getOption().matches(options::OPT_fsanitize_recover_EQ) || + A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) || + A->getOption().matches(options::OPT_fsanitize_trap_EQ) || + A->getOption().matches(options::OPT_fno_sanitize_trap_EQ) || + A->getOption().matches(options::OPT_fsanitize_merge_handlers_EQ) || + A->getOption().matches(options::OPT_fno_sanitize_merge_handlers_EQ) || + A->getOption().matches(options::OPT_fno_sanitize_top_hot_EQ)) && + "Invalid argument in parseArgValues!"); SanitizerMask Kinds; for (int i = 0, n = A->getNumValues(); i != n; ++i) { const char *Value = A->getValue(i); @@ -1482,8 +1524,15 @@ SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A, if (A->getOption().matches(options::OPT_fsanitize_EQ) && 0 == strcmp("all", Value)) Kind = SanitizerMask(); - else + else if (A->getOption().matches(options::OPT_fno_sanitize_top_hot_EQ)) { + assert( + Cutoffs && + "Null Cutoffs parameter provided for parsing fno_sanitize_top_hot!"); + Kind = parseSanitizerWeightedValue(Value, /*AllowGroups=*/true, Cutoffs); + } else { + assert((!Cutoffs) && "Non-null Cutoffs parameter erroneously provided!"); Kind = parseSanitizerValue(Value, /*AllowGroups=*/true); + } if (Kind) Kinds |= Kind; @@ -1586,12 +1635,12 @@ std::string lastArgumentForMask(const Driver &D, const llvm::opt::ArgList &Args, const auto *Arg = *I; if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) { SanitizerMask AddKinds = - expandSanitizerGroups(parseArgValues(D, Arg, false)); + expandSanitizerGroups(parseArgValues(D, Arg, false, nullptr)); if (AddKinds & Mask) return describeSanitizeArg(Arg, Mask); } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) { SanitizerMask RemoveKinds = - expandSanitizerGroups(parseArgValues(D, Arg, false)); + expandSanitizerGroups(parseArgValues(D, Arg, false, nullptr)); Mask &= ~RemoveKinds; } } diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 348c56cc37da3f..78dd5099259f18 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -1436,6 +1436,19 @@ static SmallVector<StringRef, 4> serializeSanitizerKinds(SanitizerSet S) { return Values; } +static void parseSanitizerWeightedKinds( + StringRef FlagName, const std::vector<std::string> &Sanitizers, + DiagnosticsEngine &Diags, SanitizerSet &S, SanitizerMaskCutoffs *Cutoffs) { + for (const auto &Sanitizer : Sanitizers) { + SanitizerMask K = + parseSanitizerWeightedValue(Sanitizer, /*AllowGroups=*/false, Cutoffs); + if (K == SanitizerMask()) + Diags.Report(diag::err_drv_invalid_value) << FlagName << Sanitizer; + else + S.set(K, true); + } +} + static void parseXRayInstrumentationBundle(StringRef FlagName, StringRef Bundle, ArgList &Args, DiagnosticsEngine &D, XRayInstrSet &S) { @@ -1796,6 +1809,11 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts, serializeSanitizerKinds(Opts.SanitizeMergeHandlers)) GenerateArg(Consumer, OPT_fsanitize_merge_handlers_EQ, Sanitizer); + SmallVector<std::string, 4> Values; + serializeSanitizerMaskCutoffs(Opts.NoSanitizeTopHotCutoffs, Values); + for (std::string Sanitizer : Values) + GenerateArg(Consumer, OPT_fno_sanitize_top_hot_EQ, Sanitizer); + if (!Opts.EmitVersionIdentMetadata) GenerateArg(Consumer, OPT_Qn); @@ -2277,6 +2295,12 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, Args.getAllArgValues(OPT_fsanitize_merge_handlers_EQ), Diags, Opts.SanitizeMergeHandlers); + // Parse -fno-sanitize-top-hot= arguments. + parseSanitizerWeightedKinds("-fno-sanitize-top-hot=", + Args.getAllArgValues(OPT_fno_sanitize_top_hot_EQ), + Diags, Opts.NoSanitizeTopHot, + &Opts.NoSanitizeTopHotCutoffs); + Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true); if (!LangOpts->CUDAIsDevice) diff --git a/clang/test/CodeGen/allow-ubsan-check.c b/clang/test/CodeGen/allow-ubsan-check.c index 5232d240854666..1c76049b57bda8 100644 --- a/clang/test/CodeGen/allow-ubsan-check.c +++ b/clang/test/CodeGen/allow-ubsan-check.c @@ -3,7 +3,6 @@ // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -o - %s -fsanitize=signed-integer-overflow,integer-divide-by-zero,null -mllvm -ubsan-guard-checks -fsanitize-trap=signed-integer-overflow,integer-divide-by-zero,null | FileCheck %s --check-prefixes=TRAP // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -o - %s -fsanitize=signed-integer-overflow,integer-divide-by-zero,null -mllvm -ubsan-guard-checks -fsanitize-recover=signed-integer-overflow,integer-divide-by-zero,null | FileCheck %s --check-prefixes=RECOVER - // CHECK-LABEL: define dso_local i32 @d... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/121619 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
