[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

MaskRay wrote:

> This broke a bot, I reverted and it's back green here: 
> [lab.llvm.org/buildbot/#/builders/272/builds/14069](https://lab.llvm.org/buildbot/#/builders/272/builds/14069)

Thanks. llvm-project/libc has a pattern like the following.

```
namespace libc {
double log2(double x);
}
extern "C" double log2(double);

namespace std {
using ::log2;
}
using std::log2;

namespace libc {
decltype(libc::log2) __log2_impl__ __asm__("log2");
decltype(libc::log2) log2 __attribute__((alias("log2")));
double __log2_impl__(double x) { return x; }
}
```


`ItaniumMangleContextImpl::mangleCXXName` called by `mangleName` (new code in 
this PR) asserts that the parameter is one of 
FunctionDecl/VariableDecl/TemplateParamObjectDecl and fails on a 
`UsingShadowDecl`.
We can check `FunctionDecl` before `mangleCXXName` (we only handle global 
namespace names, where variables are not mangled).

To make the regression test more useful, I'll place it in `CodeGen/alias.cpp` 
separately.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Mehdi Amini via cfe-commits]

joker-eph wrote:

This broke a bot, I reverted and it's back green here: 
https://lab.llvm.org/buildbot/#/builders/272/builds/14069

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay closed 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/87130

>From 23422a0b3af3e070fed5ae86ed0f67acec066c0a Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Fri, 29 Mar 2024 17:48:14 -0700
Subject: [PATCH 1/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
 =?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.5-bogner
---
 clang/lib/Sema/CMakeLists.txt |  1 +
 clang/lib/Sema/SemaDeclAttr.cpp   | 31 +--
 clang/test/AST/ast-dump-attr-json.cpp |  1 +
 clang/test/Sema/alias-unused.c| 30 +-
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index e8bff07ced0cfa..7657536fabd1eb 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   Core
+  Demangle
   FrontendHLSL
   FrontendOpenMP
   MC
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f25f3afd0f4af2..8ed5242f325e5e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -42,6 +42,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/Support/Error.h"
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);
+  free(Demangled);
+}
+
 static void handleIFuncAttr(Sema , Decl *D, const ParsedAttr ) {
   StringRef Str;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
@@ -1992,6 +2010,7 @@ static void handleIFuncAttr(Sema , Decl *D, const 
ParsedAttr ) {
 return;
   }
 
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) IFuncAttr(S.Context, AL, Str));
 }
 
@@ -2026,17 +2045,7 @@ static void handleAliasAttr(Sema , Decl *D, const 
ParsedAttr ) {
 }
   }
 
-  // Mark target used to prevent unneeded-internal-declaration warnings.
-  if (!S.LangOpts.CPlusPlus) {
-// FIXME: demangle Str for C++, as the attribute refers to the mangled
-// linkage name, not the pre-mangled identifier.
-const DeclarationNameInfo target((Str), AL.getLoc());
-LookupResult LR(S, target, Sema::LookupOrdinaryName);
-if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
-  for (NamedDecl *ND : LR)
-ND->markUsed(S.Context);
-  }
-
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) AliasAttr(S.Context, AL, Str));
 }
 
diff --git a/clang/test/AST/ast-dump-attr-json.cpp 
b/clang/test/AST/ast-dump-attr-json.cpp
index 051c2956abfdf7..883e584bfedf07 100644
--- a/clang/test/AST/ast-dump-attr-json.cpp
+++ b/clang/test/AST/ast-dump-attr-json.cpp
@@ -46,6 +46,7 @@ __thread __attribute__ ((tls_model ("local-exec"))) int 
tls_model_var;
 // CHECK-NEXT:"tokLen": 11
 // CHECK-NEXT:   }
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "isUsed": true,
 // CHECK-NEXT:  "name": "global_decl",
 // CHECK-NEXT:  "mangledName": "global_decl",
 // CHECK-NEXT:  "type": {
diff --git a/clang/test/Sema/alias-unused.c b/clang/test/Sema/alias-unused.c
index de9fc8cc737662..030b5737a93a21 100644
--- a/clang/test/Sema/alias-unused.c
+++ b/clang/test/Sema/alias-unused.c
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int 

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

MaskRay wrote:

> > I think I've addressed all comments and changed the patch to rebase on the 
> > improved tests.
> > @AaronBallman @tstellar Are you happy with the current version?
> 
> Please file an issue for this:
> 
> > I added microsoftDemangle tests to show the current behavior.
> > Since the feature that demangles to the function name without parameters 
> > (f3 instead of f3(int)) appears to be missing, I cannot > address 
> > -Wunused-function false positives for microsoftDemangle with reasonable 
> > time complexity.
> 
> and file a second one to come back and fix this issue once we can demangle 
> with parameters.

Created #88823 and #88825 

Thanks!

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Aaron Ballman via cfe-commits]

https://github.com/AaronBallman approved this pull request.

LG enough TM, thank you @MaskRay!

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Tom Stellard via cfe-commits]

https://github.com/tstellar commented:

I'm fine with this as long as @AaronBallman approves.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

> I think I've addressed all comments and changed the patch to rebase on the 
> improved tests.
> 
> @AaronBallman @tstellar Are you happy with the current version?

Please file an issue for this:

> I added microsoftDemangle tests to show the current behavior.
> Since the feature that demangles to the function name without parameters (f3 
> instead of f3(int)) appears to be missing, I cannot > address 
> -Wunused-function false positives for microsoftDemangle with reasonable time 
> complexity.

and file a second one to come back and fix this issue once we can demangle with 
parameters.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

MaskRay wrote:

I think I've addressed all comments and changed the patch to rebase on the 
improved tests.

@AaronBallman @tstellar Are you happy with the current version?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/87130

>From 23422a0b3af3e070fed5ae86ed0f67acec066c0a Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Fri, 29 Mar 2024 17:48:14 -0700
Subject: [PATCH 1/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
 =?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.5-bogner
---
 clang/lib/Sema/CMakeLists.txt |  1 +
 clang/lib/Sema/SemaDeclAttr.cpp   | 31 +--
 clang/test/AST/ast-dump-attr-json.cpp |  1 +
 clang/test/Sema/alias-unused.c| 30 +-
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index e8bff07ced0cfa..7657536fabd1eb 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   Core
+  Demangle
   FrontendHLSL
   FrontendOpenMP
   MC
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f25f3afd0f4af2..8ed5242f325e5e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -42,6 +42,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/Support/Error.h"
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);
+  free(Demangled);
+}
+
 static void handleIFuncAttr(Sema , Decl *D, const ParsedAttr ) {
   StringRef Str;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
@@ -1992,6 +2010,7 @@ static void handleIFuncAttr(Sema , Decl *D, const 
ParsedAttr ) {
 return;
   }
 
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) IFuncAttr(S.Context, AL, Str));
 }
 
@@ -2026,17 +2045,7 @@ static void handleAliasAttr(Sema , Decl *D, const 
ParsedAttr ) {
 }
   }
 
-  // Mark target used to prevent unneeded-internal-declaration warnings.
-  if (!S.LangOpts.CPlusPlus) {
-// FIXME: demangle Str for C++, as the attribute refers to the mangled
-// linkage name, not the pre-mangled identifier.
-const DeclarationNameInfo target((Str), AL.getLoc());
-LookupResult LR(S, target, Sema::LookupOrdinaryName);
-if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
-  for (NamedDecl *ND : LR)
-ND->markUsed(S.Context);
-  }
-
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) AliasAttr(S.Context, AL, Str));
 }
 
