llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: Yingwei Zheng (dtcxzyw) <details> <summary>Changes</summary> This patch removes unneeded enum `BuiltinCheckKind` and fixes a copy-paste mistake in `__ubsan_handle_invalid_builtin`. Address comment https://github.com/llvm/llvm-project/pull/104741#discussion_r1764323722. --- Full diff: https://github.com/llvm/llvm-project/pull/109088.diff 5 Files Affected: - (modified) clang/lib/CodeGen/CGBuiltin.cpp (+6-14) - (modified) clang/lib/CodeGen/CodeGenFunction.h (+2-9) - (modified) compiler-rt/lib/ubsan/ubsan_handlers.cpp (+2-3) - (modified) compiler-rt/lib/ubsan/ubsan_handlers.h (-8) - (modified) compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp (+7-7) ``````````diff diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index a52e880a764252..7509d0eb097bb6 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -1994,11 +1994,7 @@ struct CallObjCArcUse final : EHScopeStack::Cleanup { }; } -Value *CodeGenFunction::EmitCheckedArgForBuiltin(const Expr *E, - BuiltinCheckKind Kind) { - assert((Kind == BCK_CLZPassedZero || Kind == BCK_CTZPassedZero) - && "Unsupported builtin check kind"); - +Value *CodeGenFunction::EmitCheckedArgForBuiltin(const Expr *E) { Value *ArgValue = EmitScalarExpr(E); if (!SanOpts.has(SanitizerKind::Builtin)) return ArgValue; @@ -2008,9 +2004,7 @@ Value *CodeGenFunction::EmitCheckedArgForBuiltin(const Expr *E, ArgValue, llvm::Constant::getNullValue(ArgValue->getType())); EmitCheck(std::make_pair(Cond, SanitizerKind::Builtin), SanitizerHandler::InvalidBuiltin, - {EmitCheckSourceLocation(E->getExprLoc()), - llvm::ConstantInt::get(Builder.getInt8Ty(), Kind)}, - std::nullopt); + {EmitCheckSourceLocation(E->getExprLoc())}, std::nullopt); return ArgValue; } @@ -3228,9 +3222,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, bool HasFallback = BuiltinIDIfNoAsmLabel == Builtin::BI__builtin_ctzg && E->getNumArgs() > 1; - Value *ArgValue = - HasFallback ? EmitScalarExpr(E->getArg(0)) - : EmitCheckedArgForBuiltin(E->getArg(0), BCK_CTZPassedZero); + Value *ArgValue = HasFallback ? EmitScalarExpr(E->getArg(0)) + : EmitCheckedArgForBuiltin(E->getArg(0)); llvm::Type *ArgType = ArgValue->getType(); Function *F = CGM.getIntrinsic(Intrinsic::cttz, ArgType); @@ -3260,9 +3253,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, bool HasFallback = BuiltinIDIfNoAsmLabel == Builtin::BI__builtin_clzg && E->getNumArgs() > 1; - Value *ArgValue = - HasFallback ? EmitScalarExpr(E->getArg(0)) - : EmitCheckedArgForBuiltin(E->getArg(0), BCK_CLZPassedZero); + Value *ArgValue = HasFallback ? EmitScalarExpr(E->getArg(0)) + : EmitCheckedArgForBuiltin(E->getArg(0)); llvm::Type *ArgType = ArgValue->getType(); Function *F = CGM.getIntrinsic(Intrinsic::ctlz, ArgType); diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 6802dc7f0c1598..69bb58f0e617c5 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -5078,16 +5078,9 @@ class CodeGenFunction : public CodeGenTypeCache { bool IsSubtraction, SourceLocation Loc, CharUnits Align, const Twine &Name = ""); - /// Specifies which type of sanitizer check to apply when handling a - /// particular builtin. - enum BuiltinCheckKind { - BCK_CTZPassedZero, - BCK_CLZPassedZero, - }; - /// Emits an argument for a call to a builtin. If the builtin sanitizer is - /// enabled, a runtime check specified by \p Kind is also emitted. - llvm::Value *EmitCheckedArgForBuiltin(const Expr *E, BuiltinCheckKind Kind); + /// enabled, a runtime zero check is also emitted. + llvm::Value *EmitCheckedArgForBuiltin(const Expr *E); /// Emit a description of a type in a format suitable for passing to /// a runtime sanitizer handler. diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp index 27d01653f088da..ffefbded64ad14 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp +++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp @@ -634,12 +634,11 @@ static void handleInvalidBuiltin(InvalidBuiltinData *Data, ReportOptions Opts) { ScopedReport R(Opts, Loc, ET); Diag(Loc, DL_Error, ET, - "passing zero to %0, which is not a valid argument") - << ((Data->Kind == BCK_CTZPassedZero) ? "ctz()" : "clz()"); + "passing zero to __builtin_ctz/clz, which is not a valid argument"); } void __ubsan::__ubsan_handle_invalid_builtin(InvalidBuiltinData *Data) { - GET_REPORT_OPTIONS(true); + GET_REPORT_OPTIONS(false); handleInvalidBuiltin(Data, Opts); } void __ubsan::__ubsan_handle_invalid_builtin_abort(InvalidBuiltinData *Data) { diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.h b/compiler-rt/lib/ubsan/ubsan_handlers.h index bae661a56833dd..4b37f9916f3da1 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers.h +++ b/compiler-rt/lib/ubsan/ubsan_handlers.h @@ -154,16 +154,8 @@ struct ImplicitConversionData { RECOVERABLE(implicit_conversion, ImplicitConversionData *Data, ValueHandle Src, ValueHandle Dst) -/// Known builtin check kinds. -/// Keep in sync with the enum of the same name in CodeGenFunction.h -enum BuiltinCheckKind : unsigned char { - BCK_CTZPassedZero, - BCK_CLZPassedZero, -}; - struct InvalidBuiltinData { SourceLocation Loc; - unsigned char Kind; }; /// Handle a builtin called in an invalid way. diff --git a/compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp b/compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp index f8f564cb7baae2..2993c7a4ce9933 100644 --- a/compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp +++ b/compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp @@ -6,25 +6,25 @@ // RUN: not %run %t.abort 2>&1 | FileCheck %s --check-prefix=ABORT void check_ctz(int n) { - // ABORT: builtins.cpp:[[@LINE+2]]:17: runtime error: passing zero to ctz(), which is not a valid argument - // RECOVER: builtins.cpp:[[@LINE+1]]:17: runtime error: passing zero to ctz(), which is not a valid argument + // ABORT: builtins.cpp:[[@LINE+2]]:17: runtime error: passing zero to __builtin_ctz/clz, which is not a valid argument + // RECOVER: builtins.cpp:[[@LINE+1]]:17: runtime error: passing zero to __builtin_ctz/clz, which is not a valid argument __builtin_ctz(n); - // RECOVER: builtins.cpp:[[@LINE+1]]:18: runtime error: passing zero to ctz(), which is not a valid argument + // RECOVER: builtins.cpp:[[@LINE+1]]:18: runtime error: passing zero to __builtin_ctz/clz, which is not a valid argument __builtin_ctzl(n); - // RECOVER: builtins.cpp:[[@LINE+1]]:19: runtime error: passing zero to ctz(), which is not a valid argument + // RECOVER: builtins.cpp:[[@LINE+1]]:19: runtime error: passing zero to __builtin_ctz/clz, which is not a valid argument __builtin_ctzll(n); } void check_clz(int n) { - // RECOVER: builtins.cpp:[[@LINE+1]]:17: runtime error: passing zero to clz(), which is not a valid argument + // RECOVER: builtins.cpp:[[@LINE+1]]:17: runtime error: passing zero to __builtin_ctz/clz, which is not a valid argument __builtin_clz(n); - // RECOVER: builtins.cpp:[[@LINE+1]]:18: runtime error: passing zero to clz(), which is not a valid argument + // RECOVER: builtins.cpp:[[@LINE+1]]:18: runtime error: passing zero to __builtin_ctz/clz, which is not a valid argument __builtin_clzl(n); - // RECOVER: builtins.cpp:[[@LINE+1]]:19: runtime error: passing zero to clz(), which is not a valid argument + // RECOVER: builtins.cpp:[[@LINE+1]]:19: runtime error: passing zero to __builtin_ctz/clz, which is not a valid argument __builtin_clzll(n); } `````````` </details> https://github.com/llvm/llvm-project/pull/109088 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits