Stylie777 wrote:
It might be worth splitting each feature into its own commit rather than one
big commit, it makes the review easier. Currently it's difficult to determine
which section belongs to which feature.
https://github.com/llvm/llvm-project/pull/112341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jthackray wrote:
> It might be worth splitting each feature into its own commit rather than one
> big commit, it makes the review easier. Currently it's difficult to determine
> which section belongs to which feature.
Hmm, yeah possible.
https://github.com/llvm/llvm-project/pull/112341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Stylie777 updated
https://github.com/llvm/llvm-project/pull/112171
>From a702473aacc6a9c47eb80b204ee3200c2ff2eb26 Mon Sep 17 00:00:00 2001
From: Jack Styles
Date: Thu, 3 Oct 2024 14:20:10 +0100
Subject: [PATCH 1/6] [PAuthLR] Add support for FEAT_PAuth_LR to libunwind
This introduces support for unwinding programs where return addresses
have been signed using FEAT_PAuth_Lr, where the value of PC is used as
a diversifier (-mbranch-protection=pac-ret+pc).
A new vendor specific call frame instruction is added,
named `DW_CFA_AARCH64_negate_ra_state_with_pc`, to instruct the unwinder
tocapture the value of PC at the point of signing and update bit 1 of
the existing `RA_SIGN_STATE` pseudo-register to flag the need to use it
for authentication.
See https://github.com/ARM-software/abi-aa/pull/245 for the ABI change.
Authored-by: pratlucas
---
libunwind/src/DwarfInstructions.hpp | 54 ++---
libunwind/src/DwarfParser.hpp | 20 +++
libunwind/src/dwarf2.h | 3 +-
3 files changed, 64 insertions(+), 13 deletions(-)
diff --git a/libunwind/src/DwarfInstructions.hpp
b/libunwind/src/DwarfInstructions.hpp
index bd9ece60ee5881..e7c467de80adb6 100644
--- a/libunwind/src/DwarfInstructions.hpp
+++ b/libunwind/src/DwarfInstructions.hpp
@@ -74,8 +74,10 @@ class DwarfInstructions {
__builtin_unreachable();
}
#if defined(_LIBUNWIND_TARGET_AARCH64)
- static bool getRA_SIGN_STATE(A &addressSpace, R registers, pint_t cfa,
- PrologInfo &prolog);
+ static bool isReturnAddressSigned(A &addressSpace, R registers, pint_t cfa,
+PrologInfo &prolog);
+ static bool isReturnAddressSignedWithPC(A &addressSpace, R registers,
+ pint_t cfa, PrologInfo &prolog);
#endif
};
@@ -173,8 +175,9 @@ v128 DwarfInstructions::getSavedVectorRegister(
}
#if defined(_LIBUNWIND_TARGET_AARCH64)
template
-bool DwarfInstructions::getRA_SIGN_STATE(A &addressSpace, R registers,
- pint_t cfa, PrologInfo &prolog)
{
+bool DwarfInstructions::isReturnAddressSigned(A &addressSpace,
+R registers, pint_t cfa,
+PrologInfo &prolog) {
pint_t raSignState;
auto regloc = prolog.savedRegisters[UNW_AARCH64_RA_SIGN_STATE];
if (regloc.location == CFI_Parser::kRegisterUnused)
@@ -185,6 +188,22 @@ bool DwarfInstructions::getRA_SIGN_STATE(A
&addressSpace, R registers,
// Only bit[0] is meaningful.
return raSignState & 0x01;
}
+
+template
+bool DwarfInstructions::isReturnAddressSignedWithPC(A &addressSpace,
+ R registers,
+ pint_t cfa,
+ PrologInfo &prolog) {
+ pint_t raSignState;
+ auto regloc = prolog.savedRegisters[UNW_AARCH64_RA_SIGN_STATE];
+ if (regloc.location == CFI_Parser::kRegisterUnused)
+raSignState = static_cast(regloc.value);
+ else
+raSignState = getSavedRegister(addressSpace, registers, cfa, regloc);
+
+ // Only bit[1] is meaningful.
+ return raSignState & 0x02;
+}
#endif
template
@@ -288,7 +307,7 @@ int DwarfInstructions::stepWithDwarf(A &addressSpace,
pint_t pc,
// restored. autia1716 is used instead of autia as autia1716 assembles
// to a NOP on pre-v8.3a architectures.
if ((R::getArch() == REGISTERS_ARM64) &&
- getRA_SIGN_STATE(addressSpace, registers, cfa, prolog) &&
+ isReturnAddressSigned(addressSpace, registers, cfa, prolog) &&
returnAddress != 0) {
#if !defined(_LIBUNWIND_IS_NATIVE_ONLY)
return UNW_ECROSSRASIGNING;
@@ -296,13 +315,24 @@ int DwarfInstructions::stepWithDwarf(A
&addressSpace, pint_t pc,
register unsigned long long x17 __asm("x17") = returnAddress;
register unsigned long long x16 __asm("x16") = cfa;
-// These are the autia1716/autib1716 instructions. The hint
instructions
-// are used here as gcc does not assemble autia1716/autib1716 for pre
-// armv8.3a targets.
-if (cieInfo.addressesSignedWithBKey)
- asm("hint 0xe" : "+r"(x17) : "r"(x16)); // autib1716
-else
- asm("hint 0xc" : "+r"(x17) : "r"(x16)); // autia1716
+// We use the hint versions of the authentication instructions below to
+// ensure they're assembled by the compiler even for targets with no
+// FEAT_PAuth/FEAT_PAuth_LR support.
+if(isReturnAddressSignedWithPC(addressSpace, registers, cfa, prolog)) {
+ register unsigned long long x15 __asm("x15") =
prolog.ptrAuthDiversifier;
+ if(cieInfo.addressesSignedWithBKey) {
+asm("hint 0x27\n\t" // pacm
+"hint 0xe" : "+r"(x17) : "r"(x16), "r"(x15)); // autib1716
+ } els
mstorsjo wrote:
> I also bisected errors on Windows, down to this commit.
>
> The errors show up when running the compiler-rt testsuite, like this:
> https://github.com/mstorsjo/llvm-mingw/actions/runs/1135672/job/31598676534
> A number of tests fail, where `llvm-cov` errors out with `profile uses zlib
> compression but the profile reader was built without zlib support`.
>
> This can be reproduced with something as small as `llvm-cov show
> coverage_comments.cpp.exe -instr-profile coverage_comments.cpp.tmp.profdata`.
> Binaries of `llvm-cov` from before this change work fine, while binaries from
> after it produce this error.
This issue can be reproduced with a few prebuilt files:
https://martin.st/temp/profile/
Download those three files, build `llvm-cov` (on Linux or anywhere), and
execute this:
```
$ llvm-cov show coverage_comments.cpp.exe -instr-profile
coverage_comments.cpp.tmp.profdata
```
Before this breaking change, you'll get this:
```
error: C:/path/to/llvm-project/compiler-rt/test/profile/coverage_comments.cpp:
No such file or directory
warning: The file
'C:/path/to/llvm-project/compiler-rt/test/profile/coverage_comments.cpp' isn't
covered.
```
After this change, you instead get this:
```
error: failed to load coverage: 'coverage_comments.cpp.exe': failed to
uncompress data (zlib)
```
Can someone revert this change to get things back into working order?
https://github.com/llvm/llvm-project/pull/111332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -5903,7 +5904,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
auto Call = RValue::get(
EmitRuntimeCall(CGM.CreateRuntimeFunction(FTy, Name), Args));
if (TmpSize)
-EmitLifetimeEnd(TmpSize, TmpPtr);
+EmitLifetimeEnd(TmpSize, TmpPtr->stripPointerCasts());
AlexVlx wrote:
I would extremely strongly prefer not to mess with this ball of yarn or other
pieces of functionality (specifically, `CreateMemTemp`), as part of what is
essentially a point fix of a long standing bug. I'll leave a TODO in place for
someone that is braver and more skilled.
https://github.com/llvm/llvm-project/pull/112442
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kateinoigakukun wrote:
Thank you for your detail report and sorry for your inconvenience. I opened a
PR to revert the root cause: https://github.com/llvm/llvm-project/pull/112520
https://github.com/llvm/llvm-project/pull/111332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Mick235711 created
https://github.com/llvm/llvm-project/pull/112521
A follow-up and alternative proposal to #112289.
In this PR, no new notes are added to avoid noise. Instead, originally, every
`[[nodiscard]]` trigger, regardless of whether it is on function declaration or
return type, has the same warning. For instance, for the following program:
```cpp
struct S {};
struct [[nodiscard]] S2 {};
[[nodiscard]] S get1();
S2 get2();
int main() {
get1();
get2();
}
```
The generated warning currently is the same for two calls in `main()`:
([Compiler Explorer](https://godbolt.org/z/8bTa5oqMs))
```cpp
:6:3: warning: ignoring return value of function declared with
'nodiscard' attribute [-Wunused-result]
6 | get1();
| ^~~~
:7:3: warning: ignoring return value of function declared with
'nodiscard' attribute [-Wunused-result]
7 | get2();
| ^~~~
2 warnings generated.
```
As suggested by
https://github.com/llvm/llvm-project/pull/112289#issuecomment-2414341257, this
PR thus makes a distinction here by making `[[nodiscard]]` that are triggered
by its placement on return types specifically points out what the return type
is:
```cpp
test-3.cpp:6:3: warning: ignoring return value of function declared with
'nodiscard' attribute [-Werror,-Wunused-result]
6 | get1();
| ^~~~
test-3.cpp:7:3: warning: ignoring return value of type 'S2' declared with
'nodiscard' attribute [-Werror,-Wunused-value]
7 | get2();
| ^~~~
2 warnings generated.
```
In addition to better clarity, this also helps identify which specialization is
marked as `[[nodiscard]]`, as the standard allows `[[nodiscard]]` to be placed
on some specializations of a class template only, as demonstrated by
https://github.com/llvm/llvm-project/pull/112289#issuecomment-2414311233.
For constructor calls, a different warning message is also added. Currently,
for `[[nodiscard]]` (but not for `__attribute__((warn_unused_result))`),
temporaries that are discarded as a result of a constructor call generates a
different warning:
```cpp
warning: ignoring temporary created by a constructor declared with 'nodiscard'
attribute
```
After this PR, if `[[nodiscard]]` is placed on the constructor itself, nothing
changed. If `[[nodiscard]]` is placed on the type, the new warning is:
```cpp
warning: ignoring temporary of type 'S' declared with 'nodiscard' attribute
```
("created by a constructor" is removed since "declared with 'nodiscard'"
applies to the type, not the constructor)
One thing to note here is that for the following scenario:
```cpp
struct [[nodiscard]] S {
[[nodiscard]] S(int) {}
};
void use() { S(2); }
```
The warning message is generated in the format for constructor placement
("ignoring temporary created by a constructor"). Similarly, for
`[[nodiscard]]`/`warn_unused_result` on both functions and its return type, the
function attribute takes precedence as that is probably "more specific".
Compared to the original PR, `pure` and `const` are not touched, so there are
no builtin-related issues this time.
Some new test cases are added at the end of `SemaCXX/warn-unused-result.cpp` to
test the new warning.
>From f673e74f05756c44a0d16420949bd961c0464257 Mon Sep 17 00:00:00 2001
From: Yihe Li
Date: Wed, 16 Oct 2024 18:53:04 +0800
Subject: [PATCH] [clang] Improve diagnostic on [[nodiscard]] attribute
---
clang/include/clang/AST/Expr.h| 5 +-
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/AST/Expr.cpp| 18 +--
clang/lib/Sema/SemaStmt.cpp | 40 ---
.../dcl.attr/dcl.attr.nodiscard/p2.cpp| 28 ++---
.../dcl.attr/dcl.attr.nodiscard/p3.cpp| 2 +-
clang/test/Sema/c2x-nodiscard.c | 8 +-
clang/test/SemaCXX/warn-unused-result.cpp | 111 ++
8 files changed, 154 insertions(+), 64 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..8f5679767529fd 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3182,11 +3182,12 @@ class CallExpr : public Expr {
/// Returns the WarnUnusedResultAttr that is either declared on the called
/// function, or its return type declaration.
- const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+ std::pair
+ getUnusedResultAttr(const ASTContext &Ctx) const;
/// Returns true if this call expression should warn on unused results.
bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
}
SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c709795e7b21d8..f683bfe1df8dde 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/Di
github-actions[bot] wrote:
Thank you for submitting a Pull Request (PR) to the LLVM Project!
This PR will be automatically labeled and the relevant teams will be notified.
If you wish to, you can add reviewers by using the "Reviewers" section on this
page.
If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using `@` followed by their GitHub username.
If you have received no comments on your PR for a week, you can request a
review by "ping"ing the PR by adding a comment “Ping”. The common courtesy
"ping" rate is once a week. Please remember that you are asking for valuable
time from other developers.
If you have further questions, they may be answered by the [LLVM GitHub User
Guide](https://llvm.org/docs/GitHub.html).
You can also ask questions in a comment on this PR, on the [LLVM
Discord](https://discord.com/invite/xS7Z362) or on the
[forums](https://discourse.llvm.org/).
https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Yihe Li (Mick235711)
Changes
A follow-up and alternative proposal to #112289.
In this PR, no new notes are added to avoid noise. Instead, originally, every
`[[nodiscard]]` trigger, regardless of whether it is on function declaration or
return type, has the same warning. For instance, for the following program:
```cpp
struct S {};
struct [[nodiscard]] S2 {};
[[nodiscard]] S get1();
S2 get2();
int main() {
get1();
get2();
}
```
The generated warning currently is the same for two calls in `main()`:
([Compiler Explorer](https://godbolt.org/z/8bTa5oqMs))
```cpp
:6:3: warning: ignoring return value of function declared with
'nodiscard' attribute [-Wunused-result]
6 | get1();
| ^~~~
:7:3: warning: ignoring return value of function declared with
'nodiscard' attribute [-Wunused-result]
7 | get2();
| ^~~~
2 warnings generated.
```
As suggested by
https://github.com/llvm/llvm-project/pull/112289#issuecomment-2414341257, this
PR thus makes a distinction here by making `[[nodiscard]]` that are triggered
by its placement on return types specifically points out what the return type
is:
```cpp
test-3.cpp:6:3: warning: ignoring return value of function declared with
'nodiscard' attribute [-Werror,-Wunused-result]
6 | get1();
| ^~~~
test-3.cpp:7:3: warning: ignoring return value of type 'S2' declared with
'nodiscard' attribute [-Werror,-Wunused-value]
7 | get2();
| ^~~~
2 warnings generated.
```
In addition to better clarity, this also helps identify which specialization is
marked as `[[nodiscard]]`, as the standard allows `[[nodiscard]]` to be placed
on some specializations of a class template only, as demonstrated by
https://github.com/llvm/llvm-project/pull/112289#issuecomment-2414311233.
For constructor calls, a different warning message is also added. Currently,
for `[[nodiscard]]` (but not for `__attribute__((warn_unused_result))`),
temporaries that are discarded as a result of a constructor call generates a
different warning:
```cpp
warning: ignoring temporary created by a constructor declared with 'nodiscard'
attribute
```
After this PR, if `[[nodiscard]]` is placed on the constructor itself, nothing
changed. If `[[nodiscard]]` is placed on the type, the new warning is:
```cpp
warning: ignoring temporary of type 'S' declared with 'nodiscard' attribute
```
("created by a constructor" is removed since "declared with 'nodiscard'"
applies to the type, not the constructor)
One thing to note here is that for the following scenario:
```cpp
struct [[nodiscard]] S {
[[nodiscard]] S(int) {}
};
void use() { S(2); }
```
The warning message is generated in the format for constructor placement
("ignoring temporary created by a constructor"). Similarly, for
`[[nodiscard]]`/`warn_unused_result` on both functions and its return type, the
function attribute takes precedence as that is probably "more specific".
Compared to the original PR, `pure` and `const` are not touched, so there are
no builtin-related issues this time.
Some new test cases are added at the end of `SemaCXX/warn-unused-result.cpp` to
test the new warning.
---
Patch is 22.82 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/112521.diff
8 Files Affected:
- (modified) clang/include/clang/AST/Expr.h (+3-2)
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+6)
- (modified) clang/lib/AST/Expr.cpp (+10-8)
- (modified) clang/lib/Sema/SemaStmt.cpp (+26-14)
- (modified) clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp (+14-14)
- (modified) clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p3.cpp (+1-1)
- (modified) clang/test/Sema/c2x-nodiscard.c (+4-4)
- (modified) clang/test/SemaCXX/warn-unused-result.cpp (+90-21)
``diff
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index cbe62411d11bff..8f5679767529fd 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3182,11 +3182,12 @@ class CallExpr : public Expr {
/// Returns the WarnUnusedResultAttr that is either declared on the called
/// function, or its return type declaration.
- const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+ std::pair
+ getUnusedResultAttr(const ASTContext &Ctx) const;
/// Returns true if this call expression should warn on unused results.
bool hasUnusedResultAttr(const ASTContext &Ctx) const {
-return getUnusedResultAttr(Ctx) != nullptr;
+return getUnusedResultAttr(Ctx).second != nullptr;
}
SourceLocation getRParenLoc() const { return RParenLoc; }
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c709795e7b21d8..f683bfe1df8dde 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9265,6 +9265,12 @@ def warn_unus
Mick235711 wrote:
CC @Sirraide @erichkeane. (Sorry, I do not have permission to request
reviewers, so instead used ping)
https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1,69 +1,67 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm
-cl-ext=+cl_khr_subgroups -O0 -cl-std=clc++ -o - %s | FileCheck %s
-// FIXME: Add MS ABI manglings of OpenCL things and remove %itanium_abi_triple
arsenm wrote:
It's 2 different paths that both should work, with different output so both
should be tested
https://github.com/llvm/llvm-project/pull/112514
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2568,12 +2580,59 @@ defm CASPA : CompareAndSwapPair<1, 0, "a">;
defm CASPL : CompareAndSwapPair<0, 1, "l">;
defm CASPAL : CompareAndSwapPair<1, 1, "al">;
+// v9.6-a atomic CAST
CarolineConcatto wrote:
I believe that these CAS and CASP have alias too
https://github.com/llvm/llvm-project/pull/112341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mstorsjo wrote:
I also bisected errors on Windows, down to this commit.
The errors show up when running the compiler-rt testsuite, like this:
https://github.com/mstorsjo/llvm-mingw/actions/runs/1135672/job/31598676534
A number of tests fail, where `llvm-cov` errors out with `profile uses zlib
compression but the profile reader was built without zlib support`.
This can be reproduced with something as small as `llvm-cov show
coverage_comments.cpp.exe -instr-profile coverage_comments.cpp.tmp.profdata`.
Binaries of `llvm-cov` from before this change work fine, while binaries from
after it produce this error.
https://github.com/llvm/llvm-project/pull/111332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
glaubitz wrote:
Btw, if you submit a fix, it would be great if it could be backported to the 18
and 19 branches.
https://github.com/llvm/llvm-project/pull/111995
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zmodem wrote:
> LGTM! Can you also add a release note to clang/docs/ReleaseNotes.rst so users
> know about the new flag?
Sure, will do.
https://github.com/llvm/llvm-project/pull/112378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/bricknerb updated
https://github.com/llvm/llvm-project/pull/111856
>From 786d31e2657964e578cd1fdf2006b0fb3b19fab6 Mon Sep 17 00:00:00 2001
From: Boaz Brickner
Date: Thu, 10 Oct 2024 15:15:23 +
Subject: [PATCH 1/5] [clang] When checking for covariant return types, make
sure the pointers or references are to *classes*.
https://eel.is/c++draft/class.virtual#8.1
This prevents overriding methods with non class return types that have less
cv-qualification.
---
clang/lib/Sema/SemaDeclCXX.cpp | 4 ++--
clang/test/SemaCXX/virtual-override.cpp | 10 ++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9cb2ed02a3f764..6195b62b8afa16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18273,7 +18273,7 @@ bool Sema::CheckOverridingFunctionReturnType(const
CXXMethodDecl *New,
}
// The return types aren't either both pointers or references to a class
type.
- if (NewClassTy.isNull()) {
+ if (NewClassTy.isNull() || !NewClassTy->isStructureOrClassType()) {
Diag(New->getLocation(),
diag::err_different_return_type_for_overriding_virtual_function)
<< New->getDeclName() << NewTy << OldTy
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const
CXXMethodDecl *New,
diag::err_covariant_return_incomplete,
New->getDeclName()))
return true;
-}
+ }
// Check if the new class derives from the old class.
if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
diff --git a/clang/test/SemaCXX/virtual-override.cpp
b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f..6008b8ed063f20 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -289,3 +289,13 @@ namespace PR8168 {
static void foo() {} // expected-error{{'static' member function 'foo'
overrides a virtual function}}
};
}
+
+namespace T13 {
+ struct A {
+virtual const int *f() const; // expected-note{{overridden virtual
function is here}}
+ };
+
+ struct B : A {
+int *f() const override; // expected-error{{virtual function 'f' has a
different return type ('int *') than the function it overrides (which has
return type 'const int *')}}
+ };
+}
>From 027a985f2ca2bbe007db751af4fdbb5d8f12959d Mon Sep 17 00:00:00 2001
From: Boaz Brickner
Date: Fri, 11 Oct 2024 05:29:05 +
Subject: [PATCH 2/5] Fix indentation.
---
clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6195b62b8afa16..75d010dc4e04d8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const
CXXMethodDecl *New,
diag::err_covariant_return_incomplete,
New->getDeclName()))
return true;
- }
+}
// Check if the new class derives from the old class.
if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
>From 0b0452f3b7206fdea595aff684c329fd4563f631 Mon Sep 17 00:00:00 2001
From: Boaz Brickner
Date: Fri, 11 Oct 2024 12:27:00 +
Subject: [PATCH 3/5] [clang] Add the breaking change to more correctly check
covariance when return type doesn't point to a class to release notes.
---
clang/docs/ReleaseNotes.rst | 13 +
1 file changed, 13 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index df165b91252505..55ca61955c5b01 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -99,6 +99,19 @@ C++ Specific Potentially Breaking Changes
// Was error, now evaluates to false.
constexpr bool b = f() == g();
+- Clang will now correctly not consider pointers to non classes for covariance.
+
+ .. code-block:: c++
+
+ struct A {
+virtual const int *f() const;
+ };
+ struct B : A {
+// Return type has less cv-qualification but doesn't point to a class.
+// Error will be generated.
+int *f() const override;
+ };
+
ABI Changes in This Version
---
>From d5cf39d317ea2474477382f6dfa3a116c7db67f8 Mon Sep 17 00:00:00 2001
From: Boaz Brickner
Date: Fri, 11 Oct 2024 13:38:39 +
Subject: [PATCH 4/5] [clang] Fix code indentation in release notes for fixing
how we handle pointers to non classes when calculating covariance.
---
clang/docs/ReleaseNotes.rst | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 55ca61955c5b01..3ae716859fdcdf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -103,14 +103,14 @@ C++ Specific Potentially Breaking Changes
.. code-block:: c++
- struct
@@ -115,20 +115,20 @@ void usage() {
S('A'); // expected-warning {{ignoring temporary created by a constructor
declared with 'nodiscard' attribute: Don't let that S-Char go!}}
S(1);
S(2.2);
- Y(); // expected-warning {{ignoring temporary created by a constructor
declared with 'nodiscard' attribute: Don't throw me away either!}}
+ Y(); // expected-warning {{ignoring temporary of type 'Y' declared with
'nodiscard' attribute: Don't throw me away either!}}
S s;
- ConvertTo{}; // expected-warning {{ignoring return value of function
declared with 'nodiscard' attribute: Don't throw me away!}}
+ ConvertTo{}; // expected-warning {{ignoring return value of type 'ConvertTo'
declared with 'nodiscard' attribute: Don't throw me away!}}
Mick235711 wrote:
BTW, isn't this supposed to be a constructor call? Why is `ConvertTo{}`
different compared to `Y()` above?
https://github.com/llvm/llvm-project/pull/112521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
@@ -3767,28 +3764,26 @@ void
ExprEngine::evalEagerlyAssumeBinOpBifurcation(ExplodedNodeSet &Dst,
continue;
}
-ProgramStateRef state = Pred->getState();
-SVal V = state->getSVal(Ex, Pred->getLocationContext());
+ProgramStateRef State = Pred->getState();
+SVal V = State->getSVal(Ex, Pred->getLocationContext());
std::optional SEV = V.getAs();
if (SEV && SEV->isExpression()) {
- const std::pair &tags =
-geteagerlyAssumeBinOpBifurcationTags();
+ auto [TrueTag, FalseTag] = getEagerlyAssumeBifurcationTags();
- ProgramStateRef StateTrue, StateFalse;
- std::tie(StateTrue, StateFalse) = state->assume(*SEV);
+ auto [StateTrue, StateFalse] = State->assume(*SEV);
// First assume that the condition is true.
if (StateTrue) {
SVal Val = svalBuilder.makeIntVal(1U, Ex->getType());
StateTrue = StateTrue->BindExpr(Ex, Pred->getLocationContext(), Val);
-Bldr.generateNode(Ex, Pred, StateTrue, tags.first);
+Bldr.generateNode(Ex, Pred, StateTrue, TrueTag);
}
// Next, assume that the condition is false.
if (StateFalse) {
SVal Val = svalBuilder.makeIntVal(0U, Ex->getType());
StateFalse = StateFalse->BindExpr(Ex, Pred->getLocationContext(), Val);
-Bldr.generateNode(Ex, Pred, StateFalse, tags.second);
+Bldr.generateNode(Ex, Pred, StateFalse, FalseTag);
steakhal wrote:
I'd be fine with having a different tag there. Actually, that would be really
useful at times - e.g. with tooling around egraphs that is not present
upstream. So having a tag is not an issue. Having a tag that lies about that we
took an eager decision here is the problem. I agree that this shouldn't be done
part of this PR. And I'm also fine leaving this alone in the future. I just
wanted to raise this never the less.
https://github.com/llvm/llvm-project/pull/112209
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -5903,7 +5904,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
auto Call = RValue::get(
EmitRuntimeCall(CGM.CreateRuntimeFunction(FTy, Name), Args));
if (TmpSize)
-EmitLifetimeEnd(TmpSize, TmpPtr);
+EmitLifetimeEnd(TmpSize, TmpPtr->stripPointerCasts());
arsenm wrote:
Can still just do the stripPointerCasts once above
https://github.com/llvm/llvm-project/pull/112442
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
@@ -299,13 +299,12 @@ ANALYZER_OPTION(
ANALYZER_OPTION(
bool, ShouldEagerlyAssume, "eagerly-assume",
-"Whether we should eagerly assume evaluations of conditionals, thus, "
-"bifurcating the path. This indicates how the engine should handle "
-"expressions such as: 'x = (y != 0)'. When this is true then the "
-"subexpression 'y != 0' will be eagerly assumed to be true or false, thus "
-"evaluating it to the integers 0 or 1 respectively. The upside is that "
-"this can increase analysis precision until we have a better way to lazily
"
-"evaluate such logic. The downside is that it eagerly bifurcates paths.",
+"If this is enabled (the default behavior), when the analyzer encounters "
+"a comparison operator or logical negation, it immediately splits the "
steakhal wrote:
I don't get your response here. I thought we want to update the doc comments
here to reflect what the function does. I raised that I think implicit
conversions also trigger this code path (and do eager splits). Consequently, I
figured that this behavior should be also mentioned here and now.
If bool conversions don't trigger this function, then of course my suggestion
is void.
I didn't mean to suggest function changes by my comment.
About the TODO notes: I wish we had some lightweight list of things that we as
maintainers agree that is easy to do, we have a clear NFC or semantic change in
mind but nobody has the time doing it. I have in mind a lot of NFC refactors,
API cleanups where I never get to, but I wish would be done.
https://github.com/llvm/llvm-project/pull/112209
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy
Message-ID:
In-Reply-To:
https://github.com/steakhal approved this pull request.
LGTM, modulo single inline unresolved comment thread.
https://github.com/llvm/llvm-project/pull/112209
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tbaederr wrote:
Victory!
1) Iterate composite fields backwards for big-endian targets
2) Fix `BitcatBuffer::data()` pointing to the wrong byte if the number of
bytes in the buffer is not a multiple of `sizeof(uint64_t)` (which is what
`llvm::BitBuffer` uses internally) and we're on a big-endian host.
3) Validate that all of this works on little-endian and big-endian hosts, with
asan and ubsan enabled.
https://github.com/llvm/llvm-project/pull/112126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tbaederr wrote:
Also, I've removed the logic for tracking uninitialized/indeterminate bits for
now since it can be added later.
https://github.com/llvm/llvm-project/pull/112126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -437,7 +437,9 @@ ExprResult Parser::createEmbedExpr() {
SourceLocation StartLoc = ConsumeAnnotationToken();
if (Data->BinaryData.size() == 1) {
Res = IntegerLiteral::Create(Context,
- llvm::APInt(CHAR_BIT,
Data->BinaryData.back()),
+ llvm::APInt(CHAR_BIT, Data->BinaryData.back(),
nikic wrote:
Done, though I used `(unsigned char)` to match `CHAR_BIT` more closely.
https://github.com/llvm/llvm-project/pull/80309
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -446,8 +474,10 @@ define amdgpu_kernel void @add_i32_uniform(ptr
addrspace(1) %out, ptr addrspace(
; GFX11W64-NEXT:; implicit-def: $vgpr1
; GFX11W64-NEXT:s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) |
instid1(VALU_DEP_1)
; GFX11W64-NEXT:v_mbcnt_hi_u32_b32 v0, s5, v0
-; GFX11W64-NEXT:v_cmpx_eq_u32_e32 0, v0
-; GFX11W64-NEXT:s_cbranch_execz .LBB1_2
+; GFX11W64-NEXT:v_cmp_eq_u32_e32 vcc, 0, v0
+; GFX11W64-NEXT:s_cmp_lg_u64 vcc, 0
+; GFX11W64-NEXT:s_cmov_b64 exec, vcc
+; GFX11W64-NEXT:s_cbranch_scc0 .LBB1_2
alex-t wrote:
No, we cannot. S_AND_SAVEEXEC changes the EXEC unconditionally.
The idea is that we only change the exec if we are going to execute "Then"
block but leave it unchanged for Flow block. All further lowering is based on
this assumption.
https://github.com/llvm/llvm-project/pull/108596
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -268,7 +268,7 @@ struct UniqueVirtualMethod {
/// subobject in which that virtual function occurs).
class OverridingMethods {
using ValuesT = SmallVector;
- using MapType = llvm::MapVector;
+ using MapType = llvm::SmallMapVector;
nikic wrote:
It looks like this is nested inside another MapVector. I'd drop this one if it
doesn't have significant effect.
https://github.com/llvm/llvm-project/pull/111836
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -51,7 +51,7 @@ class ConstraintSystem {
/// A map of variables (IR values) to their corresponding index in the
/// constraint system.
- DenseMap Value2Index;
+ SmallDenseMap Value2Index;
nikic wrote:
I'd add a using/typedef for this one, especially as it needs to be synchronized
with ConstraintElimination.
https://github.com/llvm/llvm-project/pull/111836
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,14 @@
+# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
+// RUN: llvm-mc -triple aarch64-none-linux-gnu -show-encoding -mattr=+occmo
-mattr=+mte < %s | FileCheck %s
jthackray wrote:
I think these tests should also check instructions are rejected with
`-mattr=+nooccmo`
https://github.com/llvm/llvm-project/pull/112341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,12 @@
+# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
+// RUN: llvm-mc -triple aarch64 -show-encoding -mattr=+pcdphint %s | FileCheck
%s
jthackray wrote:
I think these tests should also check the instructions are rejected with
`-mattr=+nopcdphint`
https://github.com/llvm/llvm-project/pull/112341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1,69 +1,67 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm
-cl-ext=+cl_khr_subgroups -O0 -cl-std=clc++ -o - %s | FileCheck %s
-// FIXME: Add MS ABI manglings of OpenCL things and remove %itanium_abi_triple
arsenm wrote:
Test both run lines?
https://github.com/llvm/llvm-project/pull/112514
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
alex-t wrote:
> > Although, revisiting this now, I still don't understand why they decided to
> > include ALL spill opcodes in the prologue, but not only the SGPR spills?
> > Clearly, none of the VGPR reloads really belong to the prologue.
> > At a first glance, changing the isSpill(opcode) to isSGPRSpill(opcode) in
> > the snippet below would solve the initial case. ` return
> > IsNullOrVectorRegister && (isSpill(Opcode) || (!MI.isTerminator() && Opcode
> > != AMDGPU::COPY && MI.modifiesRegister(AMDGPU::EXEC, &RI))); }`
> > I need to look at this a bit more. I am sure they would have done this if
> > such a simple change had solved the problem.
>
> I like this change - it fixes the problem I reported in #109294.
It fixes the problem but requires us to consider WWM reloads as prologue
instructions. Any VGPR reload starts a new live range and possibly introduces
interference. CD's reply here
(https://github.com/llvm/llvm-project/pull/111496#issuecomment-2411434387) made
me hope that it would work. However, I would like to see the corresponding MIR
test as proof.
https://github.com/llvm/llvm-project/pull/108596
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1,69 +1,67 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm
-cl-ext=+cl_khr_subgroups -O0 -cl-std=clc++ -o - %s | FileCheck %s
-// FIXME: Add MS ABI manglings of OpenCL things and remove %itanium_abi_triple
svenvh wrote:
Surely I can add the old run line back, but would that bring any value?
Preservation of the pipe access qualifiers is not tested with the old run line
for example.
https://github.com/llvm/llvm-project/pull/112514
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/labrinea created
https://github.com/llvm/llvm-project/pull/112511
If we split these features in the compiler (see relevant pull request
https://github.com/llvm/llvm-project/pull/109299), we would only be able to
hand-write a 'memtag2' version using inline assembly since the compiler cannot
generate the instructions that become available with FEAT_MTE2. However these
instructions only work at Exception Level 1, so they would be unusable since
FMV is a user space facility. I am therefore unifying them.
Approved in ACLE as https://github.com/ARM-software/acle/pull/351
>From ae0d1f894f15d0af299210bb617dbaf905797a39 Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas
Date: Tue, 15 Oct 2024 17:43:25 +0100
Subject: [PATCH] [FMV][AArch64] Unify features memtag and memtag2.
If we split these features in the compiler (see relevant pull request
https://github.com/llvm/llvm-project/pull/109299), we would only be able
to hand-write a 'memtag2' version using inline assembly since the compiler
cannot generate the instructions that become available with FEAT_MTE2.
However these instructions only work at Exception Level 1, so they would
be unusable since FMV is a user space facility. I am therefore unifying
them.
Approved in ACLE as https://github.com/ARM-software/acle/pull/351
---
clang/include/clang/Basic/AttrDocs.td | 2 +-
clang/lib/Basic/Targets/AArch64.cpp | 2 +-
.../CodeGen/aarch64-cpu-supports-target.c | 2 +-
clang/test/CodeGen/aarch64-cpu-supports.c | 15 +-
clang/test/CodeGen/aarch64-fmv-dependencies.c | 9 +++--
.../test/CodeGen/attr-target-clones-aarch64.c | 20 +--
clang/test/CodeGen/attr-target-version.c | 16 +++
clang/test/Sema/attr-target-clones-aarch64.c | 2 +-
.../builtins/cpu_model/AArch64CPUFeatures.inc | 2 +-
.../builtins/cpu_model/aarch64/fmv/mrs.inc| 4 +---
.../llvm/TargetParser/AArch64CPUFeatures.inc | 2 +-
llvm/lib/Target/AArch64/AArch64FMV.td | 3 +--
12 files changed, 39 insertions(+), 40 deletions(-)
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index b1512e22ee2dd4..ee8126cadae232 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2669,7 +2669,7 @@ sign. For example:
.. code-block:: c++
-__attribute__((target_clones("sha2+memtag2", "fcma+sve2-pmull128")))
+__attribute__((target_clones("sha2+memtag", "fcma+sve2-pmull128")))
void foo() {}
For every multiversioned function a ``default`` (fallback) implementation
diff --git a/clang/lib/Basic/Targets/AArch64.cpp
b/clang/lib/Basic/Targets/AArch64.cpp
index b96fab978a3fcb..3dbba2b4d25bd6 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -784,7 +784,7 @@ bool AArch64TargetInfo::hasFeature(StringRef Feature) const
{
.Case("sme-fa64", HasSMEFA64)
.Case("sme-f16f16", HasSMEF16F16)
.Case("sme-b16b16", HasSMEB16B16)
- .Cases("memtag", "memtag2", HasMTE)
+ .Case("memtag", HasMTE)
.Case("sb", HasSB)
.Case("predres", HasPredRes)
.Cases("ssbs", "ssbs2", HasSSBS)
diff --git a/clang/test/CodeGen/aarch64-cpu-supports-target.c
b/clang/test/CodeGen/aarch64-cpu-supports-target.c
index 28187bcf745331..5186cab92a921d 100644
--- a/clang/test/CodeGen/aarch64-cpu-supports-target.c
+++ b/clang/test/CodeGen/aarch64-cpu-supports-target.c
@@ -17,7 +17,7 @@ int check_all_feature() {
return 7;
else if (__builtin_cpu_supports("sve2-bitperm+sve2-sha3+sve2-sm4"))
return 8;
- else if (__builtin_cpu_supports("sme+memtag+memtag2+memtag3+sb"))
+ else if (__builtin_cpu_supports("sme+memtag+memtag3+sb"))
return 9;
else if (__builtin_cpu_supports("predres+ssbs+ssbs2+bti+ls64+ls64_v"))
return 10;
diff --git a/clang/test/CodeGen/aarch64-cpu-supports.c
b/clang/test/CodeGen/aarch64-cpu-supports.c
index 823bf369df6fcc..dc96c929fdf4cb 100644
--- a/clang/test/CodeGen/aarch64-cpu-supports.c
+++ b/clang/test/CodeGen/aarch64-cpu-supports.c
@@ -1,9 +1,10 @@
-// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
UTC_ARGS: --check-globals --version 2
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
UTC_ARGS: --check-globals --global-value-regex ".*"
// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -emit-llvm -o - %s |
FileCheck %s
+//.
// CHECK: @__aarch64_cpu_features = external dso_local global { i64 }
-// CHECK-LABEL: define dso_local i32 @main
-// CHECK-SAME: () #[[ATTR0:[0-9]+]] {
+//.
+// CHECK-LABEL: @main(
// CHECK-NEXT: entry:
// CHECK-NEXT:[[RETVAL:%.*]] = alloca i32, align 4
// CHECK-NEXT:store i32 0, ptr [[RETVAL]], align 4
@@ -17,8 +18,8 @@
// CHECK-NEXT:br label [[RETURN:%.*]]
// CHECK: if.end:
// CHECK-NEXT:[[TMP4:%.*]] = load i64, ptr @__aarch64_cpu_features, align 8
-// CHECK-NEXT:[[TMP5:%.*]] = and i64 [[TMP4]],
llvmbot wrote:
@llvm/pr-subscribers-clang-tools-extra
Author: Discookie (Discookie)
Changes
Previously the name of anonymous enums in the check were `enum 'enum (unnamed
at /full/path/to/file.c:1:1)'`, which breaks reproducibility of clang-tidy
reports when the analyzed project is in a different folder.
We should think about removing emitted file paths globally from clang-tidy as
well, if appropriate. Or at least removing the file paths from all locations
where the warning/note tag already points to the declaration.
---
Full diff: https://github.com/llvm/llvm-project/pull/112496.diff
3 Files Affected:
- (modified) clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
(+16-6)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
(+11-1)
``diff
diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
index 1cb95c2b2347b7..5a48b103b9d602 100644
--- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
@@ -123,6 +123,13 @@ AST_MATCHER(EnumDecl, hasSequentialInitialValues) {
return !AllEnumeratorsArePowersOfTwo;
}
+std::string getName(const EnumDecl *Decl) {
+ if (!Decl->getDeclName())
+return "";
+
+ return Decl->getQualifiedNameAsString();
+}
+
} // namespace
EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name,
@@ -158,12 +165,15 @@ void EnumInitialValueCheck::registerMatchers(MatchFinder
*Finder) {
}
void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) {
+ PrintingPolicy PP = Result.Context->getPrintingPolicy();
+ PP.AnonymousTagLocations = false;
+
if (const auto *Enum = Result.Nodes.getNodeAs("inconsistent")) {
DiagnosticBuilder Diag =
diag(Enum->getBeginLoc(),
- "initial values in enum %0 are not consistent, consider explicit "
+ "initial values in enum '%0' are not consistent, consider
explicit "
"initialization of all, none or only the first enumerator")
-<< Enum;
+<< getName(Enum);
for (const EnumConstantDecl *ECD : Enum->enumerators())
if (ECD->getInitExpr() == nullptr) {
const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
@@ -183,16 +193,16 @@ void EnumInitialValueCheck::check(const
MatchFinder::MatchResult &Result) {
if (Loc.isInvalid() || Loc.isMacroID())
return;
DiagnosticBuilder Diag = diag(Loc, "zero initial value for the first "
- "enumerator in %0 can be disregarded")
- << Enum;
+ "enumerator in '%0' can be disregarded")
+ << getName(Enum);
cleanInitialValue(Diag, ECD, *Result.SourceManager, getLangOpts());
return;
}
if (const auto *Enum = Result.Nodes.getNodeAs("sequential")) {
DiagnosticBuilder Diag =
diag(Enum->getBeginLoc(),
- "sequential initial value in %0 can be ignored")
-<< Enum;
+ "sequential initial value in '%0' can be ignored")
+<< getName(Enum);
for (const EnumConstantDecl *ECD : llvm::drop_begin(Enum->enumerators()))
cleanInitialValue(Diag, ECD, *Result.SourceManager, getLangOpts());
return;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 95be0a89cd6c93..a4427ac222b475 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -232,7 +232,8 @@ Changes in existing checks
- Improved :doc:`readability-enum-initial-value
` check by only issuing
- diagnostics for the definition of an ``enum``, and by fixing a typo in the
+ diagnostics for the definition of an ``enum``, by not emitting a redundant
+ file path for anonymous enums in the diagnostic, and by fixing a typo in the
diagnostic.
- Improved :doc:`readability-implicit-bool-conversion
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
index b9a34d0683d7f3..54108585f030f8 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
@@ -53,6 +53,17 @@ enum EMacro2 {
// CHECK-FIXES: EMacro2_c = 3,
};
+
+enum {
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum
'' are not consistent
+ // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: initial values in enum
'' are not consistent
+ EAnonymous_a = 1,
+ EAnonymous_b,
+ // CHECK-FIXES: EAnonymous_b = 2,
+ EAnonymous_c = 3,
+};
+
+
enum EnumZeroFirstInitialValue {
EnumZeroFirstInitialValue_0 = 0,
// CHECK-MESSAGES-ENA
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
Author: Discookie (Discookie)
Changes
Previously the name of anonymous enums in the check were `enum 'enum (unnamed
at /full/path/to/file.c:1:1)'`, which breaks reproducibility of clang-tidy
reports when the analyzed project is in a different folder.
We should think about removing emitted file paths globally from clang-tidy as
well, if appropriate. Or at least removing the file paths from all locations
where the warning/note tag already points to the declaration.
---
Full diff: https://github.com/llvm/llvm-project/pull/112496.diff
3 Files Affected:
- (modified) clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
(+16-6)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
(+11-1)
``diff
diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
index 1cb95c2b2347b7..5a48b103b9d602 100644
--- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
@@ -123,6 +123,13 @@ AST_MATCHER(EnumDecl, hasSequentialInitialValues) {
return !AllEnumeratorsArePowersOfTwo;
}
+std::string getName(const EnumDecl *Decl) {
+ if (!Decl->getDeclName())
+return "";
+
+ return Decl->getQualifiedNameAsString();
+}
+
} // namespace
EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name,
@@ -158,12 +165,15 @@ void EnumInitialValueCheck::registerMatchers(MatchFinder
*Finder) {
}
void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) {
+ PrintingPolicy PP = Result.Context->getPrintingPolicy();
+ PP.AnonymousTagLocations = false;
+
if (const auto *Enum = Result.Nodes.getNodeAs("inconsistent")) {
DiagnosticBuilder Diag =
diag(Enum->getBeginLoc(),
- "initial values in enum %0 are not consistent, consider explicit "
+ "initial values in enum '%0' are not consistent, consider
explicit "
"initialization of all, none or only the first enumerator")
-<< Enum;
+<< getName(Enum);
for (const EnumConstantDecl *ECD : Enum->enumerators())
if (ECD->getInitExpr() == nullptr) {
const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
@@ -183,16 +193,16 @@ void EnumInitialValueCheck::check(const
MatchFinder::MatchResult &Result) {
if (Loc.isInvalid() || Loc.isMacroID())
return;
DiagnosticBuilder Diag = diag(Loc, "zero initial value for the first "
- "enumerator in %0 can be disregarded")
- << Enum;
+ "enumerator in '%0' can be disregarded")
+ << getName(Enum);
cleanInitialValue(Diag, ECD, *Result.SourceManager, getLangOpts());
return;
}
if (const auto *Enum = Result.Nodes.getNodeAs("sequential")) {
DiagnosticBuilder Diag =
diag(Enum->getBeginLoc(),
- "sequential initial value in %0 can be ignored")
-<< Enum;
+ "sequential initial value in '%0' can be ignored")
+<< getName(Enum);
for (const EnumConstantDecl *ECD : llvm::drop_begin(Enum->enumerators()))
cleanInitialValue(Diag, ECD, *Result.SourceManager, getLangOpts());
return;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 95be0a89cd6c93..a4427ac222b475 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -232,7 +232,8 @@ Changes in existing checks
- Improved :doc:`readability-enum-initial-value
` check by only issuing
- diagnostics for the definition of an ``enum``, and by fixing a typo in the
+ diagnostics for the definition of an ``enum``, by not emitting a redundant
+ file path for anonymous enums in the diagnostic, and by fixing a typo in the
diagnostic.
- Improved :doc:`readability-implicit-bool-conversion
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
index b9a34d0683d7f3..54108585f030f8 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
@@ -53,6 +53,17 @@ enum EMacro2 {
// CHECK-FIXES: EMacro2_c = 3,
};
+
+enum {
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum
'' are not consistent
+ // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: initial values in enum
'' are not consistent
+ EAnonymous_a = 1,
+ EAnonymous_b,
+ // CHECK-FIXES: EAnonymous_b = 2,
+ EAnonymous_c = 3,
+};
+
+
enum EnumZeroFirstInitialValue {
EnumZeroFirstInitialValue_0 = 0,
// CHECK-MESSAGES-ENABLE: :[
https://github.com/Discookie created
https://github.com/llvm/llvm-project/pull/112496
Previously the name of anonymous enums in the check were `enum 'enum (unnamed
at /full/path/to/file.c:1:1)'`, which breaks reproducibility of clang-tidy
reports when the analyzed project is in a different folder.
We should think about removing emitted file paths globally from clang-tidy as
well, if appropriate. Or at least removing the file paths from all locations
where the warning/note tag already points to the declaration.
>From be0bee7948a8a6394ba93df619d76048847c1ca8 Mon Sep 17 00:00:00 2001
From: Viktor
Date: Wed, 16 Oct 2024 08:07:19 +
Subject: [PATCH] [clang-tidy] Do not emit file path for anonymous enums in
`readability-enum-initial-value` check
---
.../readability/EnumInitialValueCheck.cpp | 22 ++-
clang-tools-extra/docs/ReleaseNotes.rst | 3 ++-
.../checkers/readability/enum-initial-value.c | 12 +-
3 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
index 1cb95c2b2347b7..5a48b103b9d602 100644
--- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
@@ -123,6 +123,13 @@ AST_MATCHER(EnumDecl, hasSequentialInitialValues) {
return !AllEnumeratorsArePowersOfTwo;
}
+std::string getName(const EnumDecl *Decl) {
+ if (!Decl->getDeclName())
+return "";
+
+ return Decl->getQualifiedNameAsString();
+}
+
} // namespace
EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name,
@@ -158,12 +165,15 @@ void EnumInitialValueCheck::registerMatchers(MatchFinder
*Finder) {
}
void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) {
+ PrintingPolicy PP = Result.Context->getPrintingPolicy();
+ PP.AnonymousTagLocations = false;
+
if (const auto *Enum = Result.Nodes.getNodeAs("inconsistent")) {
DiagnosticBuilder Diag =
diag(Enum->getBeginLoc(),
- "initial values in enum %0 are not consistent, consider explicit "
+ "initial values in enum '%0' are not consistent, consider
explicit "
"initialization of all, none or only the first enumerator")
-<< Enum;
+<< getName(Enum);
for (const EnumConstantDecl *ECD : Enum->enumerators())
if (ECD->getInitExpr() == nullptr) {
const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
@@ -183,16 +193,16 @@ void EnumInitialValueCheck::check(const
MatchFinder::MatchResult &Result) {
if (Loc.isInvalid() || Loc.isMacroID())
return;
DiagnosticBuilder Diag = diag(Loc, "zero initial value for the first "
- "enumerator in %0 can be disregarded")
- << Enum;
+ "enumerator in '%0' can be disregarded")
+ << getName(Enum);
cleanInitialValue(Diag, ECD, *Result.SourceManager, getLangOpts());
return;
}
if (const auto *Enum = Result.Nodes.getNodeAs("sequential")) {
DiagnosticBuilder Diag =
diag(Enum->getBeginLoc(),
- "sequential initial value in %0 can be ignored")
-<< Enum;
+ "sequential initial value in '%0' can be ignored")
+<< getName(Enum);
for (const EnumConstantDecl *ECD : llvm::drop_begin(Enum->enumerators()))
cleanInitialValue(Diag, ECD, *Result.SourceManager, getLangOpts());
return;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 95be0a89cd6c93..a4427ac222b475 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -232,7 +232,8 @@ Changes in existing checks
- Improved :doc:`readability-enum-initial-value
` check by only issuing
- diagnostics for the definition of an ``enum``, and by fixing a typo in the
+ diagnostics for the definition of an ``enum``, by not emitting a redundant
+ file path for anonymous enums in the diagnostic, and by fixing a typo in the
diagnostic.
- Improved :doc:`readability-implicit-bool-conversion
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
index b9a34d0683d7f3..54108585f030f8 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
@@ -53,6 +53,17 @@ enum EMacro2 {
// CHECK-FIXES: EMacro2_c = 3,
};
+
+enum {
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum
'' are not consistent
+ // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: initial values in enum
'' are not consistent
+ EAnonymous_a = 1,
+ EAnonymous_b,
+ // CHECK-FIXES: EAnonymous_b = 2,
+ EAn
Author: Balázs Kéri
Date: 2024-10-16T10:17:34+02:00
New Revision: 9df8d8d05c2650b51bd4233e1759206d163f3133
URL:
https://github.com/llvm/llvm-project/commit/9df8d8d05c2650b51bd4233e1759206d163f3133
DIFF:
https://github.com/llvm/llvm-project/commit/9df8d8d05c2650b51bd4233e1759206d163f3133.diff
LOG: [clang][analyzer] Improve test and documentation in cstring
NotNullTerminated checker (#112019)
CStringChecker has a sub-checker alpha.unix.cstring.NotNullTerminated
which checks for invalid objects passed to string functions. The checker
and its name are not exact and more functions could be checked, this
change only adds some tests and improves documentation.
Added:
Modified:
clang/docs/analyzer/checkers.rst
clang/test/Analysis/string.c
clang/test/Analysis/string.cpp
Removed:
diff --git a/clang/docs/analyzer/checkers.rst
b/clang/docs/analyzer/checkers.rst
index 81264428c72ed1..58dbd686a6dc9f 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3371,12 +3371,23 @@ Checks for overlap in two buffer arguments. Applies to:
``memcpy, mempcpy, wmem
alpha.unix.cstring.NotNullTerminated (C)
-Check for arguments which are not null-terminated strings; applies to:
``strlen, strnlen, strcpy, strncpy, strcat, strncat, wcslen, wcsnlen``.
+Check for arguments which are not null-terminated strings;
+applies to the ``strlen``, ``strcpy``, ``strcat``, ``strcmp`` family of
functions.
+
+Only very fundamental cases are detected where the passed memory block is
+absolutely
diff erent from a null-terminated string. This checker does not
+find if a memory buffer is passed where the terminating zero character
+is missing.
.. code-block:: c
- void test() {
- int y = strlen((char *)&test); // warn
+ void test1() {
+ int l = strlen((char *)&test); // warn
+ }
+
+ void test2() {
+ label:
+ int l = strlen((char *)&&label); // warn
}
.. _alpha-unix-cstring-OutOfBounds:
diff --git a/clang/test/Analysis/string.c b/clang/test/Analysis/string.c
index 79b4877eedbd9c..2e0a49d083b0b0 100644
--- a/clang/test/Analysis/string.c
+++ b/clang/test/Analysis/string.c
@@ -361,6 +361,10 @@ void strcpy_fn_const(char *x) {
strcpy(x, (const char*)&strcpy_fn); // expected-warning{{Argument to string
copy function is the address of the function 'strcpy_fn', which is not a
null-terminated string}}
}
+void strcpy_fn_dst(const char *x) {
+ strcpy((char*)&strcpy_fn, x); // expected-warning{{Argument to string copy
function is the address of the function 'strcpy_fn', which is not a
null-terminated string}}
+}
+
extern int globalInt;
void strcpy_effects(char *x, char *y) {
char a = x[0];
@@ -469,8 +473,22 @@ void strcat_null_src(char *x) {
strcat(x, NULL); // expected-warning{{Null pointer passed as 2nd argument to
string concatenation function}}
}
-void strcat_fn(char *x) {
- strcat(x, (char*)&strcat_fn); // expected-warning{{Argument to string
concatenation function is the address of the function 'strcat_fn', which is not
a null-terminated string}}
+void strcat_fn_dst(const char *x) {
+ strcat((char*)&strcat_fn_dst, x); // expected-warning{{Argument to string
concatenation function is the address of the function 'strcat_fn_dst', which is
not a null-terminated string}}
+}
+
+void strcat_fn_src(char *x) {
+ strcat(x, (char*)&strcat_fn_src); // expected-warning{{Argument to string
concatenation function is the address of the function 'strcat_fn_src', which is
not a null-terminated string}}
+}
+
+void strcat_label_dst(const char *x) {
+label:
+ strcat((char*)&&label, x); // expected-warning{{Argument to string
concatenation function is the address of the label 'label', which is not a
null-terminated string}}
+}
+
+void strcat_label_src(char *x) {
+label:
+ strcat(x, (char*)&&label); // expected-warning{{Argument to string
concatenation function is the address of the label 'label', which is not a
null-terminated string}}
}
void strcat_effects(char *y) {
@@ -568,8 +586,12 @@ void strncpy_null_src(char *x) {
strncpy(x, NULL, 5); // expected-warning{{Null pointer passed as 2nd
argument to string copy function}}
}
-void strncpy_fn(char *x) {
- strncpy(x, (char*)&strcpy_fn, 5); // expected-warning{{Argument to string
copy function is the address of the function 'strcpy_fn', which is not a
null-terminated string}}
+void strncpy_fn_src(char *x) {
+ strncpy(x, (char*)&strncpy_fn_src, 5); // expected-warning{{Argument to
string copy function is the address of the function 'strncpy_fn_src', which is
not a null-terminated string}}
+}
+
+void strncpy_fn_dst(const char *x) {
+ strncpy((char*)&strncpy_fn_dst, x, 5); // expected-warning{{Argument to
string copy function is the address of the function 'strncpy_fn_dst', which is
not a null-terminated string}}
}
void strncpy_effects(char *x, char *y) {
@@ -680
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 4ddea298e60c31d0995c06189a592895d2ad512b
be0bee7948a8a6394ba93df619d76048847c1ca8 --extensions c,cpp --
clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
``
View the diff from clang-format here.
``diff
diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
index 5a48b103b9..9c000df132 100644
--- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
@@ -170,9 +170,10 @@ void EnumInitialValueCheck::check(const
MatchFinder::MatchResult &Result) {
if (const auto *Enum = Result.Nodes.getNodeAs("inconsistent")) {
DiagnosticBuilder Diag =
-diag(Enum->getBeginLoc(),
- "initial values in enum '%0' are not consistent, consider
explicit "
- "initialization of all, none or only the first enumerator")
+diag(
+Enum->getBeginLoc(),
+"initial values in enum '%0' are not consistent, consider explicit
"
+"initialization of all, none or only the first enumerator")
<< getName(Enum);
for (const EnumConstantDecl *ECD : Enum->enumerators())
if (ECD->getInitExpr() == nullptr) {
``
https://github.com/llvm/llvm-project/pull/112496
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1183,7 +1181,7 @@ class Sema final : public SemaBase {
std::optional> CachedDarwinSDKInfo;
bool WarnedDarwinSDKInfoMissing = false;
- bool WarnedStackExhausted = false;
+ SingleWarningStackAwareExecutor StackAwareExecutor;
ilya-biryukov wrote:
Should we share the state between Sema and CodeGen?
It's not unusual for deeply nested ASTs to cause the limits to be hit in both
and it would make sense to only diagnose once globally.
I am not sure which class would be a good fit to hold that state as CodeGen
does not have a reference to Sema and vice versa, so maybe it needs too much
plumbing and isn't worth it. But I wanted to throw this idea out and hear what
people think.
https://github.com/llvm/llvm-project/pull/112371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
4JustMe4 wrote:
The suggestions have been applied. The document re-generation script has been
launched.
https://github.com/llvm/llvm-project/pull/112190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -5335,6 +5335,217 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D,
const ParsedAttr &AL) {
D->addAttr(::new (S.Context) PreferredTypeAttr(S.Context, AL, ParmTSI));
}
+// This function is called only if function call is not inside template body.
+// TODO: Add call for function calls inside template body.
+// Emit warnings if parent function misses format attributes.
+void Sema::DiagnoseMissingFormatAttributes(Stmt *Body,
+ const FunctionDecl *FDecl) {
+ assert(FDecl);
+
+ // If there are no function body, exit.
+ if (!Body)
+return;
+
+ // Get missing format attributes
+ std::vector MissingFormatAttributes =
+ GetMissingFormatAttributes(Body, FDecl);
+ if (MissingFormatAttributes.empty())
+return;
+
+ // Check if there are more than one format type found. In that case do not
+ // emit diagnostic.
+ const clang::IdentifierInfo *AttrType =
MissingFormatAttributes[0]->getType();
+ if (llvm::any_of(MissingFormatAttributes, [&](const FormatAttr *Attr) {
+return AttrType != Attr->getType();
+ }))
+return;
+
+ for (const FormatAttr *FA : MissingFormatAttributes) {
+// If format index and first-to-check argument index are negative, it means
+// that this attribute is only saved for multiple format types checking.
+if (FA->getFormatIdx() < 0 || FA->getFirstArg() < 0)
+ continue;
+
+// Emit diagnostic
+SourceLocation Loc = FDecl->getLocation();
+Diag(Loc, diag::warn_missing_format_attribute)
+<< FA->getType() << FDecl
+<< FixItHint::CreateInsertion(Loc,
+ (llvm::Twine("__attribute__((format(") +
+ FA->getType()->getName() + ", " +
+ llvm::Twine(FA->getFormatIdx()) + ", " +
+ llvm::Twine(FA->getFirstArg()) + ")))")
+ .str());
+ }
+}
+
+// Returns vector of format attributes. There are no two attributes with same
+// arguments in returning vector. There can be attributes that effectivelly
only
+// store information about format type.
+std::vector
+Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
+ unsigned int FunctionFormatArgumentIndexOffset =
+ checkIfMethodHasImplicitObjectParameter(FDecl) ? 2 : 1;
+
+ std::vector MissingAttributes;
+
+ // Iterate over body statements.
+ for (auto *Child : Body->children()) {
+// If child statement is compound statement, recursively get missing
+// attributes.
+if (dyn_cast_or_null(Child)) {
+ std::vector CompoundStmtMissingAttributes =
+ GetMissingFormatAttributes(Child, FDecl);
+
+ // If there are already missing attributes with same arguments, do not
add
+ // duplicates.
+ for (FormatAttr *FA : CompoundStmtMissingAttributes) {
+if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
+ return FA->getType() == Attr->getType() &&
+ FA->getFormatIdx() == Attr->getFormatIdx() &&
+ FA->getFirstArg() == Attr->getFirstArg();
+}))
+ MissingAttributes.push_back(FA);
+ }
+
+ continue;
+}
+
+ValueStmt *VS = dyn_cast_or_null(Child);
+if (!VS)
+ continue;
+Expr *TheExpr = VS->getExprStmt();
+if (!TheExpr)
+ continue;
+CallExpr *TheCall = dyn_cast_or_null(TheExpr);
+if (!TheCall)
+ continue;
+const FunctionDecl *ChildFunction =
+dyn_cast_or_null(TheCall->getCalleeDecl());
+if (!ChildFunction || !ChildFunction->hasAttr())
+ continue;
+
+Expr **Args = TheCall->getArgs();
+unsigned int NumArgs = TheCall->getNumArgs();
+
+// If child expression is function, check if it is format function.
+// If it is, check if parent function misses format attributes.
+
+unsigned int ChildFunctionFormatArgumentIndexOffset =
+checkIfMethodHasImplicitObjectParameter(ChildFunction) ? 2 : 1;
+
+// If child function is format function and format arguments are not
+// relevant to emit diagnostic, save only information about format type
+// (format index and first-to-check argument index are set to -1).
+// Information about format type is later used to determine if there are
+// more than one format type found.
+
+// Check if function has format attribute with forwarded format string.
+IdentifierInfo *AttrType;
+const ParmVarDecl *FormatArg;
+if (!llvm::any_of(ChildFunction->specific_attrs(),
+ [&](const FormatAttr *Attr) {
+AttrType = Attr->getType();
+
+int OffsetFormatIndex =
+Attr->getFormatIdx() -
+ChildFunctionFormatArgumentIndexOffset;
+if (OffsetFormatIndex < 0 ||
+(unsigned)OffsetFor
@@ -1183,7 +1181,7 @@ class Sema final : public SemaBase {
std::optional> CachedDarwinSDKInfo;
bool WarnedDarwinSDKInfoMissing = false;
- bool WarnedStackExhausted = false;
+ SingleWarningStackAwareExecutor StackAwareExecutor;
bricknerb wrote:
Yes, I considered that and it seems somewhat to add some complexity.
Assuming the same code might hit this in both Sema and CodeGen, I don't see a
reason the user should see this warning twice.
If Sema and CodeGen would typically be hitting different use cases, it might
make sense, but I don't think that's the case.
It's anyways beyond the scope of this change, and we could consider it as a
future improvement, though I doubt it should be high priority.
https://github.com/llvm/llvm-project/pull/112371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -16,6 +16,8 @@
#include
+#include "clang/Basic/Diagnostic.h"
ilya-biryukov wrote:
This dependency seems unnecessary in this file, the other functions are
lower-level (don't depend on SourceLocations, etc).
There are a few files that depend on it for things like `noteBottomOfStack()`.
In the interest of keeping the dependencies minimal, I'd suggest moving this to
a new file that depends on `Stack.h` and source locations + diagnostics engine.
https://github.com/llvm/llvm-project/pull/112371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NagyDonat wrote:
@steakhal May I merge this commit?
https://github.com/llvm/llvm-project/pull/112209
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1183,7 +1181,7 @@ class Sema final : public SemaBase {
std::optional> CachedDarwinSDKInfo;
bool WarnedDarwinSDKInfoMissing = false;
- bool WarnedStackExhausted = false;
+ SingleWarningStackAwareExecutor StackAwareExecutor;
ilya-biryukov wrote:
Makes sense to keep this as an action for the future. Sharing the logic makes
sense, though.
Should we file an issue on GitHub and/or add a FIXME to the code that we might
want to share a single instance Sema and CodeGen in the future?
https://github.com/llvm/llvm-project/pull/112371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ilya-biryukov wrote:
Adding @ChuanqiXu9 to get another pair of eyes on it (for naming, code
structure, etc). I believe he was involved in reviewing some changes touching
`runWithSufficientStackSpace` I've previously sent.
Please give him a chance to review this.
https://github.com/llvm/llvm-project/pull/112371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 commented:
Took a pretty quick scanning and this looks good to me.
https://github.com/llvm/llvm-project/pull/112371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1183,7 +1181,7 @@ class Sema final : public SemaBase {
std::optional> CachedDarwinSDKInfo;
bool WarnedDarwinSDKInfoMissing = false;
- bool WarnedStackExhausted = false;
+ SingleWarningStackAwareExecutor StackAwareExecutor;
ChuanqiXu9 wrote:
Abstractly, we can generate codes without constructing a Sema by loading the
AST file by ASTUnit and generate it. So I'd like to make them seperate.
https://github.com/llvm/llvm-project/pull/112371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -5335,6 +5335,217 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D,
const ParsedAttr &AL) {
D->addAttr(::new (S.Context) PreferredTypeAttr(S.Context, AL, ParmTSI));
}
+// This function is called only if function call is not inside template body.
+// TODO: Add call for function calls inside template body.
+// Emit warnings if parent function misses format attributes.
+void Sema::DiagnoseMissingFormatAttributes(Stmt *Body,
+ const FunctionDecl *FDecl) {
+ assert(FDecl);
+
+ // If there are no function body, exit.
+ if (!Body)
+return;
+
+ // Get missing format attributes
+ std::vector MissingFormatAttributes =
+ GetMissingFormatAttributes(Body, FDecl);
+ if (MissingFormatAttributes.empty())
+return;
+
+ // Check if there are more than one format type found. In that case do not
+ // emit diagnostic.
+ const clang::IdentifierInfo *AttrType =
MissingFormatAttributes[0]->getType();
+ if (llvm::any_of(MissingFormatAttributes, [&](const FormatAttr *Attr) {
+return AttrType != Attr->getType();
+ }))
+return;
+
+ for (const FormatAttr *FA : MissingFormatAttributes) {
+// If format index and first-to-check argument index are negative, it means
+// that this attribute is only saved for multiple format types checking.
+if (FA->getFormatIdx() < 0 || FA->getFirstArg() < 0)
+ continue;
+
+// Emit diagnostic
+SourceLocation Loc = FDecl->getLocation();
+Diag(Loc, diag::warn_missing_format_attribute)
+<< FA->getType() << FDecl
+<< FixItHint::CreateInsertion(Loc,
+ (llvm::Twine("__attribute__((format(") +
+ FA->getType()->getName() + ", " +
+ llvm::Twine(FA->getFormatIdx()) + ", " +
+ llvm::Twine(FA->getFirstArg()) + ")))")
+ .str());
+ }
+}
+
+// Returns vector of format attributes. There are no two attributes with same
+// arguments in returning vector. There can be attributes that effectivelly
only
+// store information about format type.
+std::vector
+Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
+ unsigned int FunctionFormatArgumentIndexOffset =
+ checkIfMethodHasImplicitObjectParameter(FDecl) ? 2 : 1;
+
+ std::vector MissingAttributes;
+
+ // Iterate over body statements.
+ for (auto *Child : Body->children()) {
+// If child statement is compound statement, recursively get missing
+// attributes.
+if (dyn_cast_or_null(Child)) {
budimirarandjelovichtec wrote:
Yes, I meant braces instead of branches. This snippet does not produce warning,
so I'm working to fix it.
https://github.com/llvm/llvm-project/pull/105479
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/lenary created
https://github.com/llvm/llvm-project/pull/112561
This change implements support for the `cr` and `cf` register constraints
(which allocate a RVC GPR or RVC FPR respectively), and the `N` modifier (which
prints the raw encoding of a register rather than the name).
The intention behind these additions is to make it easier to use inline
assembly when assembling raw instructions that are not supported by the
compiler, for instance when experimenting with new instructions or when
supporting proprietary extensions outside the toolchain.
These implement part of my proposal in riscv-non-isa/riscv-c-api-doc#92
As part of the implementation, I felt there was not enough coverage of inline
assembly and the "in X" floating-point extensions, so I have added more
regression tests around these configurations.
>From 6d60f09f20d9e8436af95f601256dd9301912028 Mon Sep 17 00:00:00 2001
From: Sam Elliott
Date: Wed, 16 Oct 2024 05:04:45 -0700
Subject: [PATCH] [RISCV] Inline Assembly: RVC constraint and N modifier
This change implements support for the `cr` and `cf` register
constraints (which allocate a RVC GPR or RVC FPR respectively), and the
`N` modifier (which prints the raw encoding of a register rather than the
name).
The intention behind these additions is to make it easier to use inline
assembly when assembling raw instructions that are not supported by the
compiler, for instance when experimenting with new instructions or when
supporting proprietary extensions outside the toolchain.
These implement part of my proposal in riscv-non-isa/riscv-c-api-doc#92
As part of the implementation, I felt there was not enough coverage of
inline assembly and the "in X" floating-point extensions, so I have
added more regression tests around these configurations.
---
clang/lib/Basic/Targets/RISCV.cpp | 10 ++
clang/test/CodeGen/RISCV/riscv-inline-asm.c | 40 -
llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp | 9 +
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | 18 ++
llvm/lib/Target/RISCV/RISCVRegisterInfo.td| 19 ++-
.../RISCV/inline-asm-d-constraint-f.ll| 33
.../CodeGen/RISCV/inline-asm-d-modifier-N.ll | 109
.../RISCV/inline-asm-f-constraint-f.ll| 28 +++-
.../CodeGen/RISCV/inline-asm-f-modifier-N.ll | 96 +++
llvm/test/CodeGen/RISCV/inline-asm-invalid.ll | 20 +++
.../RISCV/inline-asm-zdinx-constraint-r.ll| 92 ++
.../RISCV/inline-asm-zfh-constraint-f.ll | 41 +
.../RISCV/inline-asm-zfh-modifier-N.ll| 157 +
.../RISCV/inline-asm-zfinx-constraint-r.ll| 89 ++
.../RISCV/inline-asm-zhinx-constraint-r.ll| 158 ++
llvm/test/CodeGen/RISCV/inline-asm.ll | 66
.../CodeGen/RISCV/zdinx-asm-constraint.ll | 61 +++
17 files changed, 1041 insertions(+), 5 deletions(-)
create mode 100644 llvm/test/CodeGen/RISCV/inline-asm-d-modifier-N.ll
create mode 100644 llvm/test/CodeGen/RISCV/inline-asm-f-modifier-N.ll
create mode 100644 llvm/test/CodeGen/RISCV/inline-asm-zdinx-constraint-r.ll
create mode 100644 llvm/test/CodeGen/RISCV/inline-asm-zfh-modifier-N.ll
create mode 100644 llvm/test/CodeGen/RISCV/inline-asm-zfinx-constraint-r.ll
create mode 100644 llvm/test/CodeGen/RISCV/inline-asm-zhinx-constraint-r.ll
diff --git a/clang/lib/Basic/Targets/RISCV.cpp
b/clang/lib/Basic/Targets/RISCV.cpp
index 870f0f38bc3057..eaaba7642bd7b2 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -100,6 +100,14 @@ bool RISCVTargetInfo::validateAsmConstraint(
case 'S': // A symbol or label reference with a constant offset
Info.setAllowsRegister();
return true;
+ case 'c':
+// A RVC register - GPR or FPR
+if (Name[1] == 'r' || Name[1] == 'f') {
+ Info.setAllowsRegister();
+ Name += 1;
+ return true;
+}
+return false;
case 'v':
// A vector register.
if (Name[1] == 'r' || Name[1] == 'd' || Name[1] == 'm') {
@@ -114,6 +122,8 @@ bool RISCVTargetInfo::validateAsmConstraint(
std::string RISCVTargetInfo::convertConstraint(const char *&Constraint) const {
std::string R;
switch (*Constraint) {
+ // c* and v* are two-letter constraints on RISC-V.
+ case 'c':
case 'v':
R = std::string("^") + std::string(Constraint, 2);
Constraint += 1;
diff --git a/clang/test/CodeGen/RISCV/riscv-inline-asm.c
b/clang/test/CodeGen/RISCV/riscv-inline-asm.c
index fa0bf6aa6aa471..75b91d3c497c50 100644
--- a/clang/test/CodeGen/RISCV/riscv-inline-asm.c
+++ b/clang/test/CodeGen/RISCV/riscv-inline-asm.c
@@ -3,7 +3,35 @@
// RUN: %clang_cc1 -triple riscv64 -O2 -emit-llvm %s -o - \
// RUN: | FileCheck %s
-// Test RISC-V specific inline assembly constraints.
+// Test RISC-V specific inline assembly constraints and modifiers.
+
+long test_r(long x) {
+// CHECK-LABEL: define{{.*}} {{i64|i32}} @test_r(
+// CHECK: call {{i64|i32}} asm sideeffect "", "=r,r"(