diff --git a/clang/test/AST/ast-dump-attr-json.cpp 
b/clang/test/AST/ast-dump-attr-json.cpp
index 051c2956abfdf7..883e584bfedf07 100644
--- a/clang/test/AST/ast-dump-attr-json.cpp
+++ b/clang/test/AST/ast-dump-attr-json.cpp
@@ -46,6 +46,7 @@ __thread __attribute__ ((tls_model ("local-exec"))) int 
tls_model_var;
 // CHECK-NEXT:"tokLen": 11
 // CHECK-NEXT:   }
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "isUsed": true,
 // CHECK-NEXT:  "name": "global_decl",
 // CHECK-NEXT:  "mangledName": "global_decl",
 // CHECK-NEXT:  "type": {
diff --git a/clang/test/Sema/alias-unused.c b/clang/test/Sema/alias-unused.c
index de9fc8cc737662..030b5737a93a21 100644
--- a/clang/test/Sema/alias-unused.c
+++ b/clang/test/Sema/alias-unused.c
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int 

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/87130

>From 23422a0b3af3e070fed5ae86ed0f67acec066c0a Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Fri, 29 Mar 2024 17:48:14 -0700
Subject: [PATCH 1/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
 =?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.5-bogner
---
 clang/lib/Sema/CMakeLists.txt |  1 +
 clang/lib/Sema/SemaDeclAttr.cpp   | 31 +--
 clang/test/AST/ast-dump-attr-json.cpp |  1 +
 clang/test/Sema/alias-unused.c| 30 +-
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index e8bff07ced0cfa..7657536fabd1eb 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   Core
+  Demangle
   FrontendHLSL
   FrontendOpenMP
   MC
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f25f3afd0f4af2..8ed5242f325e5e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -42,6 +42,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/Support/Error.h"
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);
+  free(Demangled);
+}
+
 static void handleIFuncAttr(Sema , Decl *D, const ParsedAttr ) {
   StringRef Str;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
@@ -1992,6 +2010,7 @@ static void handleIFuncAttr(Sema , Decl *D, const 
ParsedAttr ) {
 return;
   }
 
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) IFuncAttr(S.Context, AL, Str));
 }
 
@@ -2026,17 +2045,7 @@ static void handleAliasAttr(Sema , Decl *D, const 
ParsedAttr ) {
 }
   }
 
-  // Mark target used to prevent unneeded-internal-declaration warnings.
-  if (!S.LangOpts.CPlusPlus) {
-// FIXME: demangle Str for C++, as the attribute refers to the mangled
-// linkage name, not the pre-mangled identifier.
-const DeclarationNameInfo target((Str), AL.getLoc());
-LookupResult LR(S, target, Sema::LookupOrdinaryName);
-if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
-  for (NamedDecl *ND : LR)
-ND->markUsed(S.Context);
-  }
-
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) AliasAttr(S.Context, AL, Str));
 }
 
diff --git a/clang/test/AST/ast-dump-attr-json.cpp 
b/clang/test/AST/ast-dump-attr-json.cpp
index 051c2956abfdf7..883e584bfedf07 100644
--- a/clang/test/AST/ast-dump-attr-json.cpp
+++ b/clang/test/AST/ast-dump-attr-json.cpp
@@ -46,6 +46,7 @@ __thread __attribute__ ((tls_model ("local-exec"))) int 
tls_model_var;
 // CHECK-NEXT:"tokLen": 11
 // CHECK-NEXT:   }
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "isUsed": true,
 // CHECK-NEXT:  "name": "global_decl",
 // CHECK-NEXT:  "mangledName": "global_decl",
 // CHECK-NEXT:  "type": {
diff --git a/clang/test/Sema/alias-unused.c b/clang/test/Sema/alias-unused.c
index de9fc8cc737662..030b5737a93a21 100644
--- a/clang/test/Sema/alias-unused.c
+++ b/clang/test/Sema/alias-unused.c
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int 

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));

MaskRay wrote:

Created #88593   

But I don't think a TODO applies. Clang's behavior is probably desired. So this 
just records a difference.

This difference probably doesn't make a difference because people rarely use 
`alias` for C++ code.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused 
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function 
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}

MaskRay wrote:

To fully support qualified names in alias/ifunc attributes for the 
`-Wunused-function` diagnostic, it would take a lot of code due to complexity 
of C++ name lookup when namespaces are involved.

This PR intends to address the `ifunc` `-Wunused-function` issue. The C++ 
support is a side product. And it would seem odd to add lots of code to fix a 
`-Wunused-function` false positive related to qualified names (which people 
likely don't use).



https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; } // cxx-warning{{unused function 'f'}}
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD };
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; } // 
expected-warning{{unused function 'f0'}}
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("?f0@@YAHH@Z")));

nickdesaulniers wrote:

got it; please add a TODO that the overload of `f0` that accepts an `int` 
should not warn (similar to the TODO you added in 
clang/test/Sema/alias-unused.cpp for the namespace part.  Same for `g1` and the 
overload of `f3` that accepts no parameters.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));

nickdesaulniers wrote:

Please file a bug and link to it from a TODO comment in the tests and/or 
sources.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

https://github.com/nickdesaulniers approved this pull request.

LGTM; though I have a strong preference to get bugs on file and linked to 
regarding:
- issues with non-itanium mangling
- issues with namespaced identifiers.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused 
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function 
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}

nickdesaulniers wrote:

> may not fit

?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

https://github.com/nickdesaulniers edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

MaskRay wrote:

Ping:)

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Ed Maste via cfe-commits]

emaste wrote:

Giving this extra scrutiny is certainly warranted given the context, but #63957 
is a legitimate bug.


https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

MaskRay wrote:

