hnrklssn created this revision.
Herald added a project: All.
delcypher added a comment.
hnrklssn updated this revision to Diff 474504.
hnrklssn added reviewers: delcypher, rapidsna, fcloutier, t.p.northover,
patrykstefanski.
hnrklssn published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
@hnrklssn Could you add a regression test?
hnrklssn added a comment.
Added regression test
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5321
+ SetLLVMFunctionAttributesForDefinition(D, Fn);
CodeGenFunction(*this).GenerateCode(GD, Fn, FI);
----------------
I'm a little worried about this ordering change here. Could this have some
unintended consequences?
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5321
+ SetLLVMFunctionAttributesForDefinition(D, Fn);
CodeGenFunction(*this).GenerateCode(GD, Fn, FI);
----------------
delcypher wrote:
> I'm a little worried about this ordering change here. Could this have some
> unintended consequences?
Yeah I was also a bit worried about that when making the change, since the
functions are both quite broad and I'm not familiar with them from before.
However it doesn't break any test cases, so I'm not sure what the consequences
would be exactly.
For reference, also moving the call to setNonAliasAttributes so that it is
still before the call to SetLLVMFunctionAttributesForDefinition breaks a ton of
test cases so I'm somewhat hopeful that we have good test coverage for this
type of change.
This aligns the behaviour with that of disabling optimisations for the
translation unit entirely. Not merging the traps allows us to keep
separate debug information for each, improving the debugging experience
when finding the cause for a ubsan trap.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D137714
Files:
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/ubsan-trap-debugloc.c
Index: clang/test/CodeGen/ubsan-trap-debugloc.c
===================================================================
--- clang/test/CodeGen/ubsan-trap-debugloc.c
+++ clang/test/CodeGen/ubsan-trap-debugloc.c
@@ -7,4 +7,16 @@
a = a + 1;
}
+void bar(volatile int a) __attribute__((optnone)) {
+ // CHECK: call void @llvm.ubsantrap(i8 0){{.*}} !dbg [[LOC2:![0-9]+]]
+ // CHECK: call void @llvm.ubsantrap(i8 0){{.*}} !dbg [[LOC3:![0-9]+]]
+ a = a + 1;
+ a = a + 1;
+}
+
+// With optimisations enabled the traps are merged and need to share a debug
location
// CHECK: [[LOC]] = !DILocation(line: 0
+
+// With optimisations disabled the traps are not merged and retain accurate
debug locations
+// CHECK: [[LOC2]] = !DILocation(line: 13, column: 9
+// CHECK: [[LOC3]] = !DILocation(line: 14, column: 9
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5318,10 +5318,10 @@
// Set CodeGen attributes that represent floating point environment.
setLLVMFunctionFEnvAttributes(D, Fn);
+ SetLLVMFunctionAttributesForDefinition(D, Fn);
CodeGenFunction(*this).GenerateCode(GD, Fn, FI);
setNonAliasAttributes(GD, Fn);
- SetLLVMFunctionAttributesForDefinition(D, Fn);
if (const ConstructorAttr *CA = D->getAttr<ConstructorAttr>())
AddGlobalCtor(Fn, CA->getPriority());
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3545,7 +3545,8 @@
TrapBBs.resize(CheckHandlerID + 1);
llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID];
- if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
+ if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB ||
+ CurFn->hasFnAttribute(llvm::Attribute::OptimizeNone)) {
TrapBB = createBasicBlock("trap");
Builder.CreateCondBr(Checked, Cont, TrapBB);
EmitBlock(TrapBB);
Index: clang/test/CodeGen/ubsan-trap-debugloc.c
===================================================================
--- clang/test/CodeGen/ubsan-trap-debugloc.c
+++ clang/test/CodeGen/ubsan-trap-debugloc.c
@@ -7,4 +7,16 @@
a = a + 1;
}
+void bar(volatile int a) __attribute__((optnone)) {
+ // CHECK: call void @llvm.ubsantrap(i8 0){{.*}} !dbg [[LOC2:![0-9]+]]
+ // CHECK: call void @llvm.ubsantrap(i8 0){{.*}} !dbg [[LOC3:![0-9]+]]
+ a = a + 1;
+ a = a + 1;
+}
+
+// With optimisations enabled the traps are merged and need to share a debug location
// CHECK: [[LOC]] = !DILocation(line: 0
+
+// With optimisations disabled the traps are not merged and retain accurate debug locations
+// CHECK: [[LOC2]] = !DILocation(line: 13, column: 9
+// CHECK: [[LOC3]] = !DILocation(line: 14, column: 9
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5318,10 +5318,10 @@
// Set CodeGen attributes that represent floating point environment.
setLLVMFunctionFEnvAttributes(D, Fn);
+ SetLLVMFunctionAttributesForDefinition(D, Fn);
CodeGenFunction(*this).GenerateCode(GD, Fn, FI);
setNonAliasAttributes(GD, Fn);
- SetLLVMFunctionAttributesForDefinition(D, Fn);
if (const ConstructorAttr *CA = D->getAttr<ConstructorAttr>())
AddGlobalCtor(Fn, CA->getPriority());
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3545,7 +3545,8 @@
TrapBBs.resize(CheckHandlerID + 1);
llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID];
- if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
+ if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB ||
+ CurFn->hasFnAttribute(llvm::Attribute::OptimizeNone)) {
TrapBB = createBasicBlock("trap");
Builder.CreateCondBr(Checked, Cont, TrapBB);
EmitBlock(TrapBB);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
