[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

2023-05-25 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D151393#4373764 , @smeenai wrote:

> I searched for `__clang_call_terminate` in the Clang test directory when I 
> was working on this diff. Most tests were testing calls to it, not 
> definitions. There were a couple of tests checking the definition, but they 
> seemed pretty targeted (e.g. 
> https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGenCXX/noexcept.cpp)
>  or auto-generated, so I figured having a separate test case for the specific 
> thing I cared about was best.

Thank you for the research!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151393/new/

https://reviews.llvm.org/D151393

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

2023-05-25 Thread Shoaib Meenai via Phabricator via cfe-commits 
smeenai added a comment.

I searched for `__clang_call_terminate` in the Clang test directory when I was 
working on this diff. Most tests were testing calls to it, not definitions. 
There were a couple of tests checking the definition, but they seemed pretty 
targeted (e.g. 
https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGenCXX/noexcept.cpp)
 or auto-generated, so I figured having a separate test case for the specific 
thing I cared about was best.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151393/new/

https://reviews.llvm.org/D151393

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

2023-05-25 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

LGTM. Is there an existing test that can merge in the `uwtable` test?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151393/new/

https://reviews.llvm.org/D151393

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

2023-05-25 Thread Shoaib Meenai via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f7b51e4ec09: [CodeGen] Make __clang_call_terminate have an 
unwind table entry (authored by smeenai).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151393/new/

https://reviews.llvm.org/D151393

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp


Index: clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions 
-emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,NOUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions 
-funwind-tables=1 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,SYNCUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions 
-funwind-tables=2 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,ASYNCUNWIND %s
+
+void caller(void callback()) noexcept { callback(); }
+
+// CHECK: define {{.*}}void @__clang_call_terminate({{[^)]*}}) #[[#ATTRNUM:]]
+// CHECK: attributes #[[#ATTRNUM]] = {
+// NOUNWIND-NOT: uwtable
+// NOUNWIND-SAME: }
+// SYNCUNWIND-SAME: uwtable(sync)
+// ASYNCUNWIND-SAME: uwtable{{ }}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4689,6 +4689,7 @@
   cast(fnRef.getCallee()->stripPointerCasts());
   if (fn->empty()) {
 CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, fn, /*IsThunk=*/false);
+CGM.SetLLVMFunctionAttributesForDefinition(nullptr, fn);
 fn->setDoesNotThrow();
 fn->setDoesNotReturn();
 


Index: clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,NOUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions -funwind-tables=1 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,SYNCUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions -funwind-tables=2 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,ASYNCUNWIND %s
+
+void caller(void callback()) noexcept { callback(); }
+
+// CHECK: define {{.*}}void @__clang_call_terminate({{[^)]*}}) #[[#ATTRNUM:]]
+// CHECK: attributes #[[#ATTRNUM]] = {
+// NOUNWIND-NOT: uwtable
+// NOUNWIND-SAME: }
+// SYNCUNWIND-SAME: uwtable(sync)
+// ASYNCUNWIND-SAME: uwtable{{ }}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4689,6 +4689,7 @@
   cast(fnRef.getCallee()->stripPointerCasts());
   if (fn->empty()) {
 CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, fn, /*IsThunk=*/false);
+CGM.SetLLVMFunctionAttributesForDefinition(nullptr, fn);
 fn->setDoesNotThrow();
 fn->setDoesNotReturn();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

2023-05-25 Thread Eli Friedman via Phabricator via cfe-commits 
efriedma accepted this revision.
efriedma added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151393/new/

https://reviews.llvm.org/D151393

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

2023-05-25 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

Makes sense to me too, but wait for approval by @efriedma


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151393/new/

https://reviews.llvm.org/D151393

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

2023-05-24 Thread Shoaib Meenai via Phabricator via cfe-commits 
smeenai updated this revision to Diff 525429.
smeenai added a comment.

Call SetLLVMFunctionAttributesForDefinition instead


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151393/new/

https://reviews.llvm.org/D151393

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp


Index: clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions 
-emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,NOUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions 
-funwind-tables=1 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,SYNCUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions 
-funwind-tables=2 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,ASYNCUNWIND %s
+
+void caller(void callback()) noexcept { callback(); }
+
+// CHECK: define {{.*}}void @__clang_call_terminate({{[^)]*}}) #[[#ATTRNUM:]]
+// CHECK: attributes #[[#ATTRNUM]] = {
+// NOUNWIND-NOT: uwtable
+// NOUNWIND-SAME: }
+// SYNCUNWIND-SAME: uwtable(sync)
+// ASYNCUNWIND-SAME: uwtable{{ }}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4689,6 +4689,7 @@
   cast(fnRef.getCallee()->stripPointerCasts());
   if (fn->empty()) {
 CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, fn, /*IsThunk=*/false);
+CGM.SetLLVMFunctionAttributesForDefinition(nullptr, fn);
 fn->setDoesNotThrow();
 fn->setDoesNotReturn();
 


Index: clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,NOUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions -funwind-tables=1 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,SYNCUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions -funwind-tables=2 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,ASYNCUNWIND %s
+
+void caller(void callback()) noexcept { callback(); }
+
+// CHECK: define {{.*}}void @__clang_call_terminate({{[^)]*}}) #[[#ATTRNUM:]]
+// CHECK: attributes #[[#ATTRNUM]] = {
+// NOUNWIND-NOT: uwtable
+// NOUNWIND-SAME: }
+// SYNCUNWIND-SAME: uwtable(sync)
+// ASYNCUNWIND-SAME: uwtable{{ }}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4689,6 +4689,7 @@
   cast(fnRef.getCallee()->stripPointerCasts());
   if (fn->empty()) {
 CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, fn, /*IsThunk=*/false);
+CGM.SetLLVMFunctionAttributesForDefinition(nullptr, fn);
 fn->setDoesNotThrow();
 fn->setDoesNotReturn();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

2023-05-24 Thread Eli Friedman via Phabricator via cfe-commits 
efriedma added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4698
+if (CodeGenOpts.UnwindTables)
+  fn->setUWTableKind(llvm::UWTableKind(CodeGenOpts.UnwindTables));
+

We probably want to call SetLLVMFunctionAttributesForDefinition() here instead. 
 The end result isn't that much different, but it indicates the intent more 
clearly, and hopefully we avoid hitting similar issues in this code in the 
future.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151393/new/

https://reviews.llvm.org/D151393

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

2023-05-24 Thread Shoaib Meenai via Phabricator via cfe-commits 
smeenai created this revision.
smeenai added reviewers: efriedma, jyknight, rnk, MaskRay.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
smeenai requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This enables debuggers to step past that frame on architectures that
don't use DWARF unwinding (such as armv7) even without debug info. The
problem should theoretically be architecture-agnostic, but according to
https://discourse.llvm.org/t/51633/2 it gets masked on architectures
that use DWARF unwind info.

Fixes https://github.com/llvm/llvm-project/issues/40696


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151393

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp


Index: clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions 
-emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,NOUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions 
-funwind-tables=1 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,SYNCUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions 
-funwind-tables=2 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,ASYNCUNWIND %s
+
+void caller(void callback()) noexcept { callback(); }
+
+// CHECK: define {{.*}}void @__clang_call_terminate({{[^)]*}}) #[[#ATTRNUM:]]
+// CHECK: attributes #[[#ATTRNUM]] = {
+// NOUNWIND-NOT: uwtable
+// NOUNWIND-SAME: }
+// SYNCUNWIND-SAME: uwtable(sync)
+// ASYNCUNWIND-SAME: uwtable{{ }}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4692,6 +4692,11 @@
 fn->setDoesNotThrow();
 fn->setDoesNotReturn();
 
+// Ensure stack traces can progress past this frame (e.g. for debuggers).
+const CodeGenOptions  = CGM.getCodeGenOpts();
+if (CodeGenOpts.UnwindTables)
+  fn->setUWTableKind(llvm::UWTableKind(CodeGenOpts.UnwindTables));
+
 // What we really want is to massively penalize inlining without
 // forbidding it completely.  The difference between that and
 // 'noinline' is negligible.


Index: clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,NOUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions -funwind-tables=1 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,SYNCUNWIND %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fexceptions -fcxx-exceptions -funwind-tables=2 -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,ASYNCUNWIND %s
+
+void caller(void callback()) noexcept { callback(); }
+
+// CHECK: define {{.*}}void @__clang_call_terminate({{[^)]*}}) #[[#ATTRNUM:]]
+// CHECK: attributes #[[#ATTRNUM]] = {
+// NOUNWIND-NOT: uwtable
+// NOUNWIND-SAME: }
+// SYNCUNWIND-SAME: uwtable(sync)
+// ASYNCUNWIND-SAME: uwtable{{ }}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4692,6 +4692,11 @@
 fn->setDoesNotThrow();
 fn->setDoesNotReturn();
 
+// Ensure stack traces can progress past this frame (e.g. for debuggers).
+const CodeGenOptions  = CGM.getCodeGenOpts();
+if (CodeGenOpts.UnwindTables)
+  fn->setUWTableKind(llvm::UWTableKind(CodeGenOpts.UnwindTables));
+
 // What we really want is to massively penalize inlining without
 // forbidding it completely.  The difference between that and
 // 'noinline' is negligible.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits