https://github.com/haoNoQ approved this pull request.
https://github.com/llvm/llvm-project/pull/110213
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -190,11 +190,7 @@ class UncountedLocalVarsChecker
if (shouldSkipVarDecl(V))
return;
-const auto *ArgType = V->getType().getTypePtr();
-if (!ArgType)
haoNoQ wrote:
Fair enough!
https://github.com/llvm/llvm-project/pull/110213
https://github.com/haoNoQ approved this pull request.
Yes makes sense! I've no idea why this is a special case anyway.
https://github.com/llvm/llvm-project/pull/98
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-b
https://github.com/haoNoQ updated
https://github.com/llvm/llvm-project/pull/111624
>From b5c9082e36efcc7be2cabc73c985749f2fd41725 Mon Sep 17 00:00:00 2001
From: Artem Dergachev
Date: Tue, 8 Oct 2024 20:24:00 -0700
Subject: [PATCH 1/3] [-Wunsafe-buffer-usage] Add user documentation.
---
clang/
https://github.com/haoNoQ approved this pull request.
Aha LGTM!
https://github.com/llvm/llvm-project/pull/111222
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
haoNoQ wrote:
No worries! I've just seen folks independently rediscover some of your work so
I wanted them to see if they want to deduplicate the efforts.
https://github.com/llvm/llvm-project/pull/91991
___
cfe-commits mailing list
cfe-commits@lists.l
haoNoQ wrote:
> a function that returns a pointer and takes a reference (or a pointer) to a
> length variable
Yes, this one should be easy to catch. Both values will be `SymbolConjured`
pointing to the same function call expression / program point. (The
out-parameter value may also be `Symbol
@@ -0,0 +1,199 @@
+//== BoundsInformationChecker.cpp - bounds information checker --*- C++
-*--==//
+//
+// 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: Ap
@@ -0,0 +1,199 @@
+//== BoundsInformationChecker.cpp - bounds information checker --*- C++
-*--==//
+//
+// 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: Ap
@@ -45,32 +52,119 @@ class UncountedLambdaCapturesChecker
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
- bool VisitLambdaExpr(LambdaExpr *L) {
-Checker->visitLambdaExpr(L);
+
https://github.com/haoNoQ edited
https://github.com/llvm/llvm-project/pull/114897
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/haoNoQ commented:
The attribute gets some action! Nice!!
I noticed that currently the attribute's documentation doesn't say it can be
placed on lambdas. But it doesn't look like it's actively rejected either. So
it might be a good idea to update the documentation as part of
@@ -45,32 +52,119 @@ class UncountedLambdaCapturesChecker
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
- bool VisitLambdaExpr(LambdaExpr *L) {
-Checker->visitLambdaExpr(L);
+
@@ -0,0 +1,86 @@
+//===- MemoryUnsafeCastChecker.cpp -*- C++ -*-==//
+//
+// 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: Apache
https://github.com/haoNoQ commented:
Nice nice nice!!
https://github.com/llvm/llvm-project/pull/114606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/haoNoQ edited
https://github.com/llvm/llvm-project/pull/114606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/haoNoQ approved this pull request.
I wholeheartedly support this. I agree with everything you said here. Right now
you're much more of a maintainer than me.
https://github.com/llvm/llvm-project/pull/114991
___
cfe-commits mailing li
https://github.com/haoNoQ approved this pull request.
LGTM!
https://github.com/llvm/llvm-project/pull/109496
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -784,12 +786,12 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
return false; // possibly some user-defined printf function
ASTContext &Ctx = Finder->getASTContext();
- QualType FristParmTy = FD->getParamDecl(0)->getType();
+ QualType FirstParmTy = FD->getParam
https://github.com/haoNoQ edited
https://github.com/llvm/llvm-project/pull/109496
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/haoNoQ approved this pull request.
Ah classic! LGTM!
https://github.com/llvm/llvm-project/pull/109393
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/haoNoQ edited
https://github.com/llvm/llvm-project/pull/109393
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -102,12 +102,13 @@ class UncountedCallArgsChecker
// if ((*P)->hasAttr())
// continue;
-const auto *ArgType = (*P)->getType().getTypePtrOrNull();
-if (!ArgType)
+QualType ArgType = (*P)->getType().getCanonicalType();
+const a
@@ -146,13 +149,67 @@ class NoUncountedMemberChecker
BR->emitReport(std::move(Report));
haoNoQ wrote:
`setDeclWithIssue()` goes into a different PR right?
https://github.com/llvm/llvm-project/pull/108352
___
cfe-
https://github.com/haoNoQ approved this pull request.
LGTM!! I've got nitpicks but none of them are substantial enough to block.
We've figured out the ObjC thing offline right?
https://github.com/llvm/llvm-project/pull/108352
___
cfe-commits mailing l
@@ -134,10 +137,10 @@ class NoUncountedMemberChecker
Os << " in ";
printQuotedQualifiedName(Os, ClassCXXRD);
Os << " is a "
- << (isa(MemberType) ? "raw pointer" : "reference")
- << " to ref-countable type ";
+ << (isa(MemberType) ? "raw pointer" :
@@ -146,13 +149,67 @@ class NoUncountedMemberChecker
BR->emitReport(std::move(Report));
}
};
+
+class NoUncountedMemberChecker final : public RawPtrRefMemberChecker {
haoNoQ wrote:
Yes this is a perfectly valid way to reuse code here!
https://github.com
https://github.com/haoNoQ edited
https://github.com/llvm/llvm-project/pull/108352
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -53,48 +53,49 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, const
char *NameToMatch) {
return hasPublicMethodInBaseClass(R, NameToMatch) ? R : nullptr;
}
-std::optional isRefCountable(const CXXRecordDecl* R)
-{
+std::optional isSmartPtrCompatible(const CXXRecord
@@ -0,0 +1,53 @@
+// RUN: %clang_analyze_cc1
-analyzer-checker=alpha.webkit.NoUncheckedPtrMemberChecker -verify %s
+
+#include "mock-types.h"
+#include "mock-system-header.h"
haoNoQ wrote:
Do you need to include this one everywhere? Isn't it just a tiny test to
@@ -1771,6 +1771,10 @@ def UncountedLambdaCapturesChecker :
Checker<"UncountedLambdaCapturesChecker">,
let ParentPackage = WebKitAlpha in {
+def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
+ HelpText<"Check for no unchecked member variables.">,
+
https://github.com/haoNoQ edited
https://github.com/llvm/llvm-project/pull/109389
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -56,12 +62,16 @@ class UncountedCallArgsChecker
bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) {
if (isRefType(safeGetName(Decl)))
return true;
-return RecursiveASTVisitor::TraverseClassTemplateDecl(
-Decl);
+retur
@@ -134,18 +135,25 @@ class UncountedLocalVarsChecker
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
+ bool TraverseDecl(Decl *D) {
+llvm::SaveAndRestore SavedDecl(DeclWithIssue);
https://github.com/haoNoQ approved this pull request.
LGTM thank you so much!
https://github.com/llvm/llvm-project/pull/109389
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -190,11 +190,7 @@ class UncountedLocalVarsChecker
if (shouldSkipVarDecl(V))
return;
-const auto *ArgType = V->getType().getTypePtr();
-if (!ArgType)
haoNoQ wrote:
Some of these null checks may still be necessary (with `QualType.isNull()`)
https://github.com/haoNoQ approved this pull request.
Yeah sounds about right!
The property syntax is confusing, I don't have enough expertise to confirm that
`getResultExpr()` is the right solution so I think we should keep trying and
incrementally figuring out what's going on.
Setters proba
haoNoQ wrote:
I'm also somewhat terrified of returning C++ objects from ObjC methods by
value, ever since I learned that when you call an ObjC method on a nil it
returns a _zero-initialized_ object without calling a constructor on it.
https://github.com/llvm/llvm-project/pull/108669
__
https://github.com/haoNoQ approved this pull request.
LGTM!
https://github.com/llvm/llvm-project/pull/108656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -84,13 +84,22 @@ class DerefFuncDeleteExprVisitor
E = E->IgnoreParenCasts();
if (auto *TempE = dyn_cast(E))
E = TempE->getSubExpr();
+E = E->IgnoreParenCasts();
+if (auto *Ref = dyn_cast(E)) {
+ if (auto *Decl = Ref->getDecl()) {
+if (auto
@@ -119,6 +119,11 @@ template
ensureOnMainThread([this] {
delete static_cast(this);
});
+} else if constexpr (destructionThread ==
DestructionThread::MainRunLoop) {
+auto deleteThis = [this] {
haoNoQ
https://github.com/haoNoQ edited
https://github.com/llvm/llvm-project/pull/108656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -84,13 +84,22 @@ class DerefFuncDeleteExprVisitor
E = E->IgnoreParenCasts();
if (auto *TempE = dyn_cast(E))
E = TempE->getSubExpr();
+E = E->IgnoreParenCasts();
+if (auto *Ref = dyn_cast(E)) {
+ if (auto *Decl = Ref->getDecl()) {
+if (auto
@@ -119,6 +119,11 @@ template
ensureOnMainThread([this] {
delete static_cast(this);
});
+} else if constexpr (destructionThread ==
DestructionThread::MainRunLoop) {
+auto deleteThis = [this] {
haoNoQ
haoNoQ wrote:
Welcome!!
I completely agree that the message sounds a bit too much like "your code is
garbage" and that's not very nice.
Here's a tangent suggestion to get a better wording out of this. You can
exploit the fact that the static analyzer usually catches undefined values
_very ea
haoNoQ wrote:
Yes, right, this is more of a side effect of how we do things, not quite the
intended behavior. In particular, if you write something out of bounds first,
we will no longer report it as an uninitialized read, even though it's still an
out-of-bounds read: https://godbolt.org/z/PYa
haoNoQ wrote:
So when it comes to the ArrayBoundChecker, the checker that checks that every
access is within bounds, I think that its underlying "model" should be enabled
by default, so that to proactively terminate execution paths on which
uninitialized accesses happen. It should probably eve
haoNoQ wrote:
> The current patch actually WILL work with it ONLY enabled on the line inside
> of the function, which I think is much more intuitive.
Ooo whoa this is awesome!!
https://github.com/llvm/llvm-project/pull/136323
___
cfe-commits mailing
https://github.com/haoNoQ edited
https://github.com/llvm/llvm-project/pull/136323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/haoNoQ commented:
So from the user's perspective we're checking the flag twice: once at the `}`
to see if we need to ramp up the analysis, then again at the diagnostic
location to see if the diagnostic should be discarded. The warning will only be
emitted if it's enabled at
@@ -202,6 +202,43 @@ class SemaPPCallbacks : public PPCallbacks {
break;
}
}
+ void PragmaDiagnostic(SourceLocation Loc, StringRef Namespace,
+diag::Severity Mapping, StringRef Str) override {
+// If one of the analysis-based diagnostics
haoNoQ wrote:
We could also go with something like "uninitialized or inaccessible memory" so
that to technically cover the OOB case without triggering an immediate visceral
reaction to buffer overruns. But it'll probably still be net-negative in
confusion compared to a simple "uninitialized".
Carlos =?utf-8?q?Gálvez?= ,
Carlos =?utf-8?q?Gálvez?=
Message-ID:
In-Reply-To:
haoNoQ wrote:
The static analyzer handles this pretty well already. I haven't heard of any
problems in this area. I think it makes sense to use the same logic by default
in other tools unless you do have specific
haoNoQ wrote:
Should we try to avoid calling the *value* uninitialized? Technically you
initialize variables (or memory), not values. In the static analyzer
terminology in particular, it's somewhat important that values are seen as
immutable data that's temporarily associated with mutable memo
1201 - 1254 of 1254 matches
Mail list logo