llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Justin Stitt (JustinStitt) <details> <summary>Changes</summary> [Related RFC](https://discourse.llvm.org/t/rfc-support-globpattern-add-operator-to-invert-matches/80683/5?u=justinstitt) ### Summary Implement type-based filtering via [Sanitizer Special Case Lists](https://clang.llvm.org/docs/SanitizerSpecialCaseList.html) for the arithmetic overflow and truncation sanitizers. Currently, using the `type:` prefix with these sanitizers does nothing. I've hooked up the SSCL parsing with Clang codegen so that we don't emit the overflow/truncation checks if the arithmetic contains an ignored type. ### Usefulness You can craft ignorelists that ignore specific types that are expected to overflow or wrap-around. For example, to ignore `my_type` from `unsigned-integer-overflow` instrumentation: ```bash $ cat ignorelist.txt [unsigned-integer-overflow] type:my_type $ cat foo.c typedef int my_type; void foo() { my_type a = 2147483647; ++a; } $ clang foo.c -fsanitize=signed-integer-overflow -fsanitize-ignorelist=ignorelist.txt ; ./a.out // --> no sanitizer error ``` If a type is functionally intended to overflow, like [refcount_t](https://kernsec.org/wiki/index.php/Kernel_Protections/refcount_t) and its associated APIs in the Linux kernel, then this type filtering would prove useful for reducing sanitizer noise. Currently, the Linux kernel dealt with this by [littering](https://elixir.bootlin.com/linux/v6.10.8/source/include/linux/refcount.h#L139 ) `__attribute__((no_sanitize("signed-integer-overflow")))` annotations on all the `refcount_t` APIs. I think this serves as an example of how a codebase could be made cleaner. We could make custom types that are filtered out in an ignorelist, allowing for types to be more expressive -- without the need for annotations. This accomplishes a similar goal to https://github.com/llvm/llvm-project/pull/86618. Yet another use case for this type filtering is whitelisting. We could ignore _all_ types, save a few. ```bash $ cat ignorelist.txt [implicit-signed-integer-truncation] type:* # ignore literally all types type:short=skip # except `short` $ cat bar.c // compile with -fsanitize=implicit-signed-integer-truncation void bar(int toobig) { char a = toobig; // not instrumented short b = toobig; // instrumented } ``` ### Other ways to accomplish the goal of sanitizer allowlisting/whitelisting * ignore list SSCL type support (this PR that you're reading) * [my sanitize-allowlist branch](https://github.com/llvm/llvm-project/compare/main...JustinStitt:llvm-project:sanitize-allowlist) - this just implements a sibling flag `-fsanitize-allowlist=`, removing some of the double negative logic present with `skip`/`ignore` when trying to whitelist something. * [Glob Negation](https://discourse.llvm.org/t/rfc-support-globpattern-add-operator-to-invert-matches/80683) - Implement a negation operator to the GlobPattern class so the ignorelist query can use them to simulate allowlisting Please let me know which of the three options we like best. They are not necessarily mutually exclusive. Here's [another related PR](https://github.com/llvm/llvm-project/pull/86618) which implements a `wraps` attribute. This can accomplish a similar goal to this PR but requires in-source changes to codebases and also covers a wider variety of integer definedness problems. ### CCs @<!-- -->kees @<!-- -->vitalybuka @<!-- -->bwendling --- Full diff: https://github.com/llvm/llvm-project/pull/107332.diff 6 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+8) - (modified) clang/docs/SanitizerSpecialCaseList.rst (+60-2) - (modified) clang/include/clang/AST/ASTContext.h (+3) - (modified) clang/lib/AST/ASTContext.cpp (+31) - (modified) clang/lib/CodeGen/CGExprScalar.cpp (+32-4) - (modified) clang/test/CodeGen/ubsan-type-ignorelist.cpp (+76-8) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 0eee71d00a2c5f..0f036e90e06c5e 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -540,6 +540,14 @@ Sanitizers This new flag should allow those projects to enable integer sanitizers with less noise. +- Arithmetic overflow sanitizers ``-fsanitize=signed-integer-overflow`` and + ``-fsanitize=unsigned-integer-overflow`` as well as the implicit integer + truncation sanitizers ``-fsanitize=implicit-signed-integer-truncation`` and + ``-fsanitize=implicit-unsigned-integer-truncation`` now properly support the + "type" prefix within `Sanitizer Special Case Lists (SSCL) + <https://clang.llvm.org/docs/SanitizerSpecialCaseList.html>`_. See that link + for examples. + Python Binding Changes ---------------------- - Fixed an issue that led to crashes when calling ``Type.get_exception_specification_kind``. diff --git a/clang/docs/SanitizerSpecialCaseList.rst b/clang/docs/SanitizerSpecialCaseList.rst index c7fb0fa3f8a828..13f71b878d4cc8 100644 --- a/clang/docs/SanitizerSpecialCaseList.rst +++ b/clang/docs/SanitizerSpecialCaseList.rst @@ -15,8 +15,9 @@ file at compile-time. Goal and usage ============== -Users of sanitizer tools, such as :doc:`AddressSanitizer`, :doc:`ThreadSanitizer` -or :doc:`MemorySanitizer` may want to disable or alter some checks for +Users of sanitizer tools, such as :doc:`AddressSanitizer`, +:doc:`ThreadSanitizer`, :doc:`MemorySanitizer` or :doc: +`UndefinedBehaviorSanitizer` may want to disable or alter some checks for certain source-level entities to: * speedup hot function, which is known to be correct; @@ -48,6 +49,63 @@ Example $ clang -fsanitize=address -fsanitize-ignorelist=ignorelist.txt foo.c ; ./a.out # No error report here. +Usage with UndefinedBehaviorSanitizer +===================================== + +The arithmetic overflow sanitizers ``unsigned-integer-overflow`` and +``signed-integer-overflow`` as well as the implicit integer truncation +sanitizers ``implicit-signed-integer-truncation`` and +``implicit-unsigned-integer-truncation`` support the ability to adjust +instrumentation based on type. + +.. code-block:: bash + + $ cat foo.c + void foo() { + int a = 2147483647; // INT_MAX + ++a; // Normally, an overflow with -fsanitize=signed-integer-overflow + } + $ cat ignorelist.txt + [signed-integer-overflow] + type:int + $ clang -fsanitize=signed-integer-overflow -fsanitize-ignorelist=ignorelist.txt foo.c ; ./a.out + # no signed-integer-overflow error + +Supplying ``ignorelist.txt`` with ``-fsanitize-ignorelist=ignorelist.txt`` +disables overflow sanitizer instrumentation for arithmetic operations +containing values of type ``int``, for example. Custom types may be used. + +The following SCL categories are supported: ``=allow``, ``=skip`` and +``=forbid``. The ``allow`` category is the default for any entry and specifies +that the query, if matched, will have its sanitizer instrumentation ignored. +Conversely, both ``skip`` and ``forbid`` cause their queries, if matched, to be +left out of the ignorelist -- essentially ensuring sanitizer instrumentation +remains for those types. This is useful for whitelisting specific types. + +With this, one may disable instrumentation for all types and specifically allow +instrumentation for one or many types. + +.. code-block:: bash + + $ cat ignorelist.txt + [implicit-signed-integer-truncation] + type:*=allow + type:T=skip + $ cat foo.c + typedef char T; + typedef char U; + void foo(int toobig) { + T a = toobig; // instrumented + U b = toobig; // not instrumented + char c = toobig; // also not instrumented + } + +Note that ``skip`` and ``forbid`` operate exactly the same in this context and +both options exist simply for conformity with the `-fprofile-list +<https://clang.llvm.org/docs/UsersManual.html#instrumenting-only-selected-files-or-functions>`_ +syntax for adjusting profile instrumentation. You do not need to specify any +`default:<type>` for ``-fsanitize-ignorelist`` SSCLs, though. + Format ====== diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 89bb5768dbd40d..a9373c6102c010 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -807,6 +807,9 @@ class ASTContext : public RefCountedBase<ASTContext> { const NoSanitizeList &getNoSanitizeList() const { return *NoSanitizeL; } + bool isTypeIgnoredBySanitizer(const SanitizerMask &Mask, + const QualType &Ty) const; + const XRayFunctionFilter &getXRayFilter() const { return *XRayFilter; } diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index c61234aa4d1af1..bcda5fad585d67 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -817,6 +817,37 @@ ASTContext::getCanonicalTemplateTemplateParmDecl( return CanonTTP; } +/// Check if a type can have its sanitizer instrumentation elided. +/// Determine this by its presence in a SCL alongside its specified categories. +/// For example: +/// ignorelist.txt> +/// [{unsigned-integer-overflow,signed-integer-overflow}] +/// type:* +/// type:size_t=skip +/// <ignorelist.txt +/// Supplying the above ignorelist.txt will disable overflow sanitizer +/// instrumentation for all types except "size_t". +bool ASTContext::isTypeIgnoredBySanitizer(const SanitizerMask &Mask, + const QualType &Ty) const { + // One may specifically allow a type "type:foo=allow" + bool isAllowedBySCL = + NoSanitizeL->containsType(Mask, Ty.getAsString(), "allow"); + + // There could also be no category present "type:foo", which is the same as + // "allow" + isAllowedBySCL |= NoSanitizeL->containsType(Mask, Ty.getAsString()); + + // Explicitly specifying "skip" is also possible "type:foo=skip" + bool isSkippedBySCL = + NoSanitizeL->containsType(Mask, Ty.getAsString(), "skip"); + + // Or "forbid", as there is currently no distinction between "skip" and + // "forbid" for the purposes of the overflow/truncation sanitizer ignorelist. + isSkippedBySCL |= NoSanitizeL->containsType(Mask, Ty.getAsString(), "forbid"); + + return isAllowedBySCL && !isSkippedBySCL; +} + TargetCXXABI::Kind ASTContext::getCXXABIKind() const { auto Kind = getTargetInfo().getCXXABI().getKind(); return getLangOpts().CXXABI.value_or(Kind); diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index af11bc20a3b639..21e932a3639f73 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -196,6 +196,18 @@ static bool CanElideOverflowCheck(const ASTContext &Ctx, const BinOpInfo &Op) { if (!Op.mayHaveIntegerOverflow()) return true; + if (Op.Ty->isSignedIntegerType() && + Ctx.isTypeIgnoredBySanitizer(SanitizerKind::SignedIntegerOverflow, + Op.Ty)) { + return true; + } + + if (Op.Ty->isUnsignedIntegerType() && + Ctx.isTypeIgnoredBySanitizer(SanitizerKind::UnsignedIntegerOverflow, + Op.Ty)) { + return true; + } + const UnaryOperator *UO = dyn_cast<UnaryOperator>(Op.E); if (UO && UO->getOpcode() == UO_Minus && @@ -1120,6 +1132,10 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value *Src, QualType SrcType, if (!CGF.SanOpts.has(Check.second.second)) return; + // Does some SSCL ignore this type? + if (CGF.getContext().isTypeIgnoredBySanitizer(Check.second.second, DstType)) + return; + llvm::Constant *StaticArgs[] = { CGF.EmitCheckSourceLocation(Loc), CGF.EmitCheckTypeDescriptor(SrcType), CGF.EmitCheckTypeDescriptor(DstType), @@ -1230,6 +1246,15 @@ void ScalarExprEmitter::EmitIntegerSignChangeCheck(Value *Src, QualType SrcType, // Because here sign change check is interchangeable with truncation check. return; } + // Does an SSCL have an entry for the DstType under its respective sanitizer + // section? + if (DstSigned && CGF.getContext().isTypeIgnoredBySanitizer( + SanitizerKind::ImplicitSignedIntegerTruncation, DstType)) + return; + if (!DstSigned && + CGF.getContext().isTypeIgnoredBySanitizer( + SanitizerKind::ImplicitUnsignedIntegerTruncation, DstType)) + return; // That's it. We can't rule out any more cases with the data we have. CodeGenFunction::SanitizerScope SanScope(&CGF); @@ -2770,10 +2795,11 @@ llvm::Value *ScalarExprEmitter::EmitIncDecConsiderOverflowBehavior( return Builder.CreateNSWAdd(InVal, Amount, Name); [[fallthrough]]; case LangOptions::SOB_Trapping: - if (!E->canOverflow()) + BinOpInfo Info = createBinOpInfoFromIncDec( + E, InVal, IsInc, E->getFPFeaturesInEffect(CGF.getLangOpts())); + if (!E->canOverflow() || CanElideOverflowCheck(CGF.getContext(), Info)) return Builder.CreateNSWAdd(InVal, Amount, Name); - return EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec( - E, InVal, IsInc, E->getFPFeaturesInEffect(CGF.getLangOpts()))); + return EmitOverflowCheckedBinOp(Info); } llvm_unreachable("Unknown SignedOverflowBehaviorTy"); } @@ -2973,7 +2999,9 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV, value = EmitIncDecConsiderOverflowBehavior(E, value, isInc); } else if (E->canOverflow() && type->isUnsignedIntegerType() && CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) && - !excludeOverflowPattern) { + !excludeOverflowPattern && + !CGF.getContext().isTypeIgnoredBySanitizer( + SanitizerKind::UnsignedIntegerOverflow, E->getType())) { value = EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec( E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts()))); } else { diff --git a/clang/test/CodeGen/ubsan-type-ignorelist.cpp b/clang/test/CodeGen/ubsan-type-ignorelist.cpp index 7bea84f5fabb93..e8b14d5c415222 100644 --- a/clang/test/CodeGen/ubsan-type-ignorelist.cpp +++ b/clang/test/CodeGen/ubsan-type-ignorelist.cpp @@ -1,7 +1,28 @@ +// Verify ubsan doesn't emit checks for ignorelisted types +// RUN: echo "[{unsigned-integer-overflow,signed-integer-overflow}]" > %t-int.ignorelist +// RUN: echo "type:int" >> %t-int.ignorelist +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-ignorelist=%t-int.ignorelist -emit-llvm %s -o - | FileCheck %s --check-prefix=INT + +// RUN: echo "type:int" > %t-nosection.ignorelist +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-ignorelist=%t-nosection.ignorelist -emit-llvm %s -o - | FileCheck %s --check-prefix=INT + +// RUN: echo "type:int=allow" > %t-allow-same-as-no-category.ignorelist +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-ignorelist=%t-allow-same-as-no-category.ignorelist -emit-llvm %s -o - | FileCheck %s --check-prefix=INT + +// RUN: echo "[{unsigned-integer-overflow,signed-integer-overflow}]" > %t-myty.ignorelist +// RUN: echo "type:*" >> %t-myty.ignorelist +// RUN: echo "type:myty=skip" >> %t-myty.ignorelist +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-ignorelist=%t-myty.ignorelist -emit-llvm %s -o - | FileCheck %s --check-prefix=MYTY + +// RUN: echo "[{implicit-signed-integer-truncation,implicit-unsigned-integer-truncation}]" > %t-trunc.ignorelist +// RUN: echo "type:char" >> %t-trunc.ignorelist +// RUN: echo "type:unsigned char" >> %t-trunc.ignorelist +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=implicit-signed-integer-truncation,implicit-unsigned-integer-truncation -fsanitize-ignorelist=%t-trunc.ignorelist -emit-llvm %s -o - | FileCheck %s --check-prefix=TRUNC + // Verify ubsan vptr does not check down-casts on ignorelisted types. // RUN: echo "type:_ZTI3Foo" > %t-type.ignorelist -// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=vptr -fsanitize-recover=vptr -emit-llvm %s -o - | FileCheck %s --check-prefix=DEFAULT -// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=vptr -fsanitize-recover=vptr -fsanitize-ignorelist=%t-type.ignorelist -emit-llvm %s -o - | FileCheck %s --check-prefix=TYPE +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=vptr -fsanitize-recover=vptr -emit-llvm %s -o - | FileCheck %s --check-prefix=VPTR +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=vptr -fsanitize-recover=vptr -fsanitize-ignorelist=%t-type.ignorelist -emit-llvm %s -o - | FileCheck %s --check-prefix=VPTR-TYPE class Bar { public: @@ -11,13 +32,60 @@ class Foo : public Bar {}; Bar bar; -// DEFAULT: @_Z7checkmev -// TYPE: @_Z7checkmev +// VPTR: @_Z7checkmev +// VPTR-TYPE: @_Z7checkmev void checkme() { -// DEFAULT: call void @__ubsan_handle_dynamic_type_cache_miss({{.*}} (ptr @bar to -// TYPE-NOT: @__ubsan_handle_dynamic_type_cache_miss +// VPTR: call void @__ubsan_handle_dynamic_type_cache_miss({{.*}} (ptr @bar to +// VPTR-TYPE-NOT: @__ubsan_handle_dynamic_type_cache_miss Foo* foo = static_cast<Foo*>(&bar); // down-casting -// DEFAULT: ret void -// TYPE: ret void +// VPTR: ret void +// VPTR-TYPE: ret void return; } + +// INT-LABEL: ignore_int +void ignore_int(int A, int B, unsigned C, unsigned D, long E) { +// INT: llvm.uadd.with.overflow.i32 + (void)(C+D); +// INT-NOT: llvm.sadd.with.overflow.i32 + (void)(A+B); +// INT: llvm.sadd.with.overflow.i64 + (void)(++E); +} + + +typedef unsigned long myty; +typedef myty derivative; +// INT-LABEL: ignore_all_except_myty +// MYTY-LABEL: ignore_all_except_myty +void ignore_all_except_myty(myty A, myty B, int C, unsigned D, derivative E) { +// MYTY-NOT: llvm.sadd.with.overflow.i32 + (void)(++C); + +// MYTY-NOT: llvm.uadd.with.overflow.i32 + (void)(D+D); + +// MYTY-NOT: llvm.umul.with.overflow.i64 + (void)(E*2); + +// MYTY: llvm.uadd.with.overflow.i64 + (void)(A+B); +} + +// INT-LABEL: truncation +// MYTY-LABEL: truncation +// TRUNC-LABEL: truncation +void truncation(char A, int B, unsigned char C, short D) { +// TRUNC-NOT: %handler.implicit_conversion + A = B; +// TRUNC-NOT: %handler.implicit_conversion + A = C; +// TRUNC-NOT: %handler.implicit_conversion + C = B; + +// TRUNC: %handler.implicit_conversion + D = B; + + (void)A; + (void)D; +} `````````` </details> https://github.com/llvm/llvm-project/pull/107332 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