> > I really appreciate the suggestions. `alias-unused.cpp` and 
> > `alias-unused-win.cpp` contain test improvement that should be pre-commited 
> > once they look good enough. Then this PR can be changed to show the 
> > difference.
> > On a separate note, I wanted to clarify that `-Wunused-function` false 
> > positives/negatives shouldn't be automatically considered security bugs. 
> > Categorizing all these warning improvements as "security bugs" would dilute 
> > the meaning of "security bugs" and make it harder to prioritize real 
> > vulnerabilities.
> > (
> > ```
> > What is a GCC security bug?
> > ===
> > 
> > A security bug is one that threatens the security of a system or
> > network, or might compromise the security of data stored on it.
> > In the context of GCC, there are multiple ways in which this might
> > happen and some common scenarios are detailed below.
> > ```
> > 
> > 
> > 
> >   
> > 
> > 
> >   
> > 
> > 
> > 
> >   
> > )
> > (An attacker can easily bypass warnings: remove `-Werror` (uncommon in 
> > distros anyway), remove `-Wall` (which covers `-Wunused-function`, or use a 
> > pragma to disable `-Wunused-function` locally. )
> > The description contains an example about name mangling differences 
> > ([#87130 
> > (files)](https://github.com/llvm/llvm-project/pull/87130/files#r1554029811))
> >  and I mentioned that "This inconsistency makes alias/ifunc difficult to 
> > use in C++ with portability."
> > ```
> > extern "C" {
> > static void f0() {}
> > // GCC: void g0() __attribute__((alias("_ZL2f0v")));
> > // Clang: void g0() __attribute__((alias("f0")));
> > }
> > ```
> > 
> > 
> > 
> >   
> > 
> > 
> >   
> > 
> > 
> > 
> >   
> > I added microsoftDemangle tests to show the current behavior. Since the 
> > feature that demangles to the function name without parameters (f3 instead 
> > of f3(int)) appears to be missing, I cannot address -Wunused-function false 
> > positives for microsoftDemangle with reasonable time complexity.
> 
> I don't believe we ARE trying to make that change to security definitions. 
> However, we are all being particularly security conscious of this patch 
> because the reporter is the person who JUST did the xz exploit over years. 
> IMO, this bug report (and the original C only fix) were used in part to help 
> cover his tracks, so being particularly careful here is paramount.

The description might derail from the primary purpose of the patch.

The -Wunused-function false positive was not used in part to cover the issue.
"Hans Jansen" claimed 5% improvement for his app that called the crc func and 
contributed that ifunc code.
Lasse Collin's commit 
(https://git.tukaani.org/?p=xz.git;a=commit;h=a37a2763383e6c204fe878e1416dd35e7711d3a9),
 confirmed with the author on IRC, aimed to fix a configure issue with Clang 
-Wall.
Lasse did not report the issue. "Jia Tan" did.

The attacker did not need to care about the -Wunused-function false positive.
They could remove `static` from the resolver function to avoid the 
-Wunused-function false positive.

Anyhow, the backdoor doesn't target Clang (`if test "x$CC" != 'xgcc' > 
/dev/null`), which might be related to other codegen discrepancy the attacker 
did not intend to control.


https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Ed Maste via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s

emaste wrote:

Git does not track file moves explicitly. You'll find that `git log --follow` 
will work equally well whether you use `git add` and `git rm` or `git mv`. It 
uses a heuristic to decide that when a file disappears and a similar file 
appears it was probably moved. 

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Erich Keane via cfe-commits]

erichkeane wrote:

> I really appreciate the suggestions. `alias-unused.cpp` and 
> `alias-unused-win.cpp` contain test improvement that should be pre-commited 
> once they look good enough. Then this PR can be changed to show the 
> difference.
> 
> On a separate note, I wanted to clarify that `-Wunused-function` false 
> positives/negatives shouldn't be automatically considered security bugs. 
> Categorizing all these warning improvements as "security bugs" would dilute 
> the meaning of "security bugs" and make it harder to prioritize real 
> vulnerabilities.
> 
> (
> 
> ```
> What is a GCC security bug?
> ===
> 
> A security bug is one that threatens the security of a system or
> network, or might compromise the security of data stored on it.
> In the context of GCC, there are multiple ways in which this might
> happen and some common scenarios are detailed below.
> ```
> 
> )
> 
> (An attacker can easily bypass warnings: remove `-Werror` (uncommon in 
> distros anyway), remove `-Wall` (which covers `-Wunused-function`, or use a 
> pragma to disable `-Wunused-function` locally. )
> 
> The description contains an example about name mangling differences 
> (https://github.com/llvm/llvm-project/pull/87130/files#r1554029811) and I 
> mentioned that "This inconsistency makes alias/ifunc difficult to use in C++ 
> with portability."
> 
> ```
> extern "C" {
> static void f0() {}
> // GCC: void g0() __attribute__((alias("_ZL2f0v")));
> // Clang: void g0() __attribute__((alias("f0")));
> }
> ```
> 
> I added microsoftDemangle tests to show the current behavior. Since the 
> feature that demangles to the function name without parameters (f3 instead of 
> f3(int)) appears to be missing, I cannot address -Wunused-function false 
> positives for microsoftDemangle with reasonable time complexity.

I don't believe we ARE trying to make that change to security definitions.  
However, we are all being particularly security conscious of this patch because 
the reporter is the person who JUST did the xz exploit over years.  IMO, this 
bug report (and the original C only fix) were used in part to help cover his 
tracks, so being particularly careful here is paramount.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Aaron Ballman via cfe-commits]

@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; } // cxx-warning{{unused function 'f'}}
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD };
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; } // 
expected-warning{{unused function 'f0'}}
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("?f0@@YAHH@Z")));

AaronBallman wrote:

FWIW, I use http://demangler.com/ to help check manglings, which isn't quite 
what you were asking for but might help.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; } // cxx-warning{{unused function 'f'}}
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD };
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; } // 
expected-warning{{unused function 'f0'}}
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("?f0@@YAHH@Z")));

MaskRay wrote:

> I'm guessing this test file is demonstrating the lack of msabi mangling 
> support? And will function as a change detector in the future?

Yes, a change detector. The test shall be pre-committed once good enough.

llvm-cxxfilt seems Itanium only I compiled the file locally and used 
`llvm-objdump -t a.o` to get the symbol name.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

MaskRay wrote:

I really appreciate the suggestions. `alias-unused.cpp` and 
`alias-unused-win.cpp` contain test improvement that should be pre-commited 
once they look good enough.
Then this PR can be changed to show the difference.

On a separate note, I wanted to clarify that `-Wunused-function` false 
positives/negatives shouldn't be automatically considered security bugs.
Categorizing all these warning improvements as "security bugs" would dilute the 
meaning of "security bugs" and make it harder to prioritize real 
vulnerabilities.

(
```
What is a GCC security bug?
===

A security bug is one that threatens the security of a system or
network, or might compromise the security of data stored on it.
In the context of GCC, there are multiple ways in which this might
happen and some common scenarios are detailed below.
```
)

(An attacker can easily bypass warnings: remove `-Werror` (uncommon in distros 
anyway), remove `-Wall` (which covers `-Wunused-function`, or use a pragma to 
disable `-Wunused-function` locally.
)

The description contains an example about name mangling differences 
(https://github.com/llvm/llvm-project/pull/87130/files#r1554029811)
and I mentioned that "This inconsistency makes alias/ifunc difficult to use in 
C++ with portability."

```
extern "C" {
static void f0() {}
// GCC: void g0() __attribute__((alias("_ZL2f0v")));
// Clang: void g0() __attribute__((alias("f0")));
}
```

I added microsoftDemangle tests to show the current behavior.
Since the feature that demangles to the function name without parameters (f3 
instead of f3(int)) appears to be missing, I cannot address -Wunused-function 
false positives for microsoftDemangle with reasonable time complexity.


https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -Wunused -verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; } // cxx-warning{{unused function 'f'}}
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD };
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; } // 
expected-warning{{unused function 'f0'}}
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("?f0@@YAHH@Z")));

nickdesaulniers wrote:

As someone not familiar with microsoft's ABI, is there a tool like 
`llvm-cxxfilt` that can help me check this mangling from the command line, on 
my Linux host?

---

I'm guessing this test file is demonstrating the lack of msabi mangling 
support? And will function as a change detector in the future?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1980,6 +1981,36 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. Due to name mangling, we look up the
+// demangled name ignoring parameters. This should handle the majority of use
+// cases while leaving namespace scope names unmarked.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = nullptr;
+  if (S.getASTContext().getCXXABIKind() != TargetCXXABI::Microsoft)
+Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  std::unique_ptr MC(S.Context.createMangleContext());
+  SmallString<256> Name;
+
+  const DeclarationNameInfo Target(
+  (Demangled ? Demangled : Str), AL.getLoc());
+  LookupResult LR(S, Target, Sema::LookupOrdinaryName);
+  if (S.LookupName(LR, S.TUScope)) {
+for (NamedDecl *ND : LR) {
+  if (MC->shouldMangleDeclName(ND)) {
+llvm::raw_svector_ostream Out(Name);
+Name.clear();
+MC->mangleName(GlobalDecl(ND), Out);
+  } else {
+Name = ND->getIdentifier()->getName();

nickdesaulniers wrote:

So then oops! My bug! And leads to the exact issue I was hypothesizing about 
wrt hiding things!

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}

nickdesaulniers wrote:

> foo is mangled.

for clang, not GCC. :disappointed: 

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));

nickdesaulniers wrote:

> Improving GCC consistency is not the goal of this PR. This PR intends to make 
> clangSema and clangCodeGen consistent.

Is there a bug on file wrt to this inconsistency? I would like to link to it 
from the test, sources, and public docs on the function attribute if we don't 
fix this inconsistency BEFORE landing this PR.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused 
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function 
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}

nickdesaulniers wrote:

Surely there's another case where we need to go from mangled string back to 
named decl.  Perhaps there's somewhere else in clang that already does so and 
can be reused here?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s

nickdesaulniers wrote:

> but git does not preserve file move history...

Sure it does; isn't that the point of the `--follow` flag to `git log `?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused 
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function 
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}
+int g3() __attribute__((alias("_ZN2nsL2f3Ev")));
+}
+
+template 
+static void *f4(T) { return nullptr; }
+static void *f4() { return nullptr; } // cxx-warning{{unused function 'f4'}}
+extern void g4() __attribute__((ifunc("_ZL2f4IiEPvT_")));
+void *use3 = f4(0);

MaskRay wrote:

Added a `char` instantiations without a warning. It will end up with an error 
in CodeGen.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused 
function 'resolver'}}

MaskRay wrote:

Yes.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1980,6 +1981,36 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. Due to name mangling, we look up the
+// demangled name ignoring parameters. This should handle the majority of use
+// cases while leaving namespace scope names unmarked.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = nullptr;
+  if (S.getASTContext().getCXXABIKind() != TargetCXXABI::Microsoft)
+Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  std::unique_ptr MC(S.Context.createMangleContext());
+  SmallString<256> Name;
+
+  const DeclarationNameInfo Target(
+  (Demangled ? Demangled : Str), AL.getLoc());
+  LookupResult LR(S, Target, Sema::LookupOrdinaryName);
+  if (S.LookupName(LR, S.TUScope)) {
+for (NamedDecl *ND : LR) {
+  if (MC->shouldMangleDeclName(ND)) {
+llvm::raw_svector_ostream Out(Name);
+Name.clear();
+MC->mangleName(GlobalDecl(ND), Out);
+  } else {
+Name = ND->getIdentifier()->getName();

MaskRay wrote:

The `if (Name == Str)` comparison below is to mark the correct overload while 
leaving others unmarked.

The previous code just doesn't bother with overload checking, leading to overly 
marked functions when extended to C++ / C `__attribute__((overload))`

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}

MaskRay wrote:

`foo` is mangled. This test just runs -fsyntax-only, not -c. If -c is 
specified, we will get an error:

```
% fclang -c x.cc
x.cc:2:47: error: alias must point to a defined variable or function
2 | extern typeof(foo) bar __attribute__((unused, alias("foo")));
  |   ^
x.cc:2:47: note: the function or variable specified in an alias must refer to 
its mangled name
x.cc:2:47: note: function by that name is mangled as "_ZL3foo"
2 | extern typeof(foo) bar __attribute__((unused, alias("foo")));
  |   ^~~~
  |   alias("_ZL3foo")
```

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused 
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function 
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}

MaskRay wrote:

To support `ns::f3`, it seems that we need to use 
`LookupNestedNameSpecifierName` first to find `ns`, then find `f3`. The process 
is quite complex and may not fit into the specific `mark*` function.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));

MaskRay wrote:

> Ah! right you pointed this out in your note. Perhaps we should fix/change 
> that first in clang?

I remember an existing issue about the mangling difference, but I don't recall 
the link now.
I have analyzed it a bit and run into a feature where we need mangling.
The discrepancy from GCC is indeed unfortunate and in practice makes 
`alias`/`ifunc` not useful for internal linkage targets in C++.

https://eel.is/c++draft/dcl.link gives an example

```cpp
extern "C" {
  static void f4(); // the name of the function f4 has internal 
linkage,
// so f4 has no language linkage; its type has 
C language linkage
}
```

I am unsure whether "no language linkage" means that ignoring the C language 
linkage is fine.

---

Improving GCC consistency is not the goal of this PR. This PR intends to make 
clangSema and clangCodeGen consistent. If we want to follow GCC in the future, 
the updated test will show the differences.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s

MaskRay wrote:

I did `git mv alias-unused.c alias-unused.cpp`, but git does not preserve file 
move history...

Once the tests are in a good shape, I will pre-commit the file and change the 
PR to show the diff.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/87130

>From 23422a0b3af3e070fed5ae86ed0f67acec066c0a Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Fri, 29 Mar 2024 17:48:14 -0700
Subject: [PATCH 1/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
 =?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.5-bogner
---
 clang/lib/Sema/CMakeLists.txt |  1 +
 clang/lib/Sema/SemaDeclAttr.cpp   | 31 +--
 clang/test/AST/ast-dump-attr-json.cpp |  1 +
 clang/test/Sema/alias-unused.c| 30 +-
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index e8bff07ced0cfa..7657536fabd1eb 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   Core
+  Demangle
   FrontendHLSL
   FrontendOpenMP
   MC
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f25f3afd0f4af2..8ed5242f325e5e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -42,6 +42,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/Support/Error.h"
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);
+  free(Demangled);
+}
+
 static void handleIFuncAttr(Sema , Decl *D, const ParsedAttr ) {
   StringRef Str;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
@@ -1992,6 +2010,7 @@ static void handleIFuncAttr(Sema , Decl *D, const 
ParsedAttr ) {
 return;
   }
 
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) IFuncAttr(S.Context, AL, Str));
 }
 
@@ -2026,17 +2045,7 @@ static void handleAliasAttr(Sema , Decl *D, const 
ParsedAttr ) {
 }
   }
 
-  // Mark target used to prevent unneeded-internal-declaration warnings.
-  if (!S.LangOpts.CPlusPlus) {
-// FIXME: demangle Str for C++, as the attribute refers to the mangled
-// linkage name, not the pre-mangled identifier.
-const DeclarationNameInfo target((Str), AL.getLoc());
-LookupResult LR(S, target, Sema::LookupOrdinaryName);
-if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
-  for (NamedDecl *ND : LR)
-ND->markUsed(S.Context);
-  }
-
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) AliasAttr(S.Context, AL, Str));
 }
 
diff --git a/clang/test/AST/ast-dump-attr-json.cpp 
b/clang/test/AST/ast-dump-attr-json.cpp
index 051c2956abfdf7..883e584bfedf07 100644
--- a/clang/test/AST/ast-dump-attr-json.cpp
+++ b/clang/test/AST/ast-dump-attr-json.cpp
@@ -46,6 +46,7 @@ __thread __attribute__ ((tls_model ("local-exec"))) int 
tls_model_var;
 // CHECK-NEXT:"tokLen": 11
 // CHECK-NEXT:   }
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "isUsed": true,
 // CHECK-NEXT:  "name": "global_decl",
 // CHECK-NEXT:  "mangledName": "global_decl",
 // CHECK-NEXT:  "type": {
diff --git a/clang/test/Sema/alias-unused.c b/clang/test/Sema/alias-unused.c
index de9fc8cc737662..030b5737a93a21 100644
--- a/clang/test/Sema/alias-unused.c
+++ b/clang/test/Sema/alias-unused.c
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int 

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

https://github.com/nickdesaulniers edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+
+static int (*resolver1(void))(void) { return f; }
+static int (*resolver1(int))(void) { return f; }
+int ifunc1(void) __attribute__((ifunc("_ZL9resolver1v")));
+
+namespace ns {
+static int f2(int) { return 42; } // expected-warning{{unused function 'f2'}}
+static int f2(void) { return 42; } // expected-warning{{unused function 'f2'}}

nickdesaulniers wrote:

`LookupParsedName` smells like it might handle namespaced lookups?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}

nickdesaulniers wrote:

Why should we warn here? Is it because `bar` is itself unused, or because its 
`alias` isn't mangled?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused 
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function 
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}
+int g3() __attribute__((alias("_ZN2nsL2f3Ev")));
+}
+
+template 
+static void *f4(T) { return nullptr; }
+static void *f4() { return nullptr; } // cxx-warning{{unused function 'f4'}}
+extern void g4() __attribute__((ifunc("_ZL2f4IiEPvT_")));
+void *use3 = f4(0);

nickdesaulniers wrote:

I'm curious what happens without the explicit usage. Template instantiation 
would fail.  Then we'd error that no such function was found? Perhaps consider 
adding another test case where that happens?  I'd like to ensure folks can't 
try to hide potential gadgets that way.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1980,6 +1981,36 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. Due to name mangling, we look up the
+// demangled name ignoring parameters. This should handle the majority of use
+// cases while leaving namespace scope names unmarked.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = nullptr;
+  if (S.getASTContext().getCXXABIKind() != TargetCXXABI::Microsoft)
+Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  std::unique_ptr MC(S.Context.createMangleContext());
+  SmallString<256> Name;
+
+  const DeclarationNameInfo Target(
+  (Demangled ? Demangled : Str), AL.getLoc());
+  LookupResult LR(S, Target, Sema::LookupOrdinaryName);
+  if (S.LookupName(LR, S.TUScope)) {
+for (NamedDecl *ND : LR) {
+  if (MC->shouldMangleDeclName(ND)) {
+llvm::raw_svector_ostream Out(Name);
+Name.clear();
+MC->mangleName(GlobalDecl(ND), Out);
+  } else {
+Name = ND->getIdentifier()->getName();

nickdesaulniers wrote:

The code before didn't need to compare `Str` against 
`ND->getIdentifier()->getName()`.  Is there a way to rewrite this loop such 
that we don't need to reassign `Name` or perform any comparisons when we should 
not mangle the decl name? Or did I have a bug in my code?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

https://github.com/nickdesaulniers edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));

nickdesaulniers wrote:

This seems wrong to me, a false negative.

FWICT, for C++ mode, even in an `extern "C"` block, it seems that functions 
with static linkage get mangled.  Or...  Oh, look, more behavioral differences 
from GCC: https://godbolt.org/z/sa4e7aYqj

We mangle, they do not.  That's going to lead to further compatibility issues 
when trying to use this function attribute.

---

Ah! right you pointed this out in your note.  Perhaps we should fix/change that 
first in clang?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused 
function 'resolver'}}

nickdesaulniers wrote:

Why warn here? (Probably same reason as for `foo` above.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

https://github.com/nickdesaulniers commented:

Progressing in the right direction and I'm in favor of fixing this issue in 
clang, with similar levels of caution as my fellow reviewers.

I think we can support Microsoft ABI mangling trivially here.

That we mangle globals with static linkage in `extern "C"` blocks looks to me 
like a bug in clang that should get fixed first BEFORE this lands.  Otherwise 
that seems like a massive compatibility issue.  Perhaps another approach would 
be to at least document this difference.  But then again, this behavioral 
difference is surprising, and I worry that it might be used to try to hide 
something.

The namespace lookup false positive is something that perhaps doesn't need to 
get fixed here; if we don't plan to remove that final false positive in the PR, 
then please file a bug @MaskRay and link to it from a TODO comment in the test 
case and/or sources.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c++ 
-verify=expected,cxx %s
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+static int f(void) { return 42; }
+int g(void) __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD }; // cxx-warning{{variable 'foo' is not 
needed and will not be emitted}}
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; } // cxx-warning{{unused 
function 'resolver'}}
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+static int __attribute__((overloadable)) f0(int x) { return x; }
+static float __attribute__((overloadable)) f0(float x) { return x; } // 
expected-warning{{unused function 'f0'}}
+int g0(void) __attribute__((alias("_ZL2f0i")));
+
+#ifdef __cplusplus
+static int f1() { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+
+static int f2(int) { return 42; } // cxx-warning{{unused function 'f2'}}
+static int f2() { return 42; }
+int g2() __attribute__((alias("_ZL2f2v")));
+
+static int (*resolver1())() { return f; } // cxx-warning{{unused function 
'resolver1'}}
+static int (*resolver1(int))() { return f; }
+int ifunc1() __attribute__((ifunc("_ZL9resolver1i")));
+
+/// We should report "unused function" for f3(int).
+namespace ns {
+static int f3(int) { return 42; } // cxx-warning{{unused function 'f3'}}
+static int f3() { return 42; } // cxx-warning{{unused function 'f3'}}

nickdesaulniers wrote:

> While our approach has false negatives for namespace scope names

Isn't this with `ns::f3()` here a false positive, not a false negative? 
Consider adding `TODO: ` to make it even more blatant.

Is this a bug in Sema::LookupOrdinaryName ?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Aaron Ballman via cfe-commits]

https://github.com/AaronBallman requested changes to this pull request.

Marking as requesting changes until we get agreement on the behavior for the 
Microsoft ABI.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Aaron Ballman via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

AaronBallman wrote:

> I added a `S.getASTContext().getCXXABIKind() != TargetCXXABI::Microsoft` 
> check.
> 
> Note: while `alias` works for Windows, it is only checked in tests 
> CodeGenCXX/visibility-dllstorageclass.cpp/CodeGen/debug-info-alias-pointer.c 
> . I don't consider an internal linkage target used or cared by users.

I'm not certain why there's so much resistance to doing this when three people 
have pointed out the concern. It still works for Windows. e.g.,
```
F:\source\llvm-project>cat "C:\Users\aballman\OneDrive - Intel 
Corporation\Desktop\test.cpp"
namespace NS {
  extern "C" int puts(const char *);
  void f() { puts("I did the thing!"); }
}

void func() __attribute__((alias("?f@NS@@YAXXZ")));

int main() {
  func();
}

F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe 
"C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp"

F:\source\llvm-project>a.exe
I did the thing!
```
We should support it properly rather than only halfway, *especially considering 
the context of the bug report*.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -Wunused -x c -verify %s

nickdesaulniers wrote:

consider using `git mv` when you rename files or move test cases. Makes it very 
obvious in code review when existing test cases are deleted vs moved and or 
updated.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

nickdesaulniers wrote:

ADT has a `FreeDeleter`; smells useful here.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);

MaskRay wrote:

I've added code to check the mangled name to avoid -Wunused-function false 
negatives.


https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/87130

>From 23422a0b3af3e070fed5ae86ed0f67acec066c0a Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Fri, 29 Mar 2024 17:48:14 -0700
Subject: [PATCH 1/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
 =?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.5-bogner
---
 clang/lib/Sema/CMakeLists.txt |  1 +
 clang/lib/Sema/SemaDeclAttr.cpp   | 31 +--
 clang/test/AST/ast-dump-attr-json.cpp |  1 +
 clang/test/Sema/alias-unused.c| 30 +-
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index e8bff07ced0cfa..7657536fabd1eb 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   Core
+  Demangle
   FrontendHLSL
   FrontendOpenMP
   MC
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f25f3afd0f4af2..8ed5242f325e5e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -42,6 +42,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/Support/Error.h"
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);
+  free(Demangled);
+}
+
 static void handleIFuncAttr(Sema , Decl *D, const ParsedAttr ) {
   StringRef Str;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
@@ -1992,6 +2010,7 @@ static void handleIFuncAttr(Sema , Decl *D, const 
ParsedAttr ) {
 return;
   }
 
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) IFuncAttr(S.Context, AL, Str));
 }
 
@@ -2026,17 +2045,7 @@ static void handleAliasAttr(Sema , Decl *D, const 
ParsedAttr ) {
 }
   }
 
-  // Mark target used to prevent unneeded-internal-declaration warnings.
-  if (!S.LangOpts.CPlusPlus) {
-// FIXME: demangle Str for C++, as the attribute refers to the mangled
-// linkage name, not the pre-mangled identifier.
-const DeclarationNameInfo target((Str), AL.getLoc());
-LookupResult LR(S, target, Sema::LookupOrdinaryName);
-if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
-  for (NamedDecl *ND : LR)
-ND->markUsed(S.Context);
-  }
-
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) AliasAttr(S.Context, AL, Str));
 }
 
diff --git a/clang/test/AST/ast-dump-attr-json.cpp 
b/clang/test/AST/ast-dump-attr-json.cpp
index 051c2956abfdf7..883e584bfedf07 100644
--- a/clang/test/AST/ast-dump-attr-json.cpp
+++ b/clang/test/AST/ast-dump-attr-json.cpp
@@ -46,6 +46,7 @@ __thread __attribute__ ((tls_model ("local-exec"))) int 
tls_model_var;
 // CHECK-NEXT:"tokLen": 11
 // CHECK-NEXT:   }
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "isUsed": true,
 // CHECK-NEXT:  "name": "global_decl",
 // CHECK-NEXT:  "mangledName": "global_decl",
 // CHECK-NEXT:  "type": {
diff --git a/clang/test/Sema/alias-unused.c b/clang/test/Sema/alias-unused.c
index de9fc8cc737662..030b5737a93a21 100644
--- a/clang/test/Sema/alias-unused.c
+++ b/clang/test/Sema/alias-unused.c
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int 

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name

MaskRay wrote:

Thanks for mentioning `overloadable`. Added a test and fixed the comment.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

MaskRay wrote:

C++20 (lambdas in unevaluated contexts) gives us `std::unique_ptr ptr((char*)malloc(10))`. With C++17, 
unique_ptr with `free` will add quite a bit boilerplate, which is probably not 
suitable...

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/87130

>From 23422a0b3af3e070fed5ae86ed0f67acec066c0a Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Fri, 29 Mar 2024 17:48:14 -0700
Subject: [PATCH 1/2] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
 =?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.5-bogner
---
 clang/lib/Sema/CMakeLists.txt |  1 +
 clang/lib/Sema/SemaDeclAttr.cpp   | 31 +--
 clang/test/AST/ast-dump-attr-json.cpp |  1 +
 clang/test/Sema/alias-unused.c| 30 +-
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index e8bff07ced0cfa..7657536fabd1eb 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   Core
+  Demangle
   FrontendHLSL
   FrontendOpenMP
   MC
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f25f3afd0f4af2..8ed5242f325e5e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -42,6 +42,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/Support/Error.h"
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);
+  free(Demangled);
+}
+
 static void handleIFuncAttr(Sema , Decl *D, const ParsedAttr ) {
   StringRef Str;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
@@ -1992,6 +2010,7 @@ static void handleIFuncAttr(Sema , Decl *D, const 
ParsedAttr ) {
 return;
   }
 
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) IFuncAttr(S.Context, AL, Str));
 }
 
@@ -2026,17 +2045,7 @@ static void handleAliasAttr(Sema , Decl *D, const 
ParsedAttr ) {
 }
   }
 
-  // Mark target used to prevent unneeded-internal-declaration warnings.
-  if (!S.LangOpts.CPlusPlus) {
-// FIXME: demangle Str for C++, as the attribute refers to the mangled
-// linkage name, not the pre-mangled identifier.
-const DeclarationNameInfo target((Str), AL.getLoc());
-LookupResult LR(S, target, Sema::LookupOrdinaryName);
-if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
-  for (NamedDecl *ND : LR)
-ND->markUsed(S.Context);
-  }
-
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) AliasAttr(S.Context, AL, Str));
 }
 
diff --git a/clang/test/AST/ast-dump-attr-json.cpp 
b/clang/test/AST/ast-dump-attr-json.cpp
index 051c2956abfdf7..883e584bfedf07 100644
--- a/clang/test/AST/ast-dump-attr-json.cpp
+++ b/clang/test/AST/ast-dump-attr-json.cpp
@@ -46,6 +46,7 @@ __thread __attribute__ ((tls_model ("local-exec"))) int 
tls_model_var;
 // CHECK-NEXT:"tokLen": 11
 // CHECK-NEXT:   }
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "isUsed": true,
 // CHECK-NEXT:  "name": "global_decl",
 // CHECK-NEXT:  "mangledName": "global_decl",
 // CHECK-NEXT:  "type": {
diff --git a/clang/test/Sema/alias-unused.c b/clang/test/Sema/alias-unused.c
index de9fc8cc737662..030b5737a93a21 100644
--- a/clang/test/Sema/alias-unused.c
+++ b/clang/test/Sema/alias-unused.c
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int 

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }

MaskRay wrote:

Added AST::createMangleContex and avoided false positives.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

MaskRay wrote:

I added a `S.getASTContext().getCXXABIKind() != TargetCXXABI::Microsoft` check.

Note: while `alias` works for Windows, it is only checked in tests 
CodeGenCXX/visibility-dllstorageclass.cpp/CodeGen/debug-info-alias-pointer.c . 
I don't consider it used publicly.

I've also updated the description to say that our behavior is different from 
GCC for `extern "C" { static void f(); }`. So, the C++ handling is best-effort.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/87130

>From 23422a0b3af3e070fed5ae86ed0f67acec066c0a Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Fri, 29 Mar 2024 17:48:14 -0700
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
 =?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.5-bogner
---
 clang/lib/Sema/CMakeLists.txt |  1 +
 clang/lib/Sema/SemaDeclAttr.cpp   | 31 +--
 clang/test/AST/ast-dump-attr-json.cpp |  1 +
 clang/test/Sema/alias-unused.c| 30 +-
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index e8bff07ced0cfa..7657536fabd1eb 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   Core
+  Demangle
   FrontendHLSL
   FrontendOpenMP
   MC
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f25f3afd0f4af2..8ed5242f325e5e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -42,6 +42,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/Support/Error.h"
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);
+  free(Demangled);
+}
+
 static void handleIFuncAttr(Sema , Decl *D, const ParsedAttr ) {
   StringRef Str;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
@@ -1992,6 +2010,7 @@ static void handleIFuncAttr(Sema , Decl *D, const 
ParsedAttr ) {
 return;
   }
 
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) IFuncAttr(S.Context, AL, Str));
 }
 
@@ -2026,17 +2045,7 @@ static void handleAliasAttr(Sema , Decl *D, const 
ParsedAttr ) {
 }
   }
 
-  // Mark target used to prevent unneeded-internal-declaration warnings.
-  if (!S.LangOpts.CPlusPlus) {
-// FIXME: demangle Str for C++, as the attribute refers to the mangled
-// linkage name, not the pre-mangled identifier.
-const DeclarationNameInfo target((Str), AL.getLoc());
-LookupResult LR(S, target, Sema::LookupOrdinaryName);
-if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
-  for (NamedDecl *ND : LR)
-ND->markUsed(S.Context);
-  }
-
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) AliasAttr(S.Context, AL, Str));
 }
 
diff --git a/clang/test/AST/ast-dump-attr-json.cpp 
b/clang/test/AST/ast-dump-attr-json.cpp
index 051c2956abfdf7..883e584bfedf07 100644
--- a/clang/test/AST/ast-dump-attr-json.cpp
+++ b/clang/test/AST/ast-dump-attr-json.cpp
@@ -46,6 +46,7 @@ __thread __attribute__ ((tls_model ("local-exec"))) int 
tls_model_var;
 // CHECK-NEXT:"tokLen": 11
 // CHECK-NEXT:   }
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "isUsed": true,
 // CHECK-NEXT:  "name": "global_decl",
 // CHECK-NEXT:  "mangledName": "global_decl",
 // CHECK-NEXT:  "type": {
diff --git a/clang/test/Sema/alias-unused.c b/clang/test/Sema/alias-unused.c
index de9fc8cc737662..030b5737a93a21 100644
--- a/clang/test/Sema/alias-unused.c
+++ b/clang/test/Sema/alias-unused.c
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) 

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

nickdesaulniers wrote:

> While we can add S.getASTContext().getCXXABIKind() != TargetCXXABI::Microsoft 
> check for downstream users, it doesn't appear to add any value?

If we know the target ABI uses Microsoft mangling, then we should be using 
`microsoftDemangle` rather than `itaniumDemangle`.  There's also 
`llvm::demangle` which can try to figure this out.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+
+static int (*resolver1(void))(void) { return f; }
+static int (*resolver1(int))(void) { return f; }
+int ifunc1(void) __attribute__((ifunc("_ZL9resolver1v")));
+
+namespace ns {
+static int f2(int) { return 42; } // expected-warning{{unused function 'f2'}}
+static int f2(void) { return 42; } // expected-warning{{unused function 'f2'}}
+int g2(void) __attribute__((alias("_ZN2nsL2f2Ev")));

nickdesaulniers wrote:

Dont forget `f1` and one of the overloads of `resolver1`.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }

nickdesaulniers wrote:

>but checking whether a declaration will have the specified mangled name is 
>challenging. 

Hence the TODO.

> We would need a MangleContext

Maybe `AST::createMangleContext` can help?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

MaskRay wrote:

> alias(mangled) referring to non-existent definitions are rejected by another 
> diagnostic.

It's an error
```
% fclang -c b.cc
b.cc:3:28: error: ifunc must point to a defined function
3 | int i1(int) __attribute__((ifunc("f")));
  |^
b.cc:3:28: note: the function specified in an ifunc must refer to its mangled 
name
b.cc:3:28: note: function by that name is mangled as "_Z1fv"
3 | int i1(int) __attribute__((ifunc("f")));
  |^~
  |ifunc("_Z1fv")
1 error generated.
```

The reported issue has nothing to do with security as all. It's about a 
`-Wunused-function` false positive. 
(https://git.tukaani.org/?p=xz.git;a=commitdiff;h=a37a2763383e6c204fe878e1416dd35e7711d3a9)

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Erich Keane via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

erichkeane wrote:

Isn't that diagnostic just a warning?  FWIW, I think being as 'defensive' as 
possible here based on the reporter is worth while.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Erich Keane via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;

erichkeane wrote:

Ah, right, my mistake, I thought it was modifying 'Str' as an 'out' parameter.  
Disregard.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/87130

>From 23422a0b3af3e070fed5ae86ed0f67acec066c0a Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Fri, 29 Mar 2024 17:48:14 -0700
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
 =?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.5-bogner
---
 clang/lib/Sema/CMakeLists.txt |  1 +
 clang/lib/Sema/SemaDeclAttr.cpp   | 31 +--
 clang/test/AST/ast-dump-attr-json.cpp |  1 +
 clang/test/Sema/alias-unused.c| 30 +-
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index e8bff07ced0cfa..7657536fabd1eb 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   Core
+  Demangle
   FrontendHLSL
   FrontendOpenMP
   MC
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f25f3afd0f4af2..8ed5242f325e5e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -42,6 +42,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/Support/Error.h"
@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);
+  free(Demangled);
+}
+
 static void handleIFuncAttr(Sema , Decl *D, const ParsedAttr ) {
   StringRef Str;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
@@ -1992,6 +2010,7 @@ static void handleIFuncAttr(Sema , Decl *D, const 
ParsedAttr ) {
 return;
   }
 
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) IFuncAttr(S.Context, AL, Str));
 }
 
@@ -2026,17 +2045,7 @@ static void handleAliasAttr(Sema , Decl *D, const 
ParsedAttr ) {
 }
   }
 
-  // Mark target used to prevent unneeded-internal-declaration warnings.
-  if (!S.LangOpts.CPlusPlus) {
-// FIXME: demangle Str for C++, as the attribute refers to the mangled
-// linkage name, not the pre-mangled identifier.
-const DeclarationNameInfo target((Str), AL.getLoc());
-LookupResult LR(S, target, Sema::LookupOrdinaryName);
-if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
-  for (NamedDecl *ND : LR)
-ND->markUsed(S.Context);
-  }
-
+  markUsedForAliasOrIfunc(S, D, AL, Str);
   D->addAttr(::new (S.Context) AliasAttr(S.Context, AL, Str));
 }
 
diff --git a/clang/test/AST/ast-dump-attr-json.cpp 
b/clang/test/AST/ast-dump-attr-json.cpp
index 051c2956abfdf7..883e584bfedf07 100644
--- a/clang/test/AST/ast-dump-attr-json.cpp
+++ b/clang/test/AST/ast-dump-attr-json.cpp
@@ -46,6 +46,7 @@ __thread __attribute__ ((tls_model ("local-exec"))) int 
tls_model_var;
 // CHECK-NEXT:"tokLen": 11
 // CHECK-NEXT:   }
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "isUsed": true,
 // CHECK-NEXT:  "name": "global_decl",
 // CHECK-NEXT:  "mangledName": "global_decl",
 // CHECK-NEXT:  "type": {
diff --git a/clang/test/Sema/alias-unused.c b/clang/test/Sema/alias-unused.c
index de9fc8cc737662..030b5737a93a21 100644
--- a/clang/test/Sema/alias-unused.c
+++ b/clang/test/Sema/alias-unused.c
@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) 

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;

MaskRay wrote:

This test passes asan. 

`itaniumDemangle` allocates memory, which will be freed. 

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

MaskRay wrote:

I agree that `ifunc` is ItaniumABI only and `alias` might be used by Windows 
but I don't know any users.

While we can add `S.getASTContext().getCXXABIKind() != TargetCXXABI::Microsoft` 
check for downstream users, it doesn't appear to add any value? 
`ifunc(mangled)` `alias(mangled)` referring to non-existent definitions are 
rejected by another diagnostic.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+
+static int (*resolver1(void))(void) { return f; }
+static int (*resolver1(int))(void) { return f; }
+int ifunc1(void) __attribute__((ifunc("_ZL9resolver1v")));
+
+namespace ns {
+static int f2(int) { return 42; } // expected-warning{{unused function 'f2'}}
+static int f2(void) { return 42; } // expected-warning{{unused function 'f2'}}

MaskRay wrote:

`c++filt -p <<< _ZNL2nsL2f2Ev => ns::f2`. We attempt to find `ns::f2` in the 
scope and will find nothing (we should use `f2`).

This weakness is mentioned in the description as "... does not handle namespace 
scope names"

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }

MaskRay wrote:

The false negatives are for overloads, which are tested here.

I agree that it would be nice not to take a compromise, but checking whether a 
declaration will have the specified mangled name is challenging. We'd need to 
replicate `clang/lib/CodeGen/CodeGenModule.cpp` `getMangledNameImpl` code here. 
We would need a MangleContext which currently lives in CodeGen ( 
`clang/lib/CodeGen/CGCXXABI.h`), inaccessible from Sema.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Fangrui Song via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);

MaskRay wrote:

"alias" can target variables. "ifunc" has an existing diagnostic to reject 
non-function targets.
```
% cat a.c
static int x;
extern void y() __attribute__((ifunc("x")));
% clang -c a.c
a.c:2:32: error: ifunc must point to a defined function
2 | extern void y() __attribute__((ifunc("x")));
  |^
1 error generated.
```

I am adding a template test:
```cpp
template 
static void *f3(T) { return nullptr; }
static void *f3() { return nullptr; } // expected-warning{{unused function 
'f3'}}
extern void g3() __attribute__((ifunc("_ZL2f3IiEPvT_")));
void *use3 = f3(0);
```

I think the current code does the right thing for templates.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

nickdesaulniers wrote:

> I could imagine an invalid ifunc/alias being used to 'hide' a function that 
> is otherwise not called (but I'm also not smart enough to figure out why that 
> would be en exploit).

If a TU is compiled without optimizations enabled, dead code elimination may 
not remove an usused function, which could be used to purposefully retain a 
gadget.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Erich Keane via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

erichkeane wrote:

Ah, good to know!  I think it is important that we match here, since that is 
the one we're going to 'find'.  Frankly, based on the reporter of this bug's 
transgression (PLUS his exploit directly involved ifuncs), I want us to be as 
careful with this as possible.  

The mangling should be the one we're mangling the current TU with at all costs. 
 I could imagine an invalid ifunc/alias being used to 'hide' a function that is 
otherwise not called (but I'm also not smart enough to figure out why that 
would be en exploit).

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Jon Roelofs via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

jroelofs wrote:

> I think ifunc is ELF/Itanium only

Darwin too, now. For both the frontend attribute, and the IR construct.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Erich Keane via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

erichkeane wrote:

> Looks like there's `getLangOpts().CXXABI`. `TargetCXXABI::Microsoft` is the 
> one that doesn't use itanium FWICT (looking at `ASTContext::createCXXABI`) 
> And `TargetInfo::getCXXABI` (looking at `ASTContext::createMangleContext`). 
> `isItaniumFamily` comes up, too.

Yep, Microsoft is the exception.  We DO typically use 
https://clang.llvm.org/doxygen/classclang_1_1TargetCXXABI.html to determine 
which.  


https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

https://github.com/nickdesaulniers edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

nickdesaulniers wrote:

Looks like there's `getLangOpts().CXXABI`.  `TargetCXXABI::Microsoft` is the 
one that doesn't use itanium FWICT (looking at `ASTContext::createCXXABI`)  And 
`TargetInfo::getCXXABI` (looking at `ASTContext::createMangleContext`).

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Erich Keane via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

erichkeane wrote:

Triple is the only way I am aware of IIRC.  So we should be checking that 
triple.  I think `ifunc` is ELF/Itanium only, and I'm unsure about `alias`.  
That said, I'm aware of at least 1 downstream (IIRC) that used an ELF system 
with an MSVC mangling (one of the offload compilers).

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+
+static int (*resolver1(void))(void) { return f; }
+static int (*resolver1(int))(void) { return f; }
+int ifunc1(void) __attribute__((ifunc("_ZL9resolver1v")));
+
+namespace ns {
+static int f2(int) { return 42; } // expected-warning{{unused function 'f2'}}
+static int f2(void) { return 42; } // expected-warning{{unused function 'f2'}}
+int g2(void) __attribute__((alias("_ZN2nsL2f2Ev")));

nickdesaulniers wrote:

If these functions are only checked in `__cplusplus` mode, omit the `void` from 
the parameter list?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }
+static int f1(void) { return 42; }
+int g1(void) __attribute__((alias("_ZL2f1v")));
+
+static int (*resolver1(void))(void) { return f; }
+static int (*resolver1(int))(void) { return f; }
+int ifunc1(void) __attribute__((ifunc("_ZL9resolver1v")));
+
+namespace ns {
+static int f2(int) { return 42; } // expected-warning{{unused function 'f2'}}
+static int f2(void) { return 42; } // expected-warning{{unused function 'f2'}}

nickdesaulniers wrote:

Why warn for `f2(void)`? Would `_ZNL2nsL2f2Ev` be the mangling? (an additional 
`L` between `N` and `2`)?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1,7 +1,35 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunused -x c++ -verify %s
+
+#ifdef __cplusplus
+extern "C" {
+#else
 // expected-no-diagnostics
+#endif
 static int f(void) { return 42; }
 int g(void) __attribute__((alias("f")));
 
 static int foo [] = { 42, 0xDEAD };
 extern typeof(foo) bar __attribute__((unused, alias("foo")));
+
+static int (*resolver(void))(void) { return f; }
+int ifunc(void) __attribute__((ifunc("resolver")));
+
+#ifdef __cplusplus
+}
+
+/// We demangle alias/ifunc target and mark all found functions as used.
+static int f1(int) { return 42; }

nickdesaulniers wrote:

This behavior differs from GCC; https://godbolt.org/z/fz8a153qY.  Even which 
function is retained for codegen purposes is different. That's sure to lead to 
compatibility issues.  So beyond this diagnostic related change, it looks like 
we may have a pre-existing codegen bug here, too.

---

Given an alias to `_ZL2f1v`, that demangles to `f1()`. So we _should_ diagnose 
that `f1(int)` is unused (`_ZL2f1i`).

Same for `_ZL9resolver1v` below.

You allude to fixing false positives in lieu of adding false negatives, but I 
would prefer to not have to compromise like that.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

https://github.com/nickdesaulniers commented:

As the author of https://reviews.llvm.org/D54188 and the TODO added from there, 
this patch is the right thing to do.  I fixed a false positive diagnostic for C 
and knew that the fix was insufficient for C++.

I do appreciate the abundance of caution here, but this looks generally like 
what we want to do.

>  We fix false positives while causing false negatives when overloads are 
> present.

Can we avoid the false negatives?

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);

nickdesaulniers wrote:

How does one specify a "non-itanium build?"

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

https://github.com/nickdesaulniers edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Erich Keane via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);

erichkeane wrote:

> I believe you can put `alias` attributes on non-functions: 
> https://godbolt.org/z/s1PbojYr4.

`ifuncs` cannot, so we perhaps need to be more careful there.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Nick Desaulniers via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);

nickdesaulniers wrote:

I believe you can put `alias` attributes on non-functions: 
https://godbolt.org/z/s1PbojYr4.

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Erich Keane via cfe-commits]

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Mark alias/ifunc targets used and consider mangled names (PR #87130)

[Erich Keane via cfe-commits]

@@ -1980,6 +1981,23 @@ static void handleWeakRefAttr(Sema , Decl *D, const 
ParsedAttr ) {
   D->addAttr(::new (S.Context) WeakRefAttr(S.Context, AL));
 }
 
+// Mark alias/ifunc target as used. For C++, we look up the demangled name
+// ignoring parameters. This should handle the majority of use cases while
+// leaveing false positives for namespace scope names and false negatives in
+// the presence of overloads.
+static void markUsedForAliasOrIfunc(Sema , Decl *D, const ParsedAttr ,
+StringRef Str) {
+  char *Demangled = llvm::itaniumDemangle(Str, /*ParseParams=*/false);
+  if (Demangled)
+Str = Demangled;
+  const DeclarationNameInfo target((Str), AL.getLoc());
+  LookupResult LR(S, target, Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+for (NamedDecl *ND : LR)
+  ND->markUsed(S.Context);

erichkeane wrote:

This loop doesn't seem quite right either.  First, we need to ensure we're only 
marking 'functions' as used, not all possible named decls I think.  

Second, we need to ensure we're doing a better job with testing, this doesn't 
seem to be adding enough test infrastructure to make sure we're picking up the 
right function.

Finally, what does this mean in the case of template 
declarations/instantiations?  We need to make sure we're marking those 
correctly (or not picking them up at all perhaps?).

https://github.com/llvm/llvm-project/pull/87130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits