@@ -271,26 +271,32 @@ class CFGWalker {
// translateAttrExpr needs it, but that should be moved too.
class CapabilityExpr {
private:
- /// The capability expression and whether it's negated.
- llvm::PointerIntPair CapExpr;
+ /// The capability expression and flags.
+ llvm::
aaronpuchert wrote:
@melver, this request came from @AaronBallman. But since you're also working on
Thread Safety Analysis in C, you might have some thoughts of your own about
this.
I haven't checked any real-world code yet. (Specifically, how many functions
would be affected by this exclusio
https://github.com/aaronpuchert created
https://github.com/llvm/llvm-project/pull/141432
The analysis already excludes functions with a zero-argument acquire or release
attribute. According to the requirements enforced by
-Wthread-safety-attributes, these are methods of a capability class wher
https://github.com/aaronpuchert edited
https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -235,6 +266,20 @@ class FactSet {
return false;
}
+ std::optional replaceLock(FactManager &FM, iterator It,
+std::unique_ptr Entry) {
+if (It == end())
+ return std::nullopt;
+FactID F = FM.newFact(std::move(Entry));
+
https://github.com/aaronpuchert approved this pull request.
I think this looks good, thanks for your contribution!
https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/m
https://github.com/aaronpuchert closed
https://github.com/llvm/llvm-project/pull/135390
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1011,6 +979,30 @@ void SExprBuilder::exitCFG(const CFGBlock *Last) {
IncompleteArgs.clear();
}
+static CapabilityExpr makeCapabilityExpr(const til::SExpr *E, QualType VDT,
+ bool Neg) {
+ // We need to look at the declaration of t
@@ -0,0 +1,34 @@
+//===--- UseEnumClassCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -0,0 +1,34 @@
+//===--- UseEnumClassCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
aaronpuchert wrote:
Could also add this under "Bug Fixes to C++ Support". The entries about type
traits [don't seem to be consistently
categorized](https://releases.llvm.org/19.1.0/tools/clang/docs/ReleaseNotes.html#bug-fixes-in-this-version),
but I think this is all C++-only.
https://github.
https://github.com/aaronpuchert updated
https://github.com/llvm/llvm-project/pull/135390
>From d8bc5ebd7976d25e800987b3c95057364dc1c07c Mon Sep 17 00:00:00 2001
From: Aaron Puchert
Date: Fri, 11 Apr 2025 13:47:02 +0200
Subject: [PATCH] Suppress errors from well-formed-testing type traits in
SF
@@ -12240,16 +12240,16 @@ class Sema final : public SemaBase {
bool PrevLastDiagnosticIgnored;
public:
-explicit SFINAETrap(Sema &SemaRef, bool AccessCheckingSFINAE = false)
+explicit SFINAETrap(Sema &SemaRef, bool TestWellformedSFINAE = false)
a
@@ -12240,16 +12240,16 @@ class Sema final : public SemaBase {
bool PrevLastDiagnosticIgnored;
public:
-explicit SFINAETrap(Sema &SemaRef, bool AccessCheckingSFINAE = false)
+explicit SFINAETrap(Sema &SemaRef, bool TestWellformedSFINAE = false)
a
@@ -0,0 +1,34 @@
+//===--- UseEnumClassCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -0,0 +1,34 @@
+//===--- UseEnumClassCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -0,0 +1,34 @@
+//===--- UseEnumClassCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
https://github.com/aaronpuchert closed
https://github.com/llvm/llvm-project/pull/137477
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/aaronpuchert created
https://github.com/llvm/llvm-project/pull/137477
Support for attribute parameter packs was added some time ago in commit
ead1690d31f815c00fdd2bc23db4766191bbeabc. But template substitution didn't
expand the packs yet. For now expansion can only happen wi
@@ -114,31 +112,39 @@ class FactEntry : public CapabilityExpr {
};
private:
- const FactEntryKind Kind : 8;
+ const FactEntryKind Kind : 4;
/// Exclusive or shared.
- LockKind LKind : 8;
+ const LockKind LKind : 4;
+
+ /// How it was acquired.
+ const SourceKind S
@@ -168,6 +197,8 @@ class FactManager {
public:
FactID newFact(std::unique_ptr Entry) {
Facts.push_back(std::move(Entry));
+assert(Facts.size() - 1 <= std::numeric_limits::max() &&
aaronpuchert wrote:
```suggestion
assert(Facts.size() - 1 <= std:
@@ -235,6 +266,20 @@ class FactSet {
return false;
}
+ std::optional replaceLock(FactManager &FM, iterator It,
+std::unique_ptr Entry) {
+if (It == end())
+ return std::nullopt;
+FactID F = FM.newFact(std::move(Entry));
+
https://github.com/aaronpuchert commented:
Two more things come to mind:
* Consider adding a check to `SemaDeclAttr.cpp` that the new attribute is
always accompanied by `capability` or `lockable`. Although I wonder whether
that's too early. I'm not sure if we can see the other attributes alread
@@ -114,31 +112,39 @@ class FactEntry : public CapabilityExpr {
};
private:
- const FactEntryKind Kind : 8;
+ const FactEntryKind Kind : 4;
/// Exclusive or shared.
- LockKind LKind : 8;
+ const LockKind LKind : 4;
+
+ /// How it was acquired.
+ const SourceKind S
@@ -114,31 +112,39 @@ class FactEntry : public CapabilityExpr {
};
private:
- const FactEntryKind Kind : 8;
+ const FactEntryKind Kind : 4;
/// Exclusive or shared.
- LockKind LKind : 8;
+ const LockKind LKind : 4;
+
+ /// How it was acquired.
+ const SourceKind S
@@ -388,7 +395,7 @@ class SExprBuilder {
til::LiteralPtr *createVariable(const VarDecl *VD);
// Create placeholder for this: we don't know the VarDecl on construction
yet.
- std::pair
+ std::pair
aaronpuchert wrote:
Nice idea, but I'm not sure if it's
@@ -271,26 +271,32 @@ class CFGWalker {
// translateAttrExpr needs it, but that should be moved too.
class CapabilityExpr {
private:
- /// The capability expression and whether it's negated.
- llvm::PointerIntPair CapExpr;
+ /// The capability expression and flags.
+ llvm::
@@ -163,15 +184,15 @@ using FactID = unsigned short;
/// the analysis of a single routine.
class FactManager {
private:
- std::vector> Facts;
+ std::vector> Facts;
aaronpuchert wrote:
The FactEntries themselves should never be replaced, they are immutable. W
@@ -81,26 +81,25 @@ static bool isCalleeArrow(const Expr *E) {
return ME ? ME->isArrow() : false;
}
-static StringRef ClassifyDiagnostic(const CapabilityAttr *A) {
- return A->getName();
-}
-
-static StringRef ClassifyDiagnostic(QualType VDT) {
+static CapabilityExpr makeCa
https://github.com/aaronpuchert edited
https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/aaronpuchert edited
https://github.com/llvm/llvm-project/pull/137133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -271,26 +271,32 @@ class CFGWalker {
// translateAttrExpr needs it, but that should be moved too.
class CapabilityExpr {
private:
- /// The capability expression and whether it's negated.
- llvm::PointerIntPair CapExpr;
+ /// The capability expression and flags.
+ llvm::
@@ -103,6 +103,23 @@ static StringRef ClassifyDiagnostic(QualType VDT) {
return "mutex";
}
+static unsigned getCapabilityExprFlags(QualType VDT) {
+ unsigned Flags = 0;
+
+ if (const auto *RT = VDT->getAs()) {
+if (const auto *RD = RT->getDecl())
+ if (RD->hasAttr
@@ -271,26 +271,32 @@ class CFGWalker {
// translateAttrExpr needs it, but that should be moved too.
class CapabilityExpr {
private:
- /// The capability expression and whether it's negated.
- llvm::PointerIntPair CapExpr;
+ /// The capability expression and flags.
+ llvm::
@@ -6708,15 +6708,15 @@ int testAdoptShared() {
} // namespace ReturnScopedLockable
-#endif
+#endif // __cpp_guaranteed_copy_elision
aaronpuchert wrote:
These changes are fine, but please just commit them separately. (No review
required.)
https://github.co
@@ -163,15 +184,15 @@ using FactID = unsigned short;
/// the analysis of a single routine.
class FactManager {
private:
- std::vector> Facts;
+ std::vector> Facts;
aaronpuchert wrote:
This does not work, `FactEntry` has to remain `const`. See Delesley's comm
@@ -434,6 +434,16 @@ class can be used as a capability. The string argument
specifies the kind of
capability in error messages, e.g. ``"mutex"``. See the ``Container`` example
given above, or the ``Mutex`` class in :ref:`mutexheader`.
+REENTRANT
aaronpucher
@@ -434,6 +434,16 @@ class can be used as a capability. The string argument
specifies the kind of
capability in error messages, e.g. ``"mutex"``. See the ``Container`` example
given above, or the ``Mutex`` class in :ref:`mutexheader`.
+REENTRANT
+-
+
+``REENTRANT``
@@ -1831,15 +1852,15 @@ void BuildLockset::handleCall(const Expr *Exp, const
NamedDecl *D,
assert(!Self);
const auto *TagT = Exp->getType()->getAs();
if (D->hasAttrs() && TagT && Exp->isPRValue()) {
- std::pair Placeholder =
- Analyzer->SxBuilder.crea
https://github.com/aaronpuchert commented:
I think the biggest issue is that removing `const` from `FactEntry` does not
work. You'll have to undo all those changes and instead create a new
`FactEntry` for every lock/unlock.
https://github.com/llvm/llvm-project/pull/137133
_
https://github.com/aaronpuchert closed
https://github.com/llvm/llvm-project/pull/135561
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aaronpuchert wrote:
I think it's obscure enough to not need to be mentioned. Since I went with the
less intrusive variant of downgrading an error to warning, it should not break
anyone's code.
I'd be open to add `ErrorDiag` on all thread safety attributes, but I'd do so
in a separate change i
aaronpuchert wrote:
This seems to have fixed #60112. Thanks!
https://github.com/llvm/llvm-project/pull/89154
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/aaronpuchert updated
https://github.com/llvm/llvm-project/pull/135561
>From 372bfceceec7ba618d7651559f1071baacaf2fcc Mon Sep 17 00:00:00 2001
From: Aaron Puchert
Date: Sun, 13 Apr 2025 22:28:23 +0200
Subject: [PATCH] Merge similar Clang Thread Safety attributes
Some of the o
https://github.com/aaronpuchert created
https://github.com/llvm/llvm-project/pull/135561
Some of the old lock-based and new capability-based spellings behave basically
in the same way, so merging them simplifies the code significantly.
There are two minor functional changes: we only warn (inst
https://github.com/aaronpuchert updated
https://github.com/llvm/llvm-project/pull/135390
>From af21e7bb441c13714f299600966bff28befe5191 Mon Sep 17 00:00:00 2001
From: Aaron Puchert
Date: Fri, 11 Apr 2025 13:47:02 +0200
Subject: [PATCH] Suppress errors from well-formed-testing type traits in
SF
https://github.com/aaronpuchert updated
https://github.com/llvm/llvm-project/pull/135390
>From ef1b40c0247205f8147fe6050c1303628833c247 Mon Sep 17 00:00:00 2001
From: Aaron Puchert
Date: Fri, 11 Apr 2025 13:47:02 +0200
Subject: [PATCH] Suppress errors from well-formed-testing type traits in
SF
https://github.com/aaronpuchert updated
https://github.com/llvm/llvm-project/pull/135390
>From 20219354f6a5a59cb36554fb26c5864b5d9be74e Mon Sep 17 00:00:00 2001
From: Aaron Puchert
Date: Fri, 11 Apr 2025 13:47:02 +0200
Subject: [PATCH] Suppress errors from well-formed-testing type traits in
SF
aaronpuchert wrote:
> (It should be noted that the standard doesn't always base this on the
> immediate context being well-formed: for `std::common_type` it's based on
> whether some expression "denotes a valid type." But I assume that's an
> editorial issue and means the same thing.)
Filed c
https://github.com/aaronpuchert edited
https://github.com/llvm/llvm-project/pull/135390
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/aaronpuchert updated
https://github.com/llvm/llvm-project/pull/135390
>From a1eda3b0d5b54ffc3d3ad4288d1d7685f6486143 Mon Sep 17 00:00:00 2001
From: Aaron Puchert
Date: Fri, 11 Apr 2025 13:47:02 +0200
Subject: [PATCH] Suppress errors from well-formed-testing type traits in
SF
aaronpuchert wrote:
The tests would produce without the change:
```
error: 'expected-error' diagnostics seen but not expected:
File clang/test/SemaCXX/type-traits.cpp Line 2676: calling a private
constructor of class 'AllPrivate'
File clang/test/SemaCXX/type-traits.cpp Line 2833: 'operator='
aaronpuchert wrote:
Maybe the test needs to be relaxed a bit because of stack layout differences in
other OS targets? Although I'm not sure why they're different. See
https://lab.llvm.org/buildbot/#/builders/186/builds/7896:
```
TEST 'AddressSanitizer-arm-android ::
TestCa
@@ -2041,15 +2042,16 @@ void BuildLockset::handleCall(const Expr *Exp, const
NamedDecl *D,
if (!a.has_value()) {
Analyzer->Handler.handleExpectFewerUnderlyingMutexes(
Exp->getExprLoc(), D->getLocation(), Scope->toString(),
- b.value
@@ -3566,6 +3565,38 @@ void releaseMemberCall() {
ReleasableMutexLock lock(&obj.mu);
releaseMember(obj, lock);
}
+#ifdef __cpp_guaranteed_copy_elision
+// expected-note@+2{{mutex acquired here}}
+// expected-note@+1{{see attribute on function here}}
+RelockableScope returnU
https://github.com/aaronpuchert edited
https://github.com/llvm/llvm-project/pull/131831
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -3566,6 +3565,38 @@ void releaseMemberCall() {
ReleasableMutexLock lock(&obj.mu);
releaseMember(obj, lock);
}
+#ifdef __cpp_guaranteed_copy_elision
+// expected-note@+2{{mutex acquired here}}
+// expected-note@+1{{see attribute on function here}}
+RelockableScope returnU
@@ -2041,15 +2042,16 @@ void BuildLockset::handleCall(const Expr *Exp, const
NamedDecl *D,
if (!a.has_value()) {
Analyzer->Handler.handleExpectFewerUnderlyingMutexes(
Exp->getExprLoc(), D->getLocation(), Scope->toString(),
- b.value
@@ -3566,6 +3565,38 @@ void releaseMemberCall() {
ReleasableMutexLock lock(&obj.mu);
releaseMember(obj, lock);
}
+#ifdef __cpp_guaranteed_copy_elision
aaronpuchert wrote:
Add blank lines around `#ifdef` and `#endif` for readability.
https://github.com/llv
@@ -2316,6 +2337,49 @@ void BuildLockset::VisitReturnStmt(const ReturnStmt *S) {
ReturnType->getPointeeType().isConstQualified() ? AK_Read : AK_Written,
POK_ReturnPointer);
}
+
+ if (!checkRecordTypeForScopedCapability(ReturnType))
+return;
+
+ if (const
@@ -1799,11 +1799,11 @@ class ThreadSafetyReporter : public
clang::threadSafety::ThreadSafetyHandler {
: getNotes();
}
- OptionalNotes makeManagedMismatchNoteForParam(SourceLocation DeclLoc) {
+ OptionalNotes makeManagedMismatchNote(SourceLocation DeclLoc,
@@ -2316,6 +2337,49 @@ void BuildLockset::VisitReturnStmt(const ReturnStmt *S) {
ReturnType->getPointeeType().isConstQualified() ? AK_Read : AK_Written,
POK_ReturnPointer);
}
+
+ if (!checkRecordTypeForScopedCapability(ReturnType))
+return;
+
+ if (const
https://github.com/aaronpuchert commented:
Looks good to me, just some small nitpicks.
https://github.com/llvm/llvm-project/pull/131831
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aaronpuchert wrote:
Ping. Or is this not interesting enough for a review?
https://github.com/llvm/llvm-project/pull/116266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/aaronpuchert approved this pull request.
Looks good to me, and thanks for the contribution!
> Note, I think for now it might be safer to not enable by default yet, but
> I've made a note (and hinted at in changelog) that we're planning to default
> enable in future, so that
aaronpuchert wrote:
> One thought --- you could consider an attribute that could be put on pointer
> arguments to functions that says "yes, I dereference this and read or write
> it".
GCC [has such an
attribute](https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-access-f
aaronpuchert wrote:
Good question. Which AST nodes could we visit here? Let's see some examples of
initializing function pointers/references:
```c++
void f();
void (*fp)() = f;
void (*fp2)() = &f;
void (*fp3)() = fp;
void (&fr)() = f;
```
The (simplified) AST:
```
TranslationUnitDecl 0x55edc6
https://github.com/aaronpuchert commented:
I think this looks very good! I just have some minor remarks.
Thanks to @aoates for trying this out, this is always appreciated!
And sorry for the delay.
https://github.com/llvm/llvm-project/pull/127396
___
@@ -515,7 +515,8 @@ Warning flags
+ ``-Wthread-safety-analysis``: The core analysis.
+ ``-Wthread-safety-precise``: Requires that mutex expressions match
precisely.
This warning can be disabled for code which has a lot of aliases.
- + ``-Wthread-safety-reference``:
@@ -1780,6 +1782,14 @@ void ThreadSafetyAnalyzer::checkPtAccess(const FactSet
&FSet, const Expr *Exp,
Exp = CE->getSubExpr();
continue;
}
+if (const auto *UO = dyn_cast(Exp)) {
+ if (UO->getOpcode() == UO_AddrOf) {
+// Pointer access via pointe
@@ -1795,9 +1795,22 @@ void ThreadSafetyAnalyzer::checkPtAccess(const FactSet
&FSet, const Expr *Exp,
// Pass by reference warnings are under a different flag.
aaronpuchert wrote:
"reference/pointer"
https://github.com/llvm/llvm-project/pull/127396
___
@@ -528,6 +529,9 @@ for a period of time, after which they are migrated into
the standard analysis.
* ``-Wthread-safety-beta``: New features. Off by default.
+ + ``-Wthread-safety-pointer``: Checks when passing or returning pointers to
+guarded variables, or pointers
@@ -2294,6 +2309,11 @@ void BuildLockset::VisitReturnStmt(const ReturnStmt *S) {
FunctionExitFSet, RetVal,
ReturnType->getPointeeType().isConstQualified() ? AK_Read : AK_Written,
POK_ReturnByRef);
+ } else if (ReturnType->isPointerType()) {
@@ -4944,6 +4949,14 @@ class Foo {
(*datap2_)[0] = 0;// expected-warning {{reading the value pointed to
by 'datap2_' requires holding mutex 'mu_'}}
data_(); // expected-warning {{reading variable 'data_'
requires holding mutex 'mu_'}}
+
+// Calls
@@ -4955,13 +4968,18 @@ class Foo {
//showDataCell(*datap2_); // xpected-warning {{reading the value pointed
to by 'datap2_' requires holding mutex 'mu_'}}
int a = data_[0]; // expected-warning {{reading variable 'data_'
requires holding mutex 'mu_'}}
+
+(v
@@ -142,9 +145,19 @@ int main(void) {
(void)(get_value(b_) == 1);
mutex_unlock(foo_.mu_);
+ a_ = 0; // expected-warning{{writing variable 'a_' requires holding mutex
'foo_.mu_'}}
+ __WRITE_ONCE(a_, 0); // expected-warning{{writing variable 'a_' requires
holding mutex '
https://github.com/aaronpuchert edited
https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aaronpuchert wrote:
Something like `READ_ONCE` might be supported differently: suppose there is
actually a read, i.e. an lvalue-to-rvalue cast. We check those here:
```c++
void BuildLockset::VisitCastExpr(const CastExpr *CE) {
if (CE->getCastKind() != CK_LValueToRValue)
return;
checkAcce
aaronpuchert wrote:
> The equivalent of passing a `pt_guarded_by` variable by value doesn't seem to
> warn.
This inconsistency is probably my largest concern. If you have
```c
int x GUARDED_BY(mu);
int *p PT_GUARDED_BY(mu);
```
then `&x` should basically behave like `p`, and `x` like `*p`. But
@@ -133,7 +134,12 @@ int main(void) {
Foo_func3(5);
+#ifdef CHECK_ADDRESSOF
+ set_value(&a_, 0); // expected-warning{{calling function 'set_value'
requires holding mutex 'foo_.mu_' exclusively}} \
+expected-warning{{taking address of variable 'a_'
@@ -1983,11 +1983,21 @@ class ThreadSafetyReporter : public
clang::threadSafety::ThreadSafetyHandler {
void handleNoMutexHeld(const NamedDecl *D, ProtectedOperationKind POK,
AccessKind AK, SourceLocation Loc) override {
-assert((POK == POK_VarAc
@@ -515,8 +515,18 @@ Warning flags
+ ``-Wthread-safety-analysis``: The core analysis.
+ ``-Wthread-safety-precise``: Requires that mutex expressions match
precisely.
This warning can be disabled for code which has a lot of aliases.
- + ``-Wthread-safety-reference``
@@ -1,7 +1,9 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety
-Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions
-DUSE_CAPABILITY=0 %s
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety
-Wthread-safety-beta -Wno-thread-saf
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta
%s
// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta
-fexperimental-late-parse-attributes -DLATE_PARSING %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthr
https://github.com/aaronpuchert edited
https://github.com/llvm/llvm-project/pull/123063
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/aaronpuchert commented:
I've taken a brief look at the [Linux kernel
changes](https://github.com/google/kernel-sanitizers/compare/2014c95afecee3e76ca4a56956a936e23283f05b...cap-analysis).
I didn't check how many variables are affected by `__rcu_guarded`, but
otherwise there
aaronpuchert wrote:
We had a discussion on https://reviews.llvm.org/D52395 that might be relevant
here. To quote @delesley:
> When you pass an object to a function, either by pointer or by reference, no
> actual load from memory has yet occurred. Thus, there is a real risk of false
> positiv
https://github.com/aaronpuchert closed
https://github.com/llvm/llvm-project/pull/110523
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aaronpuchert wrote:
Submitted as c1e7e4500c6e3b921f5e0cda8ba8d8d66e086db6.
https://github.com/llvm/llvm-project/pull/110523
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: Malek Ben Slimane
Date: 2024-12-20T23:49:03+01:00
New Revision: c1e7e4500c6e3b921f5e0cda8ba8d8d66e086db6
URL:
https://github.com/llvm/llvm-project/commit/c1e7e4500c6e3b921f5e0cda8ba8d8d66e086db6
DIFF:
https://github.com/llvm/llvm-project/commit/c1e7e4500c6e3b921f5e0cda8ba8d8d66e086db6.d
aaronpuchert wrote:
With my change reverted, the test lives in `compiler-rt/test/sanitizer_common`,
where we get an additional `--sysroot=` argument. This comes from
`compiler-rt/test/sanitizer_common/lit.site.cfg.py.in`:
```python
# Tool-specific config options.
config.name_suffix = "@CONFIG_N
aaronpuchert wrote:
With my change reverted I get the failure as well:
```
FAIL: SanitizerCommon-msan-x86_64-Linux :: Linux/dn_expand.cpp (5088 of 5094)
TEST 'SanitizerCommon-msan-x86_64-Linux ::
Linux/dn_expand.cpp' FAILED
Exit Code: 1
Command Output (
aaronpuchert wrote:
That configuration output is from `compiler-rt`, but we need it in `llvm`. In
`llvm/lib/ExecutionEngine/Orc/CMakeLists.txt`:
```cmake
if( CMAKE_HOST_UNIX AND HAVE_LIBRT )
set(rt_lib rt)
endif()
```
We have `CMAKE_HOST_UNIX` = 1, but `HAVE_LIBRT` is empty. It comes from
`ch
aaronpuchert wrote:
Without these two options, the build fails for me:
```
FAILED: bin/llvm-jitlink
: && /usr/bin/clang++ --sysroot=/home/aaron/chrome-sysroot
-fvisibility-inlines-hidden -Werror=date-time
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter
-Wwrite-strings -W
aaronpuchert wrote:
Where do I find these?
```
-DRUNTIMES_i386-unknown-linux-gnu_CMAKE_SYSROOT=/usr/local/google/home/ayzhao/src/chromium/src/third_party/llvm-build-tools/debian_bullseye_i386_sysroot
\
-DBUILTINS_i386-unknown-linux-gnu_CMAKE_SYSROOT=/usr/local/google/home/ayzhao/src/chromium/src
aaronpuchert wrote:
Specifically, this is the output:
```
Uninitialized bytes in strcmp at offset 0 inside [0x7fffe1e0, 12)
==1131638==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5565c159 in testWrite()
/b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/test/ms
aaronpuchert wrote:
@nico, could you provide some more information? This didn't break on the
lab.llvm.org infrastructure, and I'm not sure what's different in your build.
Locally I also used a Release build with assertions on x86_64 on Linux.
For what it's worth, the change simply moves the in
https://github.com/aaronpuchert closed
https://github.com/llvm/llvm-project/pull/116487
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aaronpuchert wrote:
Check out https://lab.llvm.org/buildbot/#/builders/13/builds/4041:
```
RUN: at line 1:
/opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/stage1/bin/clang
-cc1 -internal-isystem
/opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/stage1/lib/clan
aaronpuchert wrote:
Check out https://lab.llvm.org/buildbot/#/builders/13/builds/4041:
```
RUN: at line 1:
/opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/stage1/bin/clang
-cc1 -internal-isystem
/opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/stage1/lib/clan
1 - 100 of 385 matches
Mail list logo
