[PATCH] D135926: [clang] Fix crash with -funique-internal-linkage-names

2022-10-17 Thread Ellis Hoag via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG970e1ea01aa0: [clang] Fix crash with 
-funique-internal-linkage-names (authored by ellis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135926

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/unique-internal-linkage-names.c


Index: clang/test/CodeGen/unique-internal-linkage-names.c
===
--- /dev/null
+++ clang/test/CodeGen/unique-internal-linkage-names.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -S -emit-llvm -funique-internal-linkage-names -o - | 
FileCheck %s
+
+// Check that we do not crash when overloading extern functions.
+
+inline void overloaded_external() {}
+extern void overloaded_external();
+
+// CHECK: define internal void @overloaded_internal() [[ATTR:#[0-9]+]] {
+static void overloaded_internal() {}
+extern void overloaded_internal();
+
+void markUsed() {
+  overloaded_external();
+  overloaded_internal();
+}
+
+// CHECK: attributes [[ATTR]] =
+// CHECK-SAME: "sample-profile-suffix-elision-policy"="selected"
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2264,9 +2264,8 @@
   // Add "sample-profile-suffix-elision-policy" attribute for internal linkage
   // functions with -funique-internal-linkage-names.
   if (TargetDecl && CodeGenOpts.UniqueInternalLinkageNames) {
-if (isa(TargetDecl)) {
-  if (this->getFunctionLinkage(CalleeInfo.getCalleeDecl()) ==
-  llvm::GlobalValue::InternalLinkage)
+if (const auto *FD = dyn_cast_or_null(TargetDecl)) {
+  if (!FD->isExternallyVisible())
 FuncAttrs.addAttribute("sample-profile-suffix-elision-policy",
"selected");
 }


Index: clang/test/CodeGen/unique-internal-linkage-names.c
===
--- /dev/null
+++ clang/test/CodeGen/unique-internal-linkage-names.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -S -emit-llvm -funique-internal-linkage-names -o - | FileCheck %s
+
+// Check that we do not crash when overloading extern functions.
+
+inline void overloaded_external() {}
+extern void overloaded_external();
+
+// CHECK: define internal void @overloaded_internal() [[ATTR:#[0-9]+]] {
+static void overloaded_internal() {}
+extern void overloaded_internal();
+
+void markUsed() {
+  overloaded_external();
+  overloaded_internal();
+}
+
+// CHECK: attributes [[ATTR]] =
+// CHECK-SAME: "sample-profile-suffix-elision-policy"="selected"
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2264,9 +2264,8 @@
   // Add "sample-profile-suffix-elision-policy" attribute for internal linkage
   // functions with -funique-internal-linkage-names.
   if (TargetDecl && CodeGenOpts.UniqueInternalLinkageNames) {
-if (isa(TargetDecl)) {
-  if (this->getFunctionLinkage(CalleeInfo.getCalleeDecl()) ==
-  llvm::GlobalValue::InternalLinkage)
+if (const auto *FD = dyn_cast_or_null(TargetDecl)) {
+  if (!FD->isExternallyVisible())
 FuncAttrs.addAttribute("sample-profile-suffix-elision-policy",
"selected");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135926: [clang] Fix crash with -funique-internal-linkage-names

2022-10-14 Thread Sriraman Tallam via Phabricator via cfe-commits 
tmsriram accepted this revision.
tmsriram added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for fixing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135926

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

[PATCH] D135926: [clang] Fix crash with -funique-internal-linkage-names

2022-10-13 Thread Ellis Hoag via Phabricator via cfe-commits 
ellis created this revision.
Herald added a project: All.
ellis added reviewers: tmsriram, hoy, dblaikie.
ellis published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Calling `getFunctionLinkage(CalleeInfo.getCalleeDecl())` will crash when the 
declaration does not have a body, e.g., `extern void foo();`. Instead, we can 
use `isExternallyVisible()` to see if the delcaration has internal linkage.

I believe using `!isExternallyVisible()` is correct because the clang linkage 
must be `InternalLinkage` or `UniqueExternalLinkage`, both of which are 
"internal linkage" in llvm.
https://github.com/llvm/llvm-project/blob/9c26f51f5e178ac0fda98419e3a61d205d3b58b1/clang/include/clang/Basic/Linkage.h#L28-L40

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135926

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/unique-internal-linkage-names.c


Index: clang/test/CodeGen/unique-internal-linkage-names.c
===
--- /dev/null
+++ clang/test/CodeGen/unique-internal-linkage-names.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -S -emit-llvm -funique-internal-linkage-names -o - | 
FileCheck %s
+
+// Check that we do not crash when overloading extern functions.
+
+inline void overloaded_external() {}
+extern void overloaded_external();
+
+// CHECK: define internal void @overloaded_internal() [[ATTR:#[0-9]+]] {
+static void overloaded_internal() {}
+extern void overloaded_internal();
+
+void markUsed() {
+  overloaded_external();
+  overloaded_internal();
+}
+
+// CHECK: attributes [[ATTR]] =
+// CHECK-SAME: "sample-profile-suffix-elision-policy"="selected"
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2260,9 +2260,8 @@
   // Add "sample-profile-suffix-elision-policy" attribute for internal linkage
   // functions with -funique-internal-linkage-names.
   if (TargetDecl && CodeGenOpts.UniqueInternalLinkageNames) {
-if (isa(TargetDecl)) {
-  if (this->getFunctionLinkage(CalleeInfo.getCalleeDecl()) ==
-  llvm::GlobalValue::InternalLinkage)
+if (const auto *FD = dyn_cast_or_null(TargetDecl)) {
+  if (!FD->isExternallyVisible())
 FuncAttrs.addAttribute("sample-profile-suffix-elision-policy",
"selected");
 }


Index: clang/test/CodeGen/unique-internal-linkage-names.c
===
--- /dev/null
+++ clang/test/CodeGen/unique-internal-linkage-names.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -S -emit-llvm -funique-internal-linkage-names -o - | FileCheck %s
+
+// Check that we do not crash when overloading extern functions.
+
+inline void overloaded_external() {}
+extern void overloaded_external();
+
+// CHECK: define internal void @overloaded_internal() [[ATTR:#[0-9]+]] {
+static void overloaded_internal() {}
+extern void overloaded_internal();
+
+void markUsed() {
+  overloaded_external();
+  overloaded_internal();
+}
+
+// CHECK: attributes [[ATTR]] =
+// CHECK-SAME: "sample-profile-suffix-elision-policy"="selected"
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2260,9 +2260,8 @@
   // Add "sample-profile-suffix-elision-policy" attribute for internal linkage
   // functions with -funique-internal-linkage-names.
   if (TargetDecl && CodeGenOpts.UniqueInternalLinkageNames) {
-if (isa(TargetDecl)) {
-  if (this->getFunctionLinkage(CalleeInfo.getCalleeDecl()) ==
-  llvm::GlobalValue::InternalLinkage)
+if (const auto *FD = dyn_cast_or_null(TargetDecl)) {
+  if (!FD->isExternallyVisible())
 FuncAttrs.addAttribute("sample-profile-suffix-elision-policy",
"selected");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits