https://github.com/ahatanak updated https://github.com/llvm/llvm-project/pull/79230
>From 95200f3bb3859738981240a9d8c503a13ede9601 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Tue, 16 Jan 2024 13:18:09 -0800 Subject: [PATCH 01/18] Add support for builtin_verbose_trap The builtin causes the program to stop its execution abnormally and shows a human-readable description of the reason for the termination when a debugger is attached or in a symbolicated crash log. The motivation for the builtin is explained in the following RFC: https://discourse.llvm.org/t/rfc-adding-builtin-verbose-trap-string-literal/75845 --- clang/docs/LanguageExtensions.rst | 49 ++++++++++++++++++- clang/include/clang/AST/Expr.h | 5 ++ clang/include/clang/Basic/Builtins.td | 6 +++ .../clang/Basic/DiagnosticSemaKinds.td | 2 + clang/lib/AST/ExprConstant.cpp | 18 +++++-- clang/lib/CodeGen/CGBuiltin.cpp | 12 +++++ clang/lib/CodeGen/CGDebugInfo.cpp | 42 ++++++++++++++++ clang/lib/CodeGen/CGDebugInfo.h | 20 ++++++++ clang/lib/Sema/SemaChecking.cpp | 22 +++++++++ .../CodeGenCXX/debug-info-verbose-trap.cpp | 49 +++++++++++++++++++ clang/test/SemaCXX/verbose-trap.cpp | 28 +++++++++++ 11 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 clang/test/CodeGenCXX/debug-info-verbose-trap.cpp create mode 100644 clang/test/SemaCXX/verbose-trap.cpp diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index 84fc4dee02fa8..1dd8d3bcec920 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -3467,6 +3467,54 @@ Query for this feature with ``__has_builtin(__builtin_trap)``. ``__builtin_arm_trap`` is lowered to the ``llvm.aarch64.break`` builtin, and then to ``brk #payload``. +``__builtin_verbose_trap`` +-------------------------- + +``__builtin_verbose_trap`` causes the program to stop its execution abnormally +and shows a human-readable description of the reason for the termination when a +debugger is attached or in a symbolicated crash log. + +**Syntax**: + +.. code-block:: c++ + + __builtin_verbose_trap(const char *reason) + +**Description** + +``__builtin_verbose_trap`` is lowered to the ` ``llvm.trap`` <https://llvm.org/docs/LangRef.html#llvm-trap-intrinsic>`_ builtin. +Additionally, clang emits debug metadata that represents an artificial inline +frame whose name encodes the string passed to the builtin, prefixed by a "magic" +prefix. + +For example, consider the following code: + +.. code-block:: c++ + + void foo(int* p) { + if (p == nullptr) + __builtin_verbose_trap("Argument_must_not_be_null"); + } + +The debug metadata would look as if it were produced for the following code: + +.. code-block:: c++ + + __attribute__((always_inline)) + inline void "__llvm_verbose_trap: Argument_must_not_be_null"() { + __builtin_trap(); + } + + void foo(int* p) { + if (p == nullptr) + "__llvm_verbose_trap: Argument_must_not_be_null"(); + } + +However, the LLVM IR would not actually contain a call to the artificial +function — it only exists in the debug metadata. + +Query for this feature with ``__has_builtin(__builtin_verbose_trap)``. + ``__builtin_allow_runtime_check`` --------------------------------- @@ -3514,7 +3562,6 @@ guarded check. It's unused now. It will enable kind-specific lowering in future. E.g. a higher hotness cutoff can be used for more expensive kind of check. Query for this feature with ``__has_builtin(__builtin_allow_runtime_check)``. - ``__builtin_nondeterministic_value`` ------------------------------------ diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 2bfefeabc348b..9606a5fddd47e 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -787,6 +787,11 @@ class Expr : public ValueStmt { const Expr *PtrExpression, ASTContext &Ctx, EvalResult &Status) const; + /// If the current Expr can be evaluated to a pointer to a null-terminated + /// constant string, return the constant string (without the terminating null) + /// in Result. Return true if it succeeds. + bool tryEvaluateString(std::string &Result, ASTContext &Ctx) const; + /// Enumeration used to describe the kind of Null pointer constant /// returned from \c isNullPointerConstant(). enum NullPointerConstantKind { diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td index de721a87b3341..5b3b9d04e576a 100644 --- a/clang/include/clang/Basic/Builtins.td +++ b/clang/include/clang/Basic/Builtins.td @@ -1152,6 +1152,12 @@ def Trap : Builtin { let Prototype = "void()"; } +def VerboseTrap : Builtin { + let Spellings = ["__builtin_verbose_trap"]; + let Attributes = [NoThrow, NoReturn]; + let Prototype = "void(char const*)"; +} + def Debugtrap : Builtin { let Spellings = ["__builtin_debugtrap"]; let Attributes = [NoThrow]; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 63e951daec747..417a24564cd00 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8875,6 +8875,8 @@ def err_expected_callable_argument : Error< "expected a callable expression as %ordinal0 argument to %1, found %2">; def note_building_builtin_dump_struct_call : Note< "in call to printing function with arguments '(%0)' while dumping struct">; +def err_builtin_verbose_trap_arg : Error< + "argument to __builtin_verbose_trap must be a non-empty string literal">; def err_atomic_load_store_uses_lib : Error< "atomic %select{load|store}0 requires runtime support that is not " diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index de3c2a63913e9..c5ac10f2013dc 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -1884,7 +1884,8 @@ static bool EvaluateAtomic(const Expr *E, const LValue *This, APValue &Result, EvalInfo &Info); static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result); static bool EvaluateBuiltinStrLen(const Expr *E, uint64_t &Result, - EvalInfo &Info); + EvalInfo &Info, + std::string *StringResult = nullptr); /// Evaluate an integer or fixed point expression into an APResult. static bool EvaluateFixedPointOrInteger(const Expr *E, APFixedPoint &Result, @@ -16792,7 +16793,7 @@ bool Expr::tryEvaluateObjectSize(uint64_t &Result, ASTContext &Ctx, } static bool EvaluateBuiltinStrLen(const Expr *E, uint64_t &Result, - EvalInfo &Info) { + EvalInfo &Info, std::string *StringResult) { if (!E->getType()->hasPointerRepresentation() || !E->isPRValue()) return false; @@ -16819,6 +16820,8 @@ static bool EvaluateBuiltinStrLen(const Expr *E, uint64_t &Result, Str = Str.substr(0, Pos); Result = Str.size(); + if (StringResult) + *StringResult = Str; return true; } @@ -16834,12 +16837,21 @@ static bool EvaluateBuiltinStrLen(const Expr *E, uint64_t &Result, if (!Char.getInt()) { Result = Strlen; return true; - } + } else if (StringResult) + StringResult->push_back(Char.getInt().getExtValue()); if (!HandleLValueArrayAdjustment(Info, E, String, CharTy, 1)) return false; } } +bool Expr::tryEvaluateString(std::string &StringResult, ASTContext &Ctx) const { + Expr::EvalStatus Status; + EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantFold); + uint64_t Result; + + return EvaluateBuiltinStrLen(this, Result, Info, &StringResult); +} + bool Expr::EvaluateCharRangeAsString(std::string &Result, const Expr *SizeExpression, const Expr *PtrExpression, ASTContext &Ctx, diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 7e5f2edfc732c..ad19b4d24be5a 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -3586,6 +3586,18 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, case Builtin::BI__builtin_trap: EmitTrapCall(Intrinsic::trap); return RValue::get(nullptr); + case Builtin::BI__builtin_verbose_trap: { + llvm::DILocation *TrapLocation = Builder.getCurrentDebugLocation(); + if (getDebugInfo()) { + std::string Str; + E->getArg(0)->tryEvaluateString(Str, getContext()); + TrapLocation = getDebugInfo()->CreateTrapFailureMessageFor( + TrapLocation, "__llvm_verbose_trap", Str); + } + ApplyDebugLocation ApplyTrapDI(*this, TrapLocation); + EmitTrapCall(Intrinsic::trap); + return RValue::get(nullptr); + } case Builtin::BI__debugbreak: EmitTrapCall(Intrinsic::debugtrap); return RValue::get(nullptr); diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 539ded5cca5e1..b42489746fdc6 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -1692,6 +1692,27 @@ llvm::DIType *CGDebugInfo::createFieldType( offsetInBits, flags, debugType, Annotations); } +llvm::DISubprogram * +CGDebugInfo::getFakeFuncSubprogram(const std::string &FakeFuncName) { + llvm::DISubprogram *&SP = FakeFuncMap[FakeFuncName]; + + if (!SP) { + auto FileScope = TheCU->getFile(); + llvm::DISubroutineType *DIFnTy = DBuilder.createSubroutineType(nullptr); + // Note: We use `FileScope` rather than `TheCU` as the scope because that's + // what LLVM's inliner seems to do. + SP = DBuilder.createFunction( + /*Scope=*/FileScope, /*Name=*/FakeFuncName, /*LinkageName=*/StringRef(), + /*File=*/FileScope, /*LineNo=*/0, /*Ty=*/DIFnTy, + /*ScopeLine=*/0, + /*Flags=*/llvm::DINode::FlagArtificial, + /*SPFlags=*/llvm::DISubprogram::SPFlagDefinition, + /*TParams=*/nullptr, /*ThrownTypes=*/nullptr, /*Annotations=*/nullptr); + } + + return SP; +} + void CGDebugInfo::CollectRecordLambdaFields( const CXXRecordDecl *CXXDecl, SmallVectorImpl<llvm::Metadata *> &elements, llvm::DIType *RecordTy) { @@ -3488,6 +3509,27 @@ llvm::DIMacroFile *CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent, return DBuilder.createTempMacroFile(Parent, Line, FName); } +llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor( + llvm::DebugLoc TrapLocation, StringRef Prefix, StringRef FailureMsg) { + // Create debug info that describes a fake function whose name is the failure + // message. + std::string FuncName(Prefix); + if (!FailureMsg.empty()) { + // A space in the function name identifies this as not being a real function + // because it's not a valid symbol name. + FuncName += ": "; + FuncName += FailureMsg; + } + + assert(FuncName.size() > 0); + assert(FuncName.find(' ') != std::string::npos && + "Fake function name must contain a space"); + + llvm::DISubprogram *TrapSP = getFakeFuncSubprogram(FuncName); + return llvm::DILocation::get(CGM.getLLVMContext(), /*Line=*/0, /*Column=*/0, + /*Scope=*/TrapSP, /*InlinedAt=*/TrapLocation); +} + static QualType UnwrapTypeForDebugInfo(QualType T, const ASTContext &C) { Qualifiers Quals; do { diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index d6db4d711366a..217b99d448f75 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -346,6 +346,14 @@ class CGDebugInfo { const FieldDecl *BitFieldDecl, const llvm::DIDerivedType *BitFieldDI, llvm::ArrayRef<llvm::Metadata *> PreviousFieldsDI, const RecordDecl *RD); + // A cache that maps fake function names used for __builtin_verbose_trap to + // subprograms. + std::map<std::string, llvm::DISubprogram *> FakeFuncMap; + + // A function that returns the subprogram corresponding to the fake function + // name. + llvm::DISubprogram *getFakeFuncSubprogram(const std::string &FakeFuncName); + /// Helpers for collecting fields of a record. /// @{ void CollectRecordLambdaFields(const CXXRecordDecl *CXXDecl, @@ -602,6 +610,18 @@ class CGDebugInfo { return CoroutineParameterMappings; } + // Create a debug location from `TrapLocation` that adds a fake + // inline frame where the frame name is + // + // * `<Prefix>: <FailureMsg>` if `<FailureMsg>` is not empty. + // * `<Prefix>` if `<FailureMsg>` is empty. Note `<Prefix>` must + // contain a space. + // + // This is used to store failure reasons for traps. + llvm::DILocation *CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, + StringRef Prefix, + StringRef FailureMsg); + private: /// Emit call to llvm.dbg.declare for a variable declaration. /// Returns a pointer to the DILocalVariable associated with the diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 51757f4cf727d..fc8ff6bc340cc 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -172,6 +172,23 @@ static bool checkArgCount(Sema &S, CallExpr *Call, unsigned DesiredArgCount) { << /*is non object*/ 0 << Call->getArg(1)->getSourceRange(); } +static bool checkBuiltinVerboseTrap(CallExpr *Call, Sema &S) { + Expr *Arg = Call->getArg(0); + + if (Arg->isValueDependent()) + return true; + + // FIXME: Add more checks and reject strings that can't be handled by + // debuggers. + std::string Result; + if (!Arg->tryEvaluateString(Result, S.Context) || Result.empty()) { + S.Diag(Arg->getBeginLoc(), diag::err_builtin_verbose_trap_arg) + << Arg->getSourceRange(); + return false; + } + return true; +} + static bool convertArgumentToType(Sema &S, Expr *&Value, QualType Ty) { if (Value->isTypeDependent()) return false; @@ -3204,6 +3221,11 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID, case Builtin::BI__builtin_matrix_column_major_store: return BuiltinMatrixColumnMajorStore(TheCall, TheCallResult); + case Builtin::BI__builtin_verbose_trap: + if (!checkBuiltinVerboseTrap(TheCall, *this)) + return ExprError(); + break; + case Builtin::BI__builtin_get_device_side_mangled_name: { auto Check = [](CallExpr *TheCall) { if (TheCall->getNumArgs() != 1) diff --git a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp new file mode 100644 index 0000000000000..04fa68ae93ecb --- /dev/null +++ b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp @@ -0,0 +1,49 @@ +// RUN: %clang_cc1 -std=c++20 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s + +// CHECK-LABEL: define void @_Z2f0v() +// CHECK: call void @llvm.trap(), !dbg ![[LOC17:.*]] + +// CHECK-LABEL: define void @_Z2f1v() +// CHECK: call void @llvm.trap(), !dbg ![[LOC23:.*]] +// CHECK: call void @llvm.trap(), !dbg ![[LOC25:.*]] + +// CHECK-LABEL: define void @_Z2f3v() +// CHECK: call void @_Z2f2IXadsoKcL_ZL8constMsgEEEEvv() + +// CHECK-LABEL: define internal void @_Z2f2IXadsoKcL_ZL8constMsgEEEEvv() +// CHECK: call void @llvm.trap(), !dbg ![[LOC36:.*]] + +// CHECK: ![[FILESCOPE:.*]] = !DIFile(filename: +// CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: "_Z2f0v", +// CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], inlinedAt: ![[LOC20:.*]]) +// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap: Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[LOC20]] = !DILocation(line: 34, column: 3, scope: ![[SUBPROG14]]) +// CHECK: ![[SUBPROG22:.*]] = distinct !DISubprogram(name: "f1", linkageName: "_Z2f1v", +// CHECK: ![[LOC23]] = !DILocation(line: 0, scope: ![[SUBPROG18]], inlinedAt: ![[LOC24:.*]]) +// CHECK: ![[LOC24]] = !DILocation(line: 38, column: 3, scope: ![[SUBPROG22]]) +// CHECK: ![[LOC25]] = !DILocation(line: 0, scope: ![[SUBPROG26:.*]], inlinedAt: ![[LOC27:.*]]) +// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__llvm_verbose_trap: hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[LOC27]] = !DILocation(line: 39, column: 3, scope: ![[SUBPROG22]]) +// CHECK: ![[SUBPROG32:.*]] = distinct !DISubprogram(name: "f2<&constMsg[0]>", linkageName: "_Z2f2IXadsoKcL_ZL8constMsgEEEEvv", +// CHECK: ![[LOC36]] = !DILocation(line: 0, scope: ![[SUBPROG26]], inlinedAt: ![[LOC37:.*]]) +// CHECK: ![[LOC37]] = !DILocation(line: 44, column: 3, scope: ![[SUBPROG32]]) + +char const constMsg[] = "hello"; + +void f0() { + __builtin_verbose_trap("Argument_must_not_be_null"); +} + +void f1() { + __builtin_verbose_trap("Argument_must_not_be_null"); + __builtin_verbose_trap("hello"); +} + +template <const char * const str> +void f2() { + __builtin_verbose_trap(str); +} + +void f3() { + f2<constMsg>(); +} diff --git a/clang/test/SemaCXX/verbose-trap.cpp b/clang/test/SemaCXX/verbose-trap.cpp new file mode 100644 index 0000000000000..7c152325d3081 --- /dev/null +++ b/clang/test/SemaCXX/verbose-trap.cpp @@ -0,0 +1,28 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -verify %s + +#if !__has_builtin(__builtin_verbose_trap) +#error +#endif + +constexpr char const* constMsg1 = "hello"; +char const* const constMsg2 = "hello"; +char const constMsg3[] = "hello"; + +template <const char * const str> +void f(const char * arg) { + __builtin_verbose_trap("Argument_must_not_be_null"); + __builtin_verbose_trap("hello" "world"); + __builtin_verbose_trap(constMsg1); + __builtin_verbose_trap(constMsg2); + __builtin_verbose_trap(""); // expected-error {{argument to __builtin_verbose_trap must be a non-empty string literal}} + __builtin_verbose_trap(); // expected-error {{too few arguments}} + __builtin_verbose_trap(0); // expected-error {{argument to __builtin_verbose_trap must be a non-empty string literal}} + __builtin_verbose_trap(1); // expected-error {{cannot initialize a parameter of type 'const char *' with}} + __builtin_verbose_trap(arg); // expected-error {{argument to __builtin_verbose_trap must be a non-empty string literal}} + __builtin_verbose_trap(str); + __builtin_verbose_trap(u8"hello"); +} + +void test() { + f<constMsg3>(nullptr); +} >From 156e9debc3933b931befa5938e54ae832739fd75 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Tue, 23 Jan 2024 16:13:14 -0800 Subject: [PATCH 02/18] Fix diagnostic message --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 3 ++- clang/test/SemaCXX/verbose-trap.cpp | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 417a24564cd00..60e0087c8d4d8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8876,7 +8876,8 @@ def err_expected_callable_argument : Error< def note_building_builtin_dump_struct_call : Note< "in call to printing function with arguments '(%0)' while dumping struct">; def err_builtin_verbose_trap_arg : Error< - "argument to __builtin_verbose_trap must be a non-empty string literal">; + "argument to __builtin_verbose_trap must be a pointer to a non-empty constant " + "string">; def err_atomic_load_store_uses_lib : Error< "atomic %select{load|store}0 requires runtime support that is not " diff --git a/clang/test/SemaCXX/verbose-trap.cpp b/clang/test/SemaCXX/verbose-trap.cpp index 7c152325d3081..2cbfad6c817f8 100644 --- a/clang/test/SemaCXX/verbose-trap.cpp +++ b/clang/test/SemaCXX/verbose-trap.cpp @@ -14,11 +14,11 @@ void f(const char * arg) { __builtin_verbose_trap("hello" "world"); __builtin_verbose_trap(constMsg1); __builtin_verbose_trap(constMsg2); - __builtin_verbose_trap(""); // expected-error {{argument to __builtin_verbose_trap must be a non-empty string literal}} + __builtin_verbose_trap(""); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a non-empty constant string}} __builtin_verbose_trap(); // expected-error {{too few arguments}} - __builtin_verbose_trap(0); // expected-error {{argument to __builtin_verbose_trap must be a non-empty string literal}} + __builtin_verbose_trap(0); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a non-empty constant string}} __builtin_verbose_trap(1); // expected-error {{cannot initialize a parameter of type 'const char *' with}} - __builtin_verbose_trap(arg); // expected-error {{argument to __builtin_verbose_trap must be a non-empty string literal}} + __builtin_verbose_trap(arg); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a non-empty constant string}} __builtin_verbose_trap(str); __builtin_verbose_trap(u8"hello"); } >From 78b1c451ea17defd0ea6719fc4f8d862480a20e9 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Thu, 25 Jan 2024 14:15:16 -0800 Subject: [PATCH 03/18] Address review comments --- clang/docs/LanguageExtensions.rst | 19 +++++++++++-------- clang/lib/CodeGen/CGDebugInfo.cpp | 19 +++++++++---------- clang/lib/CodeGen/CGDebugInfo.h | 16 ++++++++-------- clang/test/SemaCXX/verbose-trap.cpp | 1 + 4 files changed, 29 insertions(+), 26 deletions(-) diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index 1dd8d3bcec920..3acdfd5e6ca84 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -3483,9 +3483,9 @@ debugger is attached or in a symbolicated crash log. **Description** ``__builtin_verbose_trap`` is lowered to the ` ``llvm.trap`` <https://llvm.org/docs/LangRef.html#llvm-trap-intrinsic>`_ builtin. -Additionally, clang emits debug metadata that represents an artificial inline -frame whose name encodes the string passed to the builtin, prefixed by a "magic" -prefix. +Additionally, clang emits debugging information that represents an artificial +inline frame whose name encodes the string passed to the builtin, prefixed by a +"magic" prefix. For example, consider the following code: @@ -3493,27 +3493,30 @@ For example, consider the following code: void foo(int* p) { if (p == nullptr) - __builtin_verbose_trap("Argument_must_not_be_null"); + __builtin_verbose_trap("Argument must not be null!"); } -The debug metadata would look as if it were produced for the following code: +The debugging information would look as if it were produced for the following code: .. code-block:: c++ __attribute__((always_inline)) - inline void "__llvm_verbose_trap: Argument_must_not_be_null"() { + inline void "__llvm_verbose_trap: Argument must not be null!"() { __builtin_trap(); } void foo(int* p) { if (p == nullptr) - "__llvm_verbose_trap: Argument_must_not_be_null"(); + "__llvm_verbose_trap: Argument must not be null!"(); } However, the LLVM IR would not actually contain a call to the artificial function — it only exists in the debug metadata. -Query for this feature with ``__has_builtin(__builtin_verbose_trap)``. +Query for this feature with ``__has_builtin(__builtin_verbose_trap)``. Note that +users need to enable debug information to enable this feature. A call to this +builtin is equivalent to a call to ``__builtin_trap`` if debug information isn't +enabled. ``__builtin_allow_runtime_check`` --------------------------------- diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index b42489746fdc6..7fd9184113ce9 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -1693,8 +1693,8 @@ llvm::DIType *CGDebugInfo::createFieldType( } llvm::DISubprogram * -CGDebugInfo::getFakeFuncSubprogram(const std::string &FakeFuncName) { - llvm::DISubprogram *&SP = FakeFuncMap[FakeFuncName]; +CGDebugInfo::createInlinedTrapSubprogram(const std::string &FuncName) { + llvm::DISubprogram *&SP = InlinedTrapFuncMap[FuncName]; if (!SP) { auto FileScope = TheCU->getFile(); @@ -1702,7 +1702,7 @@ CGDebugInfo::getFakeFuncSubprogram(const std::string &FakeFuncName) { // Note: We use `FileScope` rather than `TheCU` as the scope because that's // what LLVM's inliner seems to do. SP = DBuilder.createFunction( - /*Scope=*/FileScope, /*Name=*/FakeFuncName, /*LinkageName=*/StringRef(), + /*Scope=*/FileScope, /*Name=*/FuncName, /*LinkageName=*/StringRef(), /*File=*/FileScope, /*LineNo=*/0, /*Ty=*/DIFnTy, /*ScopeLine=*/0, /*Flags=*/llvm::DINode::FlagArtificial, @@ -3511,8 +3511,11 @@ llvm::DIMacroFile *CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent, llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor( llvm::DebugLoc TrapLocation, StringRef Prefix, StringRef FailureMsg) { - // Create debug info that describes a fake function whose name is the failure - // message. + assert((!FailureMsg.empty() || Prefix.find(' ') != std::string::npos) && + "Prefix must contain a space when FailureMsg is empty"); + + // Create debug info that describes an inlined function whose name is the + // failure message. std::string FuncName(Prefix); if (!FailureMsg.empty()) { // A space in the function name identifies this as not being a real function @@ -3521,11 +3524,7 @@ llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor( FuncName += FailureMsg; } - assert(FuncName.size() > 0); - assert(FuncName.find(' ') != std::string::npos && - "Fake function name must contain a space"); - - llvm::DISubprogram *TrapSP = getFakeFuncSubprogram(FuncName); + llvm::DISubprogram *TrapSP = createInlinedTrapSubprogram(FuncName); return llvm::DILocation::get(CGM.getLLVMContext(), /*Line=*/0, /*Column=*/0, /*Scope=*/TrapSP, /*InlinedAt=*/TrapLocation); } diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 217b99d448f75..08877a0413196 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -346,13 +346,13 @@ class CGDebugInfo { const FieldDecl *BitFieldDecl, const llvm::DIDerivedType *BitFieldDI, llvm::ArrayRef<llvm::Metadata *> PreviousFieldsDI, const RecordDecl *RD); - // A cache that maps fake function names used for __builtin_verbose_trap to - // subprograms. - std::map<std::string, llvm::DISubprogram *> FakeFuncMap; + // A cache that maps artificial inlined function names used for + // __builtin_verbose_trap to subprograms. + std::map<std::string, llvm::DISubprogram *> InlinedTrapFuncMap; - // A function that returns the subprogram corresponding to the fake function - // name. - llvm::DISubprogram *getFakeFuncSubprogram(const std::string &FakeFuncName); + // A function that returns the subprogram corresponding to the artificial + // inlined function for traps. + llvm::DISubprogram *createInlinedTrapSubprogram(const std::string &FuncName); /// Helpers for collecting fields of a record. /// @{ @@ -610,8 +610,8 @@ class CGDebugInfo { return CoroutineParameterMappings; } - // Create a debug location from `TrapLocation` that adds a fake - // inline frame where the frame name is + // Create a debug location from `TrapLocation` that adds an artificial inline + // frame where the frame name is // // * `<Prefix>: <FailureMsg>` if `<FailureMsg>` is not empty. // * `<Prefix>` if `<FailureMsg>` is empty. Note `<Prefix>` must diff --git a/clang/test/SemaCXX/verbose-trap.cpp b/clang/test/SemaCXX/verbose-trap.cpp index 2cbfad6c817f8..7968e99952095 100644 --- a/clang/test/SemaCXX/verbose-trap.cpp +++ b/clang/test/SemaCXX/verbose-trap.cpp @@ -10,6 +10,7 @@ char const constMsg3[] = "hello"; template <const char * const str> void f(const char * arg) { + __builtin_verbose_trap("Arbitrary string literals can be used!"); __builtin_verbose_trap("Argument_must_not_be_null"); __builtin_verbose_trap("hello" "world"); __builtin_verbose_trap(constMsg1); >From 856fd3d950e0d85b503fc847c04292c20df2c4bf Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Mon, 29 Jan 2024 07:43:50 -0800 Subject: [PATCH 04/18] Address review comments 1. Test long string argument. 2. Test c++20 unicode. 3. Allow empty string arguments. 4. #inlcude <map> and <string>. --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 3 +-- clang/lib/CodeGen/CGDebugInfo.h | 2 ++ clang/lib/Sema/SemaChecking.cpp | 2 +- clang/test/SemaCXX/verbose-trap.cpp | 12 +++++++++--- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 60e0087c8d4d8..46f054617f430 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8876,8 +8876,7 @@ def err_expected_callable_argument : Error< def note_building_builtin_dump_struct_call : Note< "in call to printing function with arguments '(%0)' while dumping struct">; def err_builtin_verbose_trap_arg : Error< - "argument to __builtin_verbose_trap must be a pointer to a non-empty constant " - "string">; + "argument to __builtin_verbose_trap must be a pointer to a constant string">; def err_atomic_load_store_uses_lib : Error< "atomic %select{load|store}0 requires runtime support that is not " diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 08877a0413196..8222ff37dd689 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -29,7 +29,9 @@ #include "llvm/IR/DebugInfo.h" #include "llvm/IR/ValueHandle.h" #include "llvm/Support/Allocator.h" +#include <map> #include <optional> +#include <string> namespace llvm { class MDNode; diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index fc8ff6bc340cc..8e894c8a4b77e 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -181,7 +181,7 @@ static bool checkBuiltinVerboseTrap(CallExpr *Call, Sema &S) { // FIXME: Add more checks and reject strings that can't be handled by // debuggers. std::string Result; - if (!Arg->tryEvaluateString(Result, S.Context) || Result.empty()) { + if (!Arg->tryEvaluateString(Result, S.Context)) { S.Diag(Arg->getBeginLoc(), diag::err_builtin_verbose_trap_arg) << Arg->getSourceRange(); return false; diff --git a/clang/test/SemaCXX/verbose-trap.cpp b/clang/test/SemaCXX/verbose-trap.cpp index 7968e99952095..e10529cc7c548 100644 --- a/clang/test/SemaCXX/verbose-trap.cpp +++ b/clang/test/SemaCXX/verbose-trap.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -verify %s +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -verify %s #if !__has_builtin(__builtin_verbose_trap) #error @@ -15,13 +16,18 @@ void f(const char * arg) { __builtin_verbose_trap("hello" "world"); __builtin_verbose_trap(constMsg1); __builtin_verbose_trap(constMsg2); - __builtin_verbose_trap(""); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a non-empty constant string}} + __builtin_verbose_trap(""); __builtin_verbose_trap(); // expected-error {{too few arguments}} - __builtin_verbose_trap(0); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a non-empty constant string}} + __builtin_verbose_trap(0); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a constant string}} __builtin_verbose_trap(1); // expected-error {{cannot initialize a parameter of type 'const char *' with}} - __builtin_verbose_trap(arg); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a non-empty constant string}} + __builtin_verbose_trap(arg); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a constant string}} __builtin_verbose_trap(str); __builtin_verbose_trap(u8"hello"); +#if __cplusplus >= 202002L + // FIXME: Accept c++20 u8 string literals. + // expected-error@-3 {{cannot initialize a parameter of type 'const char *' with an lvalue of type 'const char8_t[6]'}} +#endif + __builtin_verbose_trap("abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd"); } void test() { >From 5efc51478e3313b3d0bab32e3786ed9b2ef5da57 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Mon, 29 Jan 2024 17:36:05 -0800 Subject: [PATCH 05/18] Add triple to test --- clang/test/CodeGenCXX/debug-info-verbose-trap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp index 04fa68ae93ecb..54a21a9644000 100644 --- a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp +++ b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++20 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++20 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s // CHECK-LABEL: define void @_Z2f0v() // CHECK: call void @llvm.trap(), !dbg ![[LOC17:.*]] >From 0ff6ac0751227170cf75971ef855ed0cef0aa92d Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Thu, 1 Feb 2024 08:43:34 -0800 Subject: [PATCH 06/18] Address review comments --- clang/lib/CodeGen/CGBuiltin.cpp | 4 ++-- clang/lib/CodeGen/CGDebugInfo.cpp | 17 ++++++++--------- clang/lib/CodeGen/CGDebugInfo.h | 7 ++++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index ad19b4d24be5a..3692b4bdb1ef3 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -3591,8 +3591,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, if (getDebugInfo()) { std::string Str; E->getArg(0)->tryEvaluateString(Str, getContext()); - TrapLocation = getDebugInfo()->CreateTrapFailureMessageFor( - TrapLocation, "__llvm_verbose_trap", Str); + TrapLocation = + getDebugInfo()->CreateTrapFailureMessageFor(TrapLocation, Str); } ApplyDebugLocation ApplyTrapDI(*this, TrapLocation); EmitTrapCall(Intrinsic::trap); diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 7fd9184113ce9..36b3c7b941f0b 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -1693,7 +1693,7 @@ llvm::DIType *CGDebugInfo::createFieldType( } llvm::DISubprogram * -CGDebugInfo::createInlinedTrapSubprogram(const std::string &FuncName) { +CGDebugInfo::createInlinedTrapSubprogram(StringRef FuncName) { llvm::DISubprogram *&SP = InlinedTrapFuncMap[FuncName]; if (!SP) { @@ -3509,14 +3509,13 @@ llvm::DIMacroFile *CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent, return DBuilder.createTempMacroFile(Parent, Line, FName); } -llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor( - llvm::DebugLoc TrapLocation, StringRef Prefix, StringRef FailureMsg) { - assert((!FailureMsg.empty() || Prefix.find(' ') != std::string::npos) && - "Prefix must contain a space when FailureMsg is empty"); - - // Create debug info that describes an inlined function whose name is the - // failure message. - std::string FuncName(Prefix); +llvm::DILocation * +CGDebugInfo::CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, + StringRef FailureMsg) { + // Create a debug location from `TrapLocation` that adds an artificial inline + // frame. + const char *Prefix = "__llvm_verbose_trap"; + SmallString<64> FuncName(Prefix); if (!FailureMsg.empty()) { // A space in the function name identifies this as not being a real function // because it's not a valid symbol name. diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 8222ff37dd689..b0b2e0f3ab63c 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -350,11 +350,11 @@ class CGDebugInfo { // A cache that maps artificial inlined function names used for // __builtin_verbose_trap to subprograms. - std::map<std::string, llvm::DISubprogram *> InlinedTrapFuncMap; + llvm::StringMap<llvm::DISubprogram *> InlinedTrapFuncMap; // A function that returns the subprogram corresponding to the artificial // inlined function for traps. - llvm::DISubprogram *createInlinedTrapSubprogram(const std::string &FuncName); + llvm::DISubprogram *createInlinedTrapSubprogram(StringRef FuncName); /// Helpers for collecting fields of a record. /// @{ @@ -619,9 +619,10 @@ class CGDebugInfo { // * `<Prefix>` if `<FailureMsg>` is empty. Note `<Prefix>` must // contain a space. // + // Currently `<Prefix>` is always "__llvm_verbose_trap". + // // This is used to store failure reasons for traps. llvm::DILocation *CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, - StringRef Prefix, StringRef FailureMsg); private: >From 206e55d9ff59066262b8552b3036964d468f92cd Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Thu, 1 Feb 2024 11:12:18 -0800 Subject: [PATCH 07/18] Pass a better file scope when creating the subprogram for the artificial inlined function `TheCU->getFile()` returns `<stdin>` when `-main-file-name` is empty. --- clang/lib/CodeGen/CGDebugInfo.cpp | 9 ++++----- clang/lib/CodeGen/CGDebugInfo.h | 3 ++- clang/test/CodeGenCXX/debug-info-verbose-trap.cpp | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 36b3c7b941f0b..07bcd1ad7df45 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -1693,14 +1693,12 @@ llvm::DIType *CGDebugInfo::createFieldType( } llvm::DISubprogram * -CGDebugInfo::createInlinedTrapSubprogram(StringRef FuncName) { +CGDebugInfo::createInlinedTrapSubprogram(StringRef FuncName, + llvm::DIFile *FileScope) { llvm::DISubprogram *&SP = InlinedTrapFuncMap[FuncName]; if (!SP) { - auto FileScope = TheCU->getFile(); llvm::DISubroutineType *DIFnTy = DBuilder.createSubroutineType(nullptr); - // Note: We use `FileScope` rather than `TheCU` as the scope because that's - // what LLVM's inliner seems to do. SP = DBuilder.createFunction( /*Scope=*/FileScope, /*Name=*/FuncName, /*LinkageName=*/StringRef(), /*File=*/FileScope, /*LineNo=*/0, /*Ty=*/DIFnTy, @@ -3523,7 +3521,8 @@ CGDebugInfo::CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, FuncName += FailureMsg; } - llvm::DISubprogram *TrapSP = createInlinedTrapSubprogram(FuncName); + llvm::DISubprogram *TrapSP = + createInlinedTrapSubprogram(FuncName, TrapLocation->getFile()); return llvm::DILocation::get(CGM.getLLVMContext(), /*Line=*/0, /*Column=*/0, /*Scope=*/TrapSP, /*InlinedAt=*/TrapLocation); } diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index b0b2e0f3ab63c..d112ae74a0fec 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -354,7 +354,8 @@ class CGDebugInfo { // A function that returns the subprogram corresponding to the artificial // inlined function for traps. - llvm::DISubprogram *createInlinedTrapSubprogram(StringRef FuncName); + llvm::DISubprogram *createInlinedTrapSubprogram(StringRef FuncName, + llvm::DIFile *FileScope); /// Helpers for collecting fields of a record. /// @{ diff --git a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp index 54a21a9644000..0b156c23a54a2 100644 --- a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp +++ b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp @@ -13,7 +13,7 @@ // CHECK-LABEL: define internal void @_Z2f2IXadsoKcL_ZL8constMsgEEEEvv() // CHECK: call void @llvm.trap(), !dbg ![[LOC36:.*]] -// CHECK: ![[FILESCOPE:.*]] = !DIFile(filename: +// CHECK: ![[FILESCOPE:.*]] = !DIFile(filename: "{{.*}}debug-info-verbose-trap.cpp" // CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: "_Z2f0v", // CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], inlinedAt: ![[LOC20:.*]]) // CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap: Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) >From 4aa91cc8475ceb8ae2355579a3d4cdb5e9433c93 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Mon, 4 Mar 2024 21:31:49 -0800 Subject: [PATCH 08/18] Address review comments --- clang/docs/LanguageExtensions.rst | 4 +++ clang/include/clang/AST/Expr.h | 6 ++-- clang/lib/AST/ExprConstant.cpp | 7 +++-- clang/lib/CodeGen/CGBuiltin.cpp | 6 ++-- clang/lib/CodeGen/CGDebugInfo.cpp | 5 ++-- clang/lib/CodeGen/CGDebugInfo.h | 3 +- clang/lib/Sema/SemaChecking.cpp | 3 +- .../CodeGenCXX/debug-info-verbose-trap.cpp | 30 +++++++++++-------- clang/test/SemaCXX/verbose-trap.cpp | 2 +- 9 files changed, 37 insertions(+), 29 deletions(-) diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index 3acdfd5e6ca84..5e63d43abba1b 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -3518,6 +3518,9 @@ users need to enable debug information to enable this feature. A call to this builtin is equivalent to a call to ``__builtin_trap`` if debug information isn't enabled. +The optimizer can merge calls to trap with different messages, which degrades +the debugging experience. + ``__builtin_allow_runtime_check`` --------------------------------- @@ -3565,6 +3568,7 @@ guarded check. It's unused now. It will enable kind-specific lowering in future. E.g. a higher hotness cutoff can be used for more expensive kind of check. Query for this feature with ``__has_builtin(__builtin_allow_runtime_check)``. + ``__builtin_nondeterministic_value`` ------------------------------------ diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 9606a5fddd47e..c7af59da24195 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -788,9 +788,9 @@ class Expr : public ValueStmt { EvalResult &Status) const; /// If the current Expr can be evaluated to a pointer to a null-terminated - /// constant string, return the constant string (without the terminating null) - /// in Result. Return true if it succeeds. - bool tryEvaluateString(std::string &Result, ASTContext &Ctx) const; + /// constant string, return the constant string (without the terminating + /// null). + std::optional<std::string> tryEvaluateString(ASTContext &Ctx) const; /// Enumeration used to describe the kind of Null pointer constant /// returned from \c isNullPointerConstant(). diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index c5ac10f2013dc..29ac550c21266 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -16844,12 +16844,15 @@ static bool EvaluateBuiltinStrLen(const Expr *E, uint64_t &Result, } } -bool Expr::tryEvaluateString(std::string &StringResult, ASTContext &Ctx) const { +std::optional<std::string> Expr::tryEvaluateString(ASTContext &Ctx) const { Expr::EvalStatus Status; EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantFold); uint64_t Result; + std::string StringResult; - return EvaluateBuiltinStrLen(this, Result, Info, &StringResult); + if (EvaluateBuiltinStrLen(this, Result, Info, &StringResult)) + return StringResult; + return {}; } bool Expr::EvaluateCharRangeAsString(std::string &Result, diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 3692b4bdb1ef3..2c3fa677facc8 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -3589,10 +3589,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, case Builtin::BI__builtin_verbose_trap: { llvm::DILocation *TrapLocation = Builder.getCurrentDebugLocation(); if (getDebugInfo()) { - std::string Str; - E->getArg(0)->tryEvaluateString(Str, getContext()); - TrapLocation = - getDebugInfo()->CreateTrapFailureMessageFor(TrapLocation, Str); + TrapLocation = getDebugInfo()->CreateTrapFailureMessageFor( + TrapLocation, *E->getArg(0)->tryEvaluateString(getContext())); } ApplyDebugLocation ApplyTrapDI(*this, TrapLocation); EmitTrapCall(Intrinsic::trap); diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 07bcd1ad7df45..0f1e634ef9085 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -1695,6 +1695,9 @@ llvm::DIType *CGDebugInfo::createFieldType( llvm::DISubprogram * CGDebugInfo::createInlinedTrapSubprogram(StringRef FuncName, llvm::DIFile *FileScope) { + // We are caching the subprogram because we don't want to duplicate + // subprograms with the same message. Note that `SPFlagDefinition` prevents + // subprograms from being uniqued. llvm::DISubprogram *&SP = InlinedTrapFuncMap[FuncName]; if (!SP) { @@ -3515,8 +3518,6 @@ CGDebugInfo::CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, const char *Prefix = "__llvm_verbose_trap"; SmallString<64> FuncName(Prefix); if (!FailureMsg.empty()) { - // A space in the function name identifies this as not being a real function - // because it's not a valid symbol name. FuncName += ": "; FuncName += FailureMsg; } diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index d112ae74a0fec..10101da1c796a 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -617,8 +617,7 @@ class CGDebugInfo { // frame where the frame name is // // * `<Prefix>: <FailureMsg>` if `<FailureMsg>` is not empty. - // * `<Prefix>` if `<FailureMsg>` is empty. Note `<Prefix>` must - // contain a space. + // * `<Prefix>` if `<FailureMsg>` is empty. // // Currently `<Prefix>` is always "__llvm_verbose_trap". // diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 8e894c8a4b77e..6b6e9235e29c4 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -180,8 +180,7 @@ static bool checkBuiltinVerboseTrap(CallExpr *Call, Sema &S) { // FIXME: Add more checks and reject strings that can't be handled by // debuggers. - std::string Result; - if (!Arg->tryEvaluateString(Result, S.Context)) { + if (!Arg->tryEvaluateString(S.Context).has_value()) { S.Diag(Arg->getBeginLoc(), diag::err_builtin_verbose_trap_arg) << Arg->getSourceRange(); return false; diff --git a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp index 0b156c23a54a2..0854b76a4bdea 100644 --- a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp +++ b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp @@ -3,6 +3,8 @@ // CHECK-LABEL: define void @_Z2f0v() // CHECK: call void @llvm.trap(), !dbg ![[LOC17:.*]] +// CHECK: declare void @llvm.trap() #[[ATTR1:.*]] + // CHECK-LABEL: define void @_Z2f1v() // CHECK: call void @llvm.trap(), !dbg ![[LOC23:.*]] // CHECK: call void @llvm.trap(), !dbg ![[LOC25:.*]] @@ -13,32 +15,34 @@ // CHECK-LABEL: define internal void @_Z2f2IXadsoKcL_ZL8constMsgEEEEvv() // CHECK: call void @llvm.trap(), !dbg ![[LOC36:.*]] +// CHECK: attributes #[[ATTR1]] = { cold {{.*}}} + // CHECK: ![[FILESCOPE:.*]] = !DIFile(filename: "{{.*}}debug-info-verbose-trap.cpp" -// CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: "_Z2f0v", -// CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], inlinedAt: ![[LOC20:.*]]) -// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap: Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) -// CHECK: ![[LOC20]] = !DILocation(line: 34, column: 3, scope: ![[SUBPROG14]]) -// CHECK: ![[SUBPROG22:.*]] = distinct !DISubprogram(name: "f1", linkageName: "_Z2f1v", -// CHECK: ![[LOC23]] = !DILocation(line: 0, scope: ![[SUBPROG18]], inlinedAt: ![[LOC24:.*]]) -// CHECK: ![[LOC24]] = !DILocation(line: 38, column: 3, scope: ![[SUBPROG22]]) -// CHECK: ![[LOC25]] = !DILocation(line: 0, scope: ![[SUBPROG26:.*]], inlinedAt: ![[LOC27:.*]]) -// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__llvm_verbose_trap: hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) -// CHECK: ![[LOC27]] = !DILocation(line: 39, column: 3, scope: ![[SUBPROG22]]) -// CHECK: ![[SUBPROG32:.*]] = distinct !DISubprogram(name: "f2<&constMsg[0]>", linkageName: "_Z2f2IXadsoKcL_ZL8constMsgEEEEvv", -// CHECK: ![[LOC36]] = !DILocation(line: 0, scope: ![[SUBPROG26]], inlinedAt: ![[LOC37:.*]]) -// CHECK: ![[LOC37]] = !DILocation(line: 44, column: 3, scope: ![[SUBPROG32]]) char const constMsg[] = "hello"; +// CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: "_Z2f0v", +// CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], inlinedAt: ![[LOC20:.*]]) +// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap: Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[LOC20]] = !DILocation(line: [[@LINE+2]], column: 3, scope: ![[SUBPROG14]]) void f0() { __builtin_verbose_trap("Argument_must_not_be_null"); } +// CHECK: ![[SUBPROG22:.*]] = distinct !DISubprogram(name: "f1", linkageName: "_Z2f1v", +// CHECK: ![[LOC23]] = !DILocation(line: 0, scope: ![[SUBPROG18]], inlinedAt: ![[LOC24:.*]]) +// CHECK: ![[LOC24]] = !DILocation(line: [[@LINE+5]], column: 3, scope: ![[SUBPROG22]]) +// CHECK: ![[LOC25]] = !DILocation(line: 0, scope: ![[SUBPROG26:.*]], inlinedAt: ![[LOC27:.*]]) +// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__llvm_verbose_trap: hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[LOC27]] = !DILocation(line: [[@LINE+3]], column: 3, scope: ![[SUBPROG22]]) void f1() { __builtin_verbose_trap("Argument_must_not_be_null"); __builtin_verbose_trap("hello"); } +// CHECK: ![[SUBPROG32:.*]] = distinct !DISubprogram(name: "f2<&constMsg[0]>", linkageName: "_Z2f2IXadsoKcL_ZL8constMsgEEEEvv", +// CHECK: ![[LOC36]] = !DILocation(line: 0, scope: ![[SUBPROG26]], inlinedAt: ![[LOC37:.*]]) +// CHECK: ![[LOC37]] = !DILocation(line: [[@LINE+3]], column: 3, scope: ![[SUBPROG32]]) template <const char * const str> void f2() { __builtin_verbose_trap(str); diff --git a/clang/test/SemaCXX/verbose-trap.cpp b/clang/test/SemaCXX/verbose-trap.cpp index e10529cc7c548..86458a3957e7f 100644 --- a/clang/test/SemaCXX/verbose-trap.cpp +++ b/clang/test/SemaCXX/verbose-trap.cpp @@ -19,7 +19,7 @@ void f(const char * arg) { __builtin_verbose_trap(""); __builtin_verbose_trap(); // expected-error {{too few arguments}} __builtin_verbose_trap(0); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a constant string}} - __builtin_verbose_trap(1); // expected-error {{cannot initialize a parameter of type 'const char *' with}} + __builtin_verbose_trap(1); // expected-error {{cannot initialize a parameter of type 'const char *' with an rvalue of type 'int'}} __builtin_verbose_trap(arg); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a constant string}} __builtin_verbose_trap(str); __builtin_verbose_trap(u8"hello"); >From 521fe9174c12164e38d6810d12a868e3787046a1 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Tue, 12 Mar 2024 16:58:23 -0700 Subject: [PATCH 09/18] Define CLANG_VERBOSE_TRAP_PREFIX in public header --- clang/include/clang/CodeGen/ModuleBuilder.h | 3 +++ clang/lib/CodeGen/CGDebugInfo.cpp | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/CodeGen/ModuleBuilder.h b/clang/include/clang/CodeGen/ModuleBuilder.h index edacd82bf899d..69f4537330ba5 100644 --- a/clang/include/clang/CodeGen/ModuleBuilder.h +++ b/clang/include/clang/CodeGen/ModuleBuilder.h @@ -27,6 +27,9 @@ namespace llvm { } } +// Prefix for __builtin_verbose_trap. +#define CLANG_VERBOSE_TRAP_PREFIX "__llvm_verbose_trap" + namespace clang { class CodeGenOptions; class CoverageSourceInfo; diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0f1e634ef9085..d461e375f620a 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -32,6 +32,7 @@ #include "clang/Basic/FileManager.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/Version.h" +#include "clang/CodeGen/ModuleBuilder.h" #include "clang/Frontend/FrontendOptions.h" #include "clang/Lex/HeaderSearchOptions.h" #include "clang/Lex/ModuleMap.h" @@ -3515,7 +3516,7 @@ CGDebugInfo::CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, StringRef FailureMsg) { // Create a debug location from `TrapLocation` that adds an artificial inline // frame. - const char *Prefix = "__llvm_verbose_trap"; + const char *Prefix = CLANG_VERBOSE_TRAP_PREFIX; SmallString<64> FuncName(Prefix); if (!FailureMsg.empty()) { FuncName += ": "; >From c72fc1767c211816e58d364caa0b17809166b521 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Tue, 19 Mar 2024 18:56:37 -0700 Subject: [PATCH 10/18] Address review comments --- clang/docs/LanguageExtensions.rst | 4 ++-- clang/lib/CodeGen/CGBuiltin.cpp | 1 + clang/lib/CodeGen/CGDebugInfo.h | 22 +++++++++++----------- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index 5e63d43abba1b..4fc2bf3f1b7ed 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -3510,8 +3510,8 @@ The debugging information would look as if it were produced for the following co "__llvm_verbose_trap: Argument must not be null!"(); } -However, the LLVM IR would not actually contain a call to the artificial -function — it only exists in the debug metadata. +However, the generated code would not actually contain a call to the artificial +function — it only exists in the debugging information. Query for this feature with ``__has_builtin(__builtin_verbose_trap)``. Note that users need to enable debug information to enable this feature. A call to this diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 2c3fa677facc8..cb25c8e5ea8e7 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -3593,6 +3593,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, TrapLocation, *E->getArg(0)->tryEvaluateString(getContext())); } ApplyDebugLocation ApplyTrapDI(*this, TrapLocation); + // Currently no attempt is made to prevent traps from being merged. EmitTrapCall(Intrinsic::trap); return RValue::get(nullptr); } diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 10101da1c796a..7e79adfb7653a 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -348,8 +348,8 @@ class CGDebugInfo { const FieldDecl *BitFieldDecl, const llvm::DIDerivedType *BitFieldDI, llvm::ArrayRef<llvm::Metadata *> PreviousFieldsDI, const RecordDecl *RD); - // A cache that maps artificial inlined function names used for - // __builtin_verbose_trap to subprograms. + /// A cache that maps artificial inlined function names used for + /// __builtin_verbose_trap to subprograms. llvm::StringMap<llvm::DISubprogram *> InlinedTrapFuncMap; // A function that returns the subprogram corresponding to the artificial @@ -613,15 +613,15 @@ class CGDebugInfo { return CoroutineParameterMappings; } - // Create a debug location from `TrapLocation` that adds an artificial inline - // frame where the frame name is - // - // * `<Prefix>: <FailureMsg>` if `<FailureMsg>` is not empty. - // * `<Prefix>` if `<FailureMsg>` is empty. - // - // Currently `<Prefix>` is always "__llvm_verbose_trap". - // - // This is used to store failure reasons for traps. + /// Create a debug location from `TrapLocation` that adds an artificial inline + /// frame where the frame name is + /// + /// * `<Prefix>: <FailureMsg>` if `<FailureMsg>` is not empty. + /// * `<Prefix>` if `<FailureMsg>` is empty. + /// + /// Currently `<Prefix>` is always "__llvm_verbose_trap". + /// + /// This is used to store failure reasons for traps. llvm::DILocation *CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, StringRef FailureMsg); >From 5bbb01be39b997d475159ec1e6b3a44944cc0352 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Wed, 20 Mar 2024 12:38:22 -0700 Subject: [PATCH 11/18] Use three forward slashes --- clang/lib/CodeGen/CGDebugInfo.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 7e79adfb7653a..5cc2f5978477e 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -352,8 +352,8 @@ class CGDebugInfo { /// __builtin_verbose_trap to subprograms. llvm::StringMap<llvm::DISubprogram *> InlinedTrapFuncMap; - // A function that returns the subprogram corresponding to the artificial - // inlined function for traps. + /// A function that returns the subprogram corresponding to the artificial + /// inlined function for traps. llvm::DISubprogram *createInlinedTrapSubprogram(StringRef FuncName, llvm::DIFile *FileScope); >From 4896af087488c294c93d2123389fded32ea65b30 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Mon, 1 Apr 2024 13:08:29 -0700 Subject: [PATCH 12/18] Fix test --- clang/test/CodeGenCXX/debug-info-verbose-trap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp index 0854b76a4bdea..966fd2b7bced9 100644 --- a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp +++ b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp @@ -40,7 +40,7 @@ void f1() { __builtin_verbose_trap("hello"); } -// CHECK: ![[SUBPROG32:.*]] = distinct !DISubprogram(name: "f2<&constMsg[0]>", linkageName: "_Z2f2IXadsoKcL_ZL8constMsgEEEEvv", +// CHECK: ![[SUBPROG32:.*]] = distinct !DISubprogram(name: "f2<constMsg>", linkageName: "_Z2f2IXadsoKcL_ZL8constMsgEEEEvv", // CHECK: ![[LOC36]] = !DILocation(line: 0, scope: ![[SUBPROG26]], inlinedAt: ![[LOC37:.*]]) // CHECK: ![[LOC37]] = !DILocation(line: [[@LINE+3]], column: 3, scope: ![[SUBPROG32]]) template <const char * const str> >From 74639861168357e3bc9f5a8b9d18e7dac9a3c6f8 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Wed, 24 Apr 2024 18:57:39 -0700 Subject: [PATCH 13/18] Pass category string to the builtin --- clang/docs/LanguageExtensions.rst | 12 +++--- clang/include/clang/Basic/Builtins.td | 2 +- clang/lib/CodeGen/CGBuiltin.cpp | 3 +- clang/lib/CodeGen/CGDebugInfo.cpp | 14 +++---- clang/lib/CodeGen/CGDebugInfo.h | 6 +-- clang/lib/Sema/SemaChecking.cpp | 25 ++++++----- .../CodeGenCXX/debug-info-verbose-trap.cpp | 23 ++++++----- clang/test/SemaCXX/verbose-trap.cpp | 41 ++++++++++++------- 8 files changed, 72 insertions(+), 54 deletions(-) diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index 4fc2bf3f1b7ed..1b6ce2d22f401 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -3478,14 +3478,14 @@ debugger is attached or in a symbolicated crash log. .. code-block:: c++ - __builtin_verbose_trap(const char *reason) + __builtin_verbose_trap(const char *category, const char *reason) **Description** ``__builtin_verbose_trap`` is lowered to the ` ``llvm.trap`` <https://llvm.org/docs/LangRef.html#llvm-trap-intrinsic>`_ builtin. Additionally, clang emits debugging information that represents an artificial -inline frame whose name encodes the string passed to the builtin, prefixed by a -"magic" prefix. +inline frame whose name encodes the category and reason strings passed to the builtin, +prefixed by a "magic" prefix. For example, consider the following code: @@ -3493,7 +3493,7 @@ For example, consider the following code: void foo(int* p) { if (p == nullptr) - __builtin_verbose_trap("Argument must not be null!"); + __builtin_verbose_trap("check null", "Argument must not be null!"); } The debugging information would look as if it were produced for the following code: @@ -3501,13 +3501,13 @@ The debugging information would look as if it were produced for the following co .. code-block:: c++ __attribute__((always_inline)) - inline void "__llvm_verbose_trap: Argument must not be null!"() { + inline void "__llvm_verbose_trap:check null:Argument must not be null!"() { __builtin_trap(); } void foo(int* p) { if (p == nullptr) - "__llvm_verbose_trap: Argument must not be null!"(); + "__llvm_verbose_trap:check null:Argument must not be null!"(); } However, the generated code would not actually contain a call to the artificial diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td index 5b3b9d04e576a..1c4453d3c84e9 100644 --- a/clang/include/clang/Basic/Builtins.td +++ b/clang/include/clang/Basic/Builtins.td @@ -1155,7 +1155,7 @@ def Trap : Builtin { def VerboseTrap : Builtin { let Spellings = ["__builtin_verbose_trap"]; let Attributes = [NoThrow, NoReturn]; - let Prototype = "void(char const*)"; + let Prototype = "void(char const*, char const*)"; } def Debugtrap : Builtin { diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index cb25c8e5ea8e7..5c762c641235c 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -3590,7 +3590,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, llvm::DILocation *TrapLocation = Builder.getCurrentDebugLocation(); if (getDebugInfo()) { TrapLocation = getDebugInfo()->CreateTrapFailureMessageFor( - TrapLocation, *E->getArg(0)->tryEvaluateString(getContext())); + TrapLocation, *E->getArg(0)->tryEvaluateString(getContext()), + *E->getArg(1)->tryEvaluateString(getContext())); } ApplyDebugLocation ApplyTrapDI(*this, TrapLocation); // Currently no attempt is made to prevent traps from being merged. diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index d461e375f620a..f8dba8c212d85 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -3511,17 +3511,17 @@ llvm::DIMacroFile *CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent, return DBuilder.createTempMacroFile(Parent, Line, FName); } -llvm::DILocation * -CGDebugInfo::CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, - StringRef FailureMsg) { +llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor( + llvm::DebugLoc TrapLocation, StringRef Category, StringRef FailureMsg) { // Create a debug location from `TrapLocation` that adds an artificial inline // frame. const char *Prefix = CLANG_VERBOSE_TRAP_PREFIX; SmallString<64> FuncName(Prefix); - if (!FailureMsg.empty()) { - FuncName += ": "; - FuncName += FailureMsg; - } + + FuncName += ":"; + FuncName += Category; + FuncName += ":"; + FuncName += FailureMsg; llvm::DISubprogram *TrapSP = createInlinedTrapSubprogram(FuncName, TrapLocation->getFile()); diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 5cc2f5978477e..7b5c9b0a81ba4 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -616,13 +616,13 @@ class CGDebugInfo { /// Create a debug location from `TrapLocation` that adds an artificial inline /// frame where the frame name is /// - /// * `<Prefix>: <FailureMsg>` if `<FailureMsg>` is not empty. - /// * `<Prefix>` if `<FailureMsg>` is empty. + /// * `<Prefix>:<Category>:<FailureMsg>` /// - /// Currently `<Prefix>` is always "__llvm_verbose_trap". + /// `<Prefix>` is "__llvm_verbose_trap". /// /// This is used to store failure reasons for traps. llvm::DILocation *CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, + StringRef Category, StringRef FailureMsg); private: diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 6b6e9235e29c4..5d5b1d13e1ec9 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -173,19 +173,24 @@ static bool checkArgCount(Sema &S, CallExpr *Call, unsigned DesiredArgCount) { } static bool checkBuiltinVerboseTrap(CallExpr *Call, Sema &S) { - Expr *Arg = Call->getArg(0); + bool HasError = false; - if (Arg->isValueDependent()) - return true; + for (int I = 0; I < Call->getNumArgs(); ++I) { + Expr *Arg = Call->getArg(I); - // FIXME: Add more checks and reject strings that can't be handled by - // debuggers. - if (!Arg->tryEvaluateString(S.Context).has_value()) { - S.Diag(Arg->getBeginLoc(), diag::err_builtin_verbose_trap_arg) - << Arg->getSourceRange(); - return false; + if (Arg->isValueDependent()) + continue; + + // FIXME: Add more checks and reject strings that can't be handled by + // debuggers. + if (!Arg->tryEvaluateString(S.Context).has_value()) { + S.Diag(Arg->getBeginLoc(), diag::err_builtin_verbose_trap_arg) + << Arg->getSourceRange(); + HasError = true; + } } - return true; + + return !HasError; } static bool convertArgumentToType(Sema &S, Expr *&Value, QualType Ty) { diff --git a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp index 966fd2b7bced9..948b967bbb9aa 100644 --- a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp +++ b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp @@ -10,44 +10,45 @@ // CHECK: call void @llvm.trap(), !dbg ![[LOC25:.*]] // CHECK-LABEL: define void @_Z2f3v() -// CHECK: call void @_Z2f2IXadsoKcL_ZL8constMsgEEEEvv() +// CHECK: call void @_Z2f2IXadsoKcL_ZL8constCatEEEXadsoS0_L_ZL8constMsgEEEEvv() -// CHECK-LABEL: define internal void @_Z2f2IXadsoKcL_ZL8constMsgEEEEvv() +// CHECK-LABEL: define internal void @_Z2f2IXadsoKcL_ZL8constCatEEEXadsoS0_L_ZL8constMsgEEEEvv // CHECK: call void @llvm.trap(), !dbg ![[LOC36:.*]] // CHECK: attributes #[[ATTR1]] = { cold {{.*}}} // CHECK: ![[FILESCOPE:.*]] = !DIFile(filename: "{{.*}}debug-info-verbose-trap.cpp" +char const constCat[] = "category2"; char const constMsg[] = "hello"; // CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: "_Z2f0v", // CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], inlinedAt: ![[LOC20:.*]]) -// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap: Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap:category1:Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) // CHECK: ![[LOC20]] = !DILocation(line: [[@LINE+2]], column: 3, scope: ![[SUBPROG14]]) void f0() { - __builtin_verbose_trap("Argument_must_not_be_null"); + __builtin_verbose_trap("category1", "Argument_must_not_be_null"); } // CHECK: ![[SUBPROG22:.*]] = distinct !DISubprogram(name: "f1", linkageName: "_Z2f1v", // CHECK: ![[LOC23]] = !DILocation(line: 0, scope: ![[SUBPROG18]], inlinedAt: ![[LOC24:.*]]) // CHECK: ![[LOC24]] = !DILocation(line: [[@LINE+5]], column: 3, scope: ![[SUBPROG22]]) // CHECK: ![[LOC25]] = !DILocation(line: 0, scope: ![[SUBPROG26:.*]], inlinedAt: ![[LOC27:.*]]) -// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__llvm_verbose_trap: hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__llvm_verbose_trap:category2:hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) // CHECK: ![[LOC27]] = !DILocation(line: [[@LINE+3]], column: 3, scope: ![[SUBPROG22]]) void f1() { - __builtin_verbose_trap("Argument_must_not_be_null"); - __builtin_verbose_trap("hello"); + __builtin_verbose_trap("category1", "Argument_must_not_be_null"); + __builtin_verbose_trap("category2", "hello"); } -// CHECK: ![[SUBPROG32:.*]] = distinct !DISubprogram(name: "f2<constMsg>", linkageName: "_Z2f2IXadsoKcL_ZL8constMsgEEEEvv", +// CHECK: ![[SUBPROG32:.*]] = distinct !DISubprogram(name: "f2<constCat, constMsg>", linkageName: "_Z2f2IXadsoKcL_ZL8constCatEEEXadsoS0_L_ZL8constMsgEEEEvv", // CHECK: ![[LOC36]] = !DILocation(line: 0, scope: ![[SUBPROG26]], inlinedAt: ![[LOC37:.*]]) // CHECK: ![[LOC37]] = !DILocation(line: [[@LINE+3]], column: 3, scope: ![[SUBPROG32]]) -template <const char * const str> +template <const char * const category, const char * const reason> void f2() { - __builtin_verbose_trap(str); + __builtin_verbose_trap(category, reason); } void f3() { - f2<constMsg>(); + f2<constCat, constMsg>(); } diff --git a/clang/test/SemaCXX/verbose-trap.cpp b/clang/test/SemaCXX/verbose-trap.cpp index 86458a3957e7f..a91dd01106cc1 100644 --- a/clang/test/SemaCXX/verbose-trap.cpp +++ b/clang/test/SemaCXX/verbose-trap.cpp @@ -5,31 +5,42 @@ #error #endif +constexpr char const* constCat1 = "cat"; +char const* const constCat2 = "cat"; +char const constCat3[] = "cat"; + constexpr char const* constMsg1 = "hello"; char const* const constMsg2 = "hello"; char const constMsg3[] = "hello"; -template <const char * const str> +template <const char * const category, const char * const reason> void f(const char * arg) { - __builtin_verbose_trap("Arbitrary string literals can be used!"); - __builtin_verbose_trap("Argument_must_not_be_null"); - __builtin_verbose_trap("hello" "world"); - __builtin_verbose_trap(constMsg1); - __builtin_verbose_trap(constMsg2); - __builtin_verbose_trap(""); + __builtin_verbose_trap("cat1", "Arbitrary string literals can be used!"); + __builtin_verbose_trap(" cat1 ", "Argument_must_not_be_null"); + __builtin_verbose_trap("cat" "egory1", "hello" "world"); + __builtin_verbose_trap(constCat1, constMsg1); + __builtin_verbose_trap(constCat2, constMsg2); + __builtin_verbose_trap("", ""); __builtin_verbose_trap(); // expected-error {{too few arguments}} - __builtin_verbose_trap(0); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a constant string}} - __builtin_verbose_trap(1); // expected-error {{cannot initialize a parameter of type 'const char *' with an rvalue of type 'int'}} - __builtin_verbose_trap(arg); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a constant string}} - __builtin_verbose_trap(str); - __builtin_verbose_trap(u8"hello"); + __builtin_verbose_trap(""); // expected-error {{too few arguments}} + __builtin_verbose_trap("", "", ""); // expected-error {{too many arguments}} + __builtin_verbose_trap("", 0); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a constant string}} + __builtin_verbose_trap(1, ""); // expected-error {{cannot initialize a parameter of type 'const char *' with an rvalue of type 'int'}} + __builtin_verbose_trap(arg, ""); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a constant string}} + __builtin_verbose_trap(category, reason); + __builtin_verbose_trap(u8"cat1", u8"hello"); #if __cplusplus >= 202002L // FIXME: Accept c++20 u8 string literals. - // expected-error@-3 {{cannot initialize a parameter of type 'const char *' with an lvalue of type 'const char8_t[6]'}} + // expected-error@-3 {{cannot initialize a parameter of type 'const char *' with an lvalue of type 'const char8_t[5]'}} #endif - __builtin_verbose_trap("abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd"); + __builtin_verbose_trap("", "abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd"); +} + +template <const char * const category> +void f2() { + __builtin_verbose_trap(category, 1); // expected-error {{cannot initialize a parameter of type 'const char *' with an rvalue of type 'int'}} } void test() { - f<constMsg3>(nullptr); + f<constCat3, constMsg3>(nullptr); } >From c7bd9145df6f51f3fa56d6c34b6eba0017062e70 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Thu, 25 Apr 2024 13:23:07 -0700 Subject: [PATCH 14/18] Use $ as the separator --- clang/docs/LanguageExtensions.rst | 4 ++-- clang/lib/CodeGen/CGDebugInfo.cpp | 4 ++-- clang/test/CodeGenCXX/debug-info-verbose-trap.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index 1b6ce2d22f401..dddeb96056825 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -3501,13 +3501,13 @@ The debugging information would look as if it were produced for the following co .. code-block:: c++ __attribute__((always_inline)) - inline void "__llvm_verbose_trap:check null:Argument must not be null!"() { + inline void "__llvm_verbose_trap$check null$Argument must not be null!"() { __builtin_trap(); } void foo(int* p) { if (p == nullptr) - "__llvm_verbose_trap:check null:Argument must not be null!"(); + "__llvm_verbose_trap$check null$Argument must not be null!"(); } However, the generated code would not actually contain a call to the artificial diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index f8dba8c212d85..2a4707860bc46 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -3518,9 +3518,9 @@ llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor( const char *Prefix = CLANG_VERBOSE_TRAP_PREFIX; SmallString<64> FuncName(Prefix); - FuncName += ":"; + FuncName += "$"; FuncName += Category; - FuncName += ":"; + FuncName += "$"; FuncName += FailureMsg; llvm::DISubprogram *TrapSP = diff --git a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp index 948b967bbb9aa..beb4a7673b731 100644 --- a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp +++ b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp @@ -24,7 +24,7 @@ char const constMsg[] = "hello"; // CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: "_Z2f0v", // CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], inlinedAt: ![[LOC20:.*]]) -// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap:category1:Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap$category1$Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) // CHECK: ![[LOC20]] = !DILocation(line: [[@LINE+2]], column: 3, scope: ![[SUBPROG14]]) void f0() { __builtin_verbose_trap("category1", "Argument_must_not_be_null"); @@ -34,7 +34,7 @@ void f0() { // CHECK: ![[LOC23]] = !DILocation(line: 0, scope: ![[SUBPROG18]], inlinedAt: ![[LOC24:.*]]) // CHECK: ![[LOC24]] = !DILocation(line: [[@LINE+5]], column: 3, scope: ![[SUBPROG22]]) // CHECK: ![[LOC25]] = !DILocation(line: 0, scope: ![[SUBPROG26:.*]], inlinedAt: ![[LOC27:.*]]) -// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__llvm_verbose_trap:category2:hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__llvm_verbose_trap$category2$hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) // CHECK: ![[LOC27]] = !DILocation(line: [[@LINE+3]], column: 3, scope: ![[SUBPROG22]]) void f1() { __builtin_verbose_trap("category1", "Argument_must_not_be_null"); >From 1db962811d35a0320b8078f28c0f2e6e114246ad Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Tue, 30 Apr 2024 17:19:39 -0700 Subject: [PATCH 15/18] Use a shorter prefix --- clang/docs/LanguageExtensions.rst | 4 ++-- clang/include/clang/CodeGen/ModuleBuilder.h | 4 ++-- clang/lib/CodeGen/CGDebugInfo.cpp | 2 +- clang/lib/CodeGen/CGDebugInfo.h | 5 ++--- clang/test/CodeGenCXX/debug-info-verbose-trap.cpp | 4 ++-- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index dddeb96056825..7bde31114bbd9 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -3501,13 +3501,13 @@ The debugging information would look as if it were produced for the following co .. code-block:: c++ __attribute__((always_inline)) - inline void "__llvm_verbose_trap$check null$Argument must not be null!"() { + inline void "__trap_msg$check null$Argument must not be null!"() { __builtin_trap(); } void foo(int* p) { if (p == nullptr) - "__llvm_verbose_trap$check null$Argument must not be null!"(); + "__trap_msg$check null$Argument must not be null!"(); } However, the generated code would not actually contain a call to the artificial diff --git a/clang/include/clang/CodeGen/ModuleBuilder.h b/clang/include/clang/CodeGen/ModuleBuilder.h index 69f4537330ba5..4a6200ad25dea 100644 --- a/clang/include/clang/CodeGen/ModuleBuilder.h +++ b/clang/include/clang/CodeGen/ModuleBuilder.h @@ -27,8 +27,8 @@ namespace llvm { } } -// Prefix for __builtin_verbose_trap. -#define CLANG_VERBOSE_TRAP_PREFIX "__llvm_verbose_trap" +// Prefix of the name of the artificial inline frame. +#define CLANG_TRAP_PREFIX "__trap_msg" namespace clang { class CodeGenOptions; diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 2a4707860bc46..73747a70c445f 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -3515,7 +3515,7 @@ llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor( llvm::DebugLoc TrapLocation, StringRef Category, StringRef FailureMsg) { // Create a debug location from `TrapLocation` that adds an artificial inline // frame. - const char *Prefix = CLANG_VERBOSE_TRAP_PREFIX; + const char *Prefix = CLANG_TRAP_PREFIX; SmallString<64> FuncName(Prefix); FuncName += "$"; diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 7b5c9b0a81ba4..5b48155ef26aa 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -348,8 +348,7 @@ class CGDebugInfo { const FieldDecl *BitFieldDecl, const llvm::DIDerivedType *BitFieldDI, llvm::ArrayRef<llvm::Metadata *> PreviousFieldsDI, const RecordDecl *RD); - /// A cache that maps artificial inlined function names used for - /// __builtin_verbose_trap to subprograms. + /// A cache that maps names of artificial inlined functions to subprograms. llvm::StringMap<llvm::DISubprogram *> InlinedTrapFuncMap; /// A function that returns the subprogram corresponding to the artificial @@ -618,7 +617,7 @@ class CGDebugInfo { /// /// * `<Prefix>:<Category>:<FailureMsg>` /// - /// `<Prefix>` is "__llvm_verbose_trap". + /// `<Prefix>` is "__trap_msg". /// /// This is used to store failure reasons for traps. llvm::DILocation *CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, diff --git a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp index beb4a7673b731..a8c78343b12a3 100644 --- a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp +++ b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp @@ -24,7 +24,7 @@ char const constMsg[] = "hello"; // CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: "_Z2f0v", // CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], inlinedAt: ![[LOC20:.*]]) -// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap$category1$Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__trap_msg$category1$Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) // CHECK: ![[LOC20]] = !DILocation(line: [[@LINE+2]], column: 3, scope: ![[SUBPROG14]]) void f0() { __builtin_verbose_trap("category1", "Argument_must_not_be_null"); @@ -34,7 +34,7 @@ void f0() { // CHECK: ![[LOC23]] = !DILocation(line: 0, scope: ![[SUBPROG18]], inlinedAt: ![[LOC24:.*]]) // CHECK: ![[LOC24]] = !DILocation(line: [[@LINE+5]], column: 3, scope: ![[SUBPROG22]]) // CHECK: ![[LOC25]] = !DILocation(line: 0, scope: ![[SUBPROG26:.*]], inlinedAt: ![[LOC27:.*]]) -// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__llvm_verbose_trap$category2$hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__trap_msg$category2$hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) // CHECK: ![[LOC27]] = !DILocation(line: [[@LINE+3]], column: 3, scope: ![[SUBPROG22]]) void f1() { __builtin_verbose_trap("category1", "Argument_must_not_be_null"); >From 6881d0d32ca877a21f8b83d4dc8c61017ee056c3 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Fri, 21 Jun 2024 10:46:11 -0700 Subject: [PATCH 16/18] Use as the prefix --- clang/docs/LanguageExtensions.rst | 4 ++-- clang/include/clang/CodeGen/ModuleBuilder.h | 2 +- clang/lib/CodeGen/CGDebugInfo.h | 2 +- clang/test/CodeGenCXX/debug-info-verbose-trap.cpp | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index 7bde31114bbd9..7217f1d668bb7 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -3501,13 +3501,13 @@ The debugging information would look as if it were produced for the following co .. code-block:: c++ __attribute__((always_inline)) - inline void "__trap_msg$check null$Argument must not be null!"() { + inline void "__clang_trap_msg$check null$Argument must not be null!"() { __builtin_trap(); } void foo(int* p) { if (p == nullptr) - "__trap_msg$check null$Argument must not be null!"(); + "__clang_trap_msg$check null$Argument must not be null!"(); } However, the generated code would not actually contain a call to the artificial diff --git a/clang/include/clang/CodeGen/ModuleBuilder.h b/clang/include/clang/CodeGen/ModuleBuilder.h index 4a6200ad25dea..6cccd13c0a0f9 100644 --- a/clang/include/clang/CodeGen/ModuleBuilder.h +++ b/clang/include/clang/CodeGen/ModuleBuilder.h @@ -28,7 +28,7 @@ namespace llvm { } // Prefix of the name of the artificial inline frame. -#define CLANG_TRAP_PREFIX "__trap_msg" +#define CLANG_TRAP_PREFIX "__clang_trap_msg" namespace clang { class CodeGenOptions; diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 5b48155ef26aa..4e5ed8540dcc3 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -617,7 +617,7 @@ class CGDebugInfo { /// /// * `<Prefix>:<Category>:<FailureMsg>` /// - /// `<Prefix>` is "__trap_msg". + /// `<Prefix>` is "__clang_trap_msg". /// /// This is used to store failure reasons for traps. llvm::DILocation *CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation, diff --git a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp index a8c78343b12a3..f492698ccab83 100644 --- a/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp +++ b/clang/test/CodeGenCXX/debug-info-verbose-trap.cpp @@ -24,7 +24,7 @@ char const constMsg[] = "hello"; // CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: "_Z2f0v", // CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], inlinedAt: ![[LOC20:.*]]) -// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__trap_msg$category1$Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__clang_trap_msg$category1$Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) // CHECK: ![[LOC20]] = !DILocation(line: [[@LINE+2]], column: 3, scope: ![[SUBPROG14]]) void f0() { __builtin_verbose_trap("category1", "Argument_must_not_be_null"); @@ -34,7 +34,7 @@ void f0() { // CHECK: ![[LOC23]] = !DILocation(line: 0, scope: ![[SUBPROG18]], inlinedAt: ![[LOC24:.*]]) // CHECK: ![[LOC24]] = !DILocation(line: [[@LINE+5]], column: 3, scope: ![[SUBPROG22]]) // CHECK: ![[LOC25]] = !DILocation(line: 0, scope: ![[SUBPROG26:.*]], inlinedAt: ![[LOC27:.*]]) -// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__trap_msg$category2$hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) +// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__clang_trap_msg$category2$hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}}) // CHECK: ![[LOC27]] = !DILocation(line: [[@LINE+3]], column: 3, scope: ![[SUBPROG22]]) void f1() { __builtin_verbose_trap("category1", "Argument_must_not_be_null"); >From 65671fff70b036e976bcb89cecb704b2975398c9 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Fri, 21 Jun 2024 11:04:47 -0700 Subject: [PATCH 17/18] Reject arguments using '$' --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/lib/Sema/SemaChecking.cpp | 16 +++++++++++----- clang/test/SemaCXX/verbose-trap.cpp | 1 + 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 46f054617f430..c9fb81f4620f0 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8876,7 +8876,7 @@ def err_expected_callable_argument : Error< def note_building_builtin_dump_struct_call : Note< "in call to printing function with arguments '(%0)' while dumping struct">; def err_builtin_verbose_trap_arg : Error< - "argument to __builtin_verbose_trap must be a pointer to a constant string">; + "argument to __builtin_verbose_trap must %select{be a pointer to a constant string|not contain $}0">; def err_atomic_load_store_uses_lib : Error< "atomic %select{load|store}0 requires runtime support that is not " diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 5d5b1d13e1ec9..b5cc173f719f7 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -175,17 +175,23 @@ static bool checkArgCount(Sema &S, CallExpr *Call, unsigned DesiredArgCount) { static bool checkBuiltinVerboseTrap(CallExpr *Call, Sema &S) { bool HasError = false; - for (int I = 0; I < Call->getNumArgs(); ++I) { + for (unsigned I = 0; I < Call->getNumArgs(); ++I) { Expr *Arg = Call->getArg(I); if (Arg->isValueDependent()) continue; - // FIXME: Add more checks and reject strings that can't be handled by - // debuggers. - if (!Arg->tryEvaluateString(S.Context).has_value()) { + std::optional<std::string> ArgString = Arg->tryEvaluateString(S.Context); + int DiagMsgKind = -1; + // Arguments must be pointers to constant strings and cannot use '$'. + if (!ArgString.has_value()) + DiagMsgKind = 0; + else if (ArgString->find('$') != std::string::npos) + DiagMsgKind = 1; + + if (DiagMsgKind >= 0) { S.Diag(Arg->getBeginLoc(), diag::err_builtin_verbose_trap_arg) - << Arg->getSourceRange(); + << DiagMsgKind << Arg->getSourceRange(); HasError = true; } } diff --git a/clang/test/SemaCXX/verbose-trap.cpp b/clang/test/SemaCXX/verbose-trap.cpp index a91dd01106cc1..2503f9860d9c3 100644 --- a/clang/test/SemaCXX/verbose-trap.cpp +++ b/clang/test/SemaCXX/verbose-trap.cpp @@ -27,6 +27,7 @@ void f(const char * arg) { __builtin_verbose_trap("", 0); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a constant string}} __builtin_verbose_trap(1, ""); // expected-error {{cannot initialize a parameter of type 'const char *' with an rvalue of type 'int'}} __builtin_verbose_trap(arg, ""); // expected-error {{argument to __builtin_verbose_trap must be a pointer to a constant string}} + __builtin_verbose_trap("cat$1", "hel$lo"); // expected-error 2 {{argument to __builtin_verbose_trap must not contain $}} __builtin_verbose_trap(category, reason); __builtin_verbose_trap(u8"cat1", u8"hello"); #if __cplusplus >= 202002L >From ae3907da7894da7c1c1fcb640283155d06d0d71f Mon Sep 17 00:00:00 2001 From: Akira Hatanaka <ahata...@gmail.com> Date: Mon, 24 Jun 2024 10:03:29 -0700 Subject: [PATCH 18/18] Define CLANG_TRAP_PREFIX as an inline constexpr StringRef --- clang/include/clang/CodeGen/ModuleBuilder.h | 3 ++- clang/lib/CodeGen/CGDebugInfo.cpp | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/CodeGen/ModuleBuilder.h b/clang/include/clang/CodeGen/ModuleBuilder.h index 6cccd13c0a0f9..5aba80b84fb92 100644 --- a/clang/include/clang/CodeGen/ModuleBuilder.h +++ b/clang/include/clang/CodeGen/ModuleBuilder.h @@ -15,6 +15,7 @@ #include "clang/AST/ASTConsumer.h" #include "clang/Basic/LLVM.h" +#include "llvm/ADT/StringRef.h" namespace llvm { class Constant; @@ -28,7 +29,7 @@ namespace llvm { } // Prefix of the name of the artificial inline frame. -#define CLANG_TRAP_PREFIX "__clang_trap_msg" +inline constexpr llvm::StringRef CLANG_TRAP_PREFIX = "__clang_trap_msg"; namespace clang { class CodeGenOptions; diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 73747a70c445f..c1f9fb9f9297f 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -3515,8 +3515,7 @@ llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor( llvm::DebugLoc TrapLocation, StringRef Category, StringRef FailureMsg) { // Create a debug location from `TrapLocation` that adds an artificial inline // frame. - const char *Prefix = CLANG_TRAP_PREFIX; - SmallString<64> FuncName(Prefix); + SmallString<64> FuncName(CLANG_TRAP_PREFIX); FuncName += "$"; FuncName += Category; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
