https://github.com/delcypher commented:
Seems reasonable to me other than my coding style nits. I don't own this code
though so you should wait for approval from one of the code owners.
https://github.com/llvm/llvm-project/pull/139809
___
cfe-commits
@@ -2808,6 +2842,9 @@ void CodeGenFunction::EmitVTablePtrCheckForCast(QualType
T, Address Derived,
if (!SanOpts.has(SanitizerKind::CFICastStrict))
ClassDecl = LeastDerivedClassWithSameLayout(ClassDecl);
+ auto [Ordinal, SSK] = ParseCFITypeCheckKind(TCK);
--
https://github.com/delcypher edited
https://github.com/llvm/llvm-project/pull/139809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/delcypher edited
https://github.com/llvm/llvm-project/pull/139809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
delcypher wrote:
@thurstond So there is a significant overlap here with what my GSoC student
(@anthonyhatran) is working on right now (CC @MiB137). The project was proposed
[here](https://llvm.org/OpenProjects.html#clang-improve-trapping-ubsan-2025).
During @anthonyhatran 's GSoC project he wi
@@ -85,6 +85,92 @@ enum VariableTypeDescriptorKind : uint16_t {
//Miscellaneous Helper Methods
//======//
+static llvm::StringRef GetTrapMessageForHandler(SanitizerHandler ID) {
+ swit
@@ -85,6 +85,92 @@ enum VariableTypeDescriptorKind : uint16_t {
//Miscellaneous Helper Methods
//======//
+static llvm::StringRef GetTrapMessageForHandler(SanitizerHandler ID) {
+ swit
@@ -4059,8 +4154,16 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
auto Call = TrapBB->begin();
assert(isa(Call) && "Expected call in trap BB");
-Call->applyMergedLocation(Call->getDebugLoc(),
- Builder.getCurrentDebugLoca
@@ -85,6 +85,92 @@ enum VariableTypeDescriptorKind : uint16_t {
//Miscellaneous Helper Methods
//======//
+static llvm::StringRef GetTrapMessageForHandler(SanitizerHandler ID) {
---
@@ -85,6 +85,92 @@ enum VariableTypeDescriptorKind : uint16_t {
//Miscellaneous Helper Methods
//======//
+static llvm::StringRef GetTrapMessageForHandler(SanitizerHandler ID) {
+ swit
@@ -4059,8 +4154,16 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
auto Call = TrapBB->begin();
assert(isa(Call) && "Expected call in trap BB");
-Call->applyMergedLocation(Call->getDebugLoc(),
- Builder.getCurrentDebugLoca
@@ -4051,6 +4137,15 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID];
+ llvm::DILocation *TrapLocation = Builder.getCurrentDebugLocation();
+ llvm::StringRef Category = "UBSan Trap Reason";
d
@@ -4051,6 +4144,14 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID];
+ llvm::DILocation *TrapLocation = Builder.getCurrentDebugLocation();
+ llvm::StringRef Category = GetTrapMessageForHandler(CheckHandlerID
https://github.com/delcypher edited
https://github.com/llvm/llvm-project/pull/143758
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,10 @@
+// RUN: %clang -O0 -g -debug-info-kind=standalone -dwarf-version=5
-fsanitize=undefined \
+// RUN: -fsanitize-trap=undefined -emit-llvm -S -c %s -o - | FileCheck %s
+
+int add_overflow(int a, int b) {
+ return a + b;
+}
+
+// CHECK: call void @llvm.ubsantrap(i8
https://github.com/delcypher requested changes to this pull request.
Thanks for working on this. We're off to a good start. Once we have your test
cases in a good shape we should look at the failing tests to figure out why
they broke so we can fix them or adjust your implementation.
https://gi
delcypher wrote:
@anthonyhatran Here's an example for `SanitizerHandler::InvalidObjCCast`
```
// Build with -fsanitize=objc-cast -fsanitize-trap=objc-cast
@interface NSFastEnumerationState
@end
#define NSUInteger unsigned int
@interface NSArray
+(NSArray*) arrayWithObjects: (id) first, ...;
-
@@ -4041,7 +4133,8 @@ void CodeGenFunction::EmitUnreachable(SourceLocation Loc)
{
void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
SanitizerHandler CheckHandlerID,
-bool NoMerge) {
+
delcypher wrote:
@anthonyhatran Here's an example for `SanitizerHandler::AlignmentAssumption`
```
#include
int32_t* get_int(void) __attribute__((assume_aligned(16)));
void retrieve_int(void) {
int* i = get_int();
*i = 7;
}
```
https://github.com/llvm/llvm-project/pull/143758
_
@@ -0,0 +1,10 @@
+// RUN: %clang -O0 -g -debug-info-kind=standalone -dwarf-version=5
-fsanitize=undefined \
delcypher wrote:
@anthonyhatran We should probably use `%clang_cc1` instead as a lot of codegen
tests do that. That by-passes the clang driver (which is
https://github.com/delcypher edited
https://github.com/llvm/llvm-project/pull/143758
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
delcypher wrote:
@Michael137 Ping
https://github.com/llvm/llvm-project/pull/141997
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
delcypher wrote:
@vitalybuka
> > Should we just rename stuff of this patch into __clang_trap_msg ?
>
> Actually no.
>
> __clang_trap_msg_* is only for trap. __ubsan_check_* is for instructions
> evaluating check (as now there is a bonus: that __ubsan_check_* works as
> __clang_trap_msg_* on
@@ -85,6 +85,94 @@ enum VariableTypeDescriptorKind : uint16_t {
//Miscellaneous Helper Methods
//======//
+static llvm::StringRef GetUBSanTrapForHandler(SanitizerHandler ID) {
+ switch
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple arm64-apple-macosx14.0.0 -O0
-debug-info-kind=standalone -dwarf-version=5 \
+// RUN: -fsanitize=signed-integer-overflow
-fsanitize-trap=signed-integer-overflow -emit-llvm %s -o - | FileCheck %s
+
+int add_overflow(int a, int b) {
+ r
https://github.com/delcypher edited
https://github.com/llvm/llvm-project/pull/145967
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/delcypher requested changes to this pull request.
Looks like we need to fix a few more things. Did you resolve the issue with the
existing test cases failing?
https://github.com/llvm/llvm-project/pull/145967
___
cfe-commits mailing
@@ -4051,6 +4139,14 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID];
+ llvm::DILocation *TrapLocation = Builder.getCurrentDebugLocation();
+ llvm::StringRef TrapMessage = GetUBSanTrapForHandler(CheckHandlerI
https://github.com/delcypher edited
https://github.com/llvm/llvm-project/pull/145967
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
delcypher wrote:
@rapidsna @patrykstefanski @hnrklssn Adding you as reviewers just so you're
aware that this change is being worked on and that it will likely impact the
downstream Swift fork of Clang.
https://github.com/llvm/llvm-project/pull/145967
___
@@ -730,6 +754,18 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
options::OPT_fno_sanitize_recover_EQ);
RecoverableKinds &= Kinds;
+ // Parse any -fsanitize-trap=<...> flags the user provided, then
+ // diagnose any which do not have a matching -fsanitize=<...>
@@ -348,6 +348,30 @@ parseSanitizeSkipHotCutoffArgs(const Driver &D, const
llvm::opt::ArgList &Args,
return Cutoffs;
}
+// Given a set of mismatched bits, TrapOnly (bits the user asked to trap but
+// that aren’t actually enabled), emit a warning based on -fsanitize-trap=NA
@@ -874,4 +874,9 @@ def warn_drv_openacc_without_cir
: Warning<"OpenACC directives will result in no runtime behavior; use "
"-fclangir to enable runtime effect">,
InGroup;
+
+def warn_drv_sanitize_trap_mismatch : Warning<
+ "-fsanitize-trap=%0 has no e
https://github.com/delcypher edited
https://github.com/llvm/llvm-project/pull/147997
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -85,6 +85,96 @@ enum VariableTypeDescriptorKind : uint16_t {
//Miscellaneous Helper Methods
//======//
+static llvm::StringRef GetUBSanTrapForHandler(SanitizerHandler ID) {
-
@@ -85,6 +85,96 @@ enum VariableTypeDescriptorKind : uint16_t {
//Miscellaneous Helper Methods
//======//
+static llvm::StringRef GetUBSanTrapForHandler(SanitizerHandler ID) {
+ switch
delcypher wrote:
> What is debug info size impact?
We'll ask @anthonyhatran to run some experiments on a few open source projects
(if you have any that you interested in then particular please let us know).
I wonder how much this will matter in practice though because (at least for
Apple plat
delcypher wrote:
@fmayer
> I would prefer this to be behind a flag (not necessarily in this PR), even if
> it's default turned on. People might not want synthetic frames added to their
> debug information and the top frame to point to the real code; also as this
> somewhat overlaps with the l
@@ -730,6 +754,18 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
options::OPT_fno_sanitize_recover_EQ);
RecoverableKinds &= Kinds;
+ // Parse any -fsanitize-trap=<...> flags the user provided, then
+ // diagnose any which do not have a matching -fsanitize=<...>
https://github.com/delcypher requested changes to this pull request.
Looks good for the sketch patch. I have a few minor questions and nits that
I've left. Please add a few driver test cases to illustrate how you expect this
to work (including a test case that suppresses the warning).
https://
delcypher wrote:
@PiJoules
> Moving forward, I think it would be good to gather consensus on a better
> scheme with respect to verbose messages handled by the debugger. Rather than
> baking a custom format into the function name, we think it's more portable
> and straightforward to use stand
delcypher wrote:
@PiJoules
> There's a large number of tests. Many of these look like they're close to an
> existing test ... in these cases is it possible to just use the existing test
> file and add checks under a prefix?
I originally recommended that @anthonyhatran write new tests so this
delcypher wrote:
@PiJoules
> This might be a bit orthogonal to this patch, but I think rather than using
> the mangled dollar scheme used by `__builtin_verbose_trap` to encode the
> UBSan description, it might be cleaner to instead use dwarf's
> `DW_AT_description` to actually store the UBSa
https://github.com/delcypher edited
https://github.com/llvm/llvm-project/pull/147997
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/delcypher edited
https://github.com/llvm/llvm-project/pull/147997
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
delcypher wrote:
@MaskRay
> The current -fsanitize-trap behavior is intentional. See
https://discourse.llvm.org/t/clang-gsoc-2025-usability-improvements-for-trapping-undefined-behavior-sanitizer/84568/11
Yes we aware the current behavior is intentional. That however doesn't mean
it's the right
https://github.com/delcypher edited
https://github.com/llvm/llvm-project/pull/147997
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
401 - 447 of 447 matches
Mail list logo