@@ -186,106 +212,193 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void setASTContext(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void setHandler(const UnsafeBufferUsageHa
https://github.com/ziqingluo-90 approved this pull request.
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ziqingluo-90 wrote:
Please wait a day or two for @jkorous-apple , @malavikasamak and @dtarditi to
take another look.
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bi
ziqingluo-90 wrote:
I don't have big concerns here. Thank you again @ivanaivanovska and
@ilya-biryukov for this effort!
It is also going to be easier to debug and will open more opportunities for
optimization this way. (E.g., I'd like to optimize how "printf" functions are
matched, but was ho
@@ -1670,30 +1936,41 @@ class ULCArraySubscriptGadget : public FixableGadget {
};
// Fixable gadget to handle stand alone pointers of the form `UPC(DRE)` in the
-// unspecified pointer context (isInUnspecifiedPointerContext). The gadget
emits
-// fixit of the form `UPC(DRE.da
@@ -68,32 +70,57 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
if (StParents.size() > 1)
return "unavailable due to multiple parents";
-if (StParents.size() == 0)
+if (StParents.empty())
break;
St = StParents.begin()->get();
@@ -1139,26 +1276,30 @@ class ArraySubscriptGadget : public WarningGadget {
const ArraySubscriptExpr *ASE;
public:
- ArraySubscriptGadget(const MatchFinder::MatchResult &Result)
+ ArraySubscriptGadget(const MatchResult &Result)
: WarningGadget(Kind::ArraySubscript),
@@ -1139,26 +1276,30 @@ class ArraySubscriptGadget : public WarningGadget {
const ArraySubscriptExpr *ASE;
public:
- ArraySubscriptGadget(const MatchFinder::MatchResult &Result)
+ ArraySubscriptGadget(const MatchResult &Result)
: WarningGadget(Kind::ArraySubscript),
@@ -1561,56 +1821,70 @@ class UnsafeLibcFunctionCallGadget : public
WarningGadget {
} WarnedFunKind = OTHERS;
public:
- UnsafeLibcFunctionCallGadget(const MatchFinder::MatchResult &Result)
+ UnsafeLibcFunctionCallGadget(const MatchResult &Result)
: WarningGadget(Ki
https://github.com/ilya-biryukov approved this pull request.
LGTM from my side, but please give some time for the owners to chime in.
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists
@@ -1561,56 +1821,70 @@ class UnsafeLibcFunctionCallGadget : public
WarningGadget {
} WarnedFunKind = OTHERS;
public:
- UnsafeLibcFunctionCallGadget(const MatchFinder::MatchResult &Result)
+ UnsafeLibcFunctionCallGadget(const MatchResult &Result)
: WarningGadget(Ki
@@ -1561,56 +1821,70 @@ class UnsafeLibcFunctionCallGadget : public
WarningGadget {
} WarnedFunKind = OTHERS;
public:
- UnsafeLibcFunctionCallGadget(const MatchFinder::MatchResult &Result)
+ UnsafeLibcFunctionCallGadget(const MatchResult &Result)
: WarningGadget(Ki
@@ -186,106 +212,208 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void setASTContext(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void setHandler(const UnsafeBufferUsageHa
@@ -186,106 +212,208 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void setASTContext(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void setHandler(const UnsafeBufferUsageHa
@@ -186,106 +212,208 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void setASTContext(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void setHandler(const UnsafeBufferUsageHa
@@ -186,106 +212,208 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void setASTContext(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void setHandler(const UnsafeBufferUsageHa
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1986,112 +2360,119 @@ class DerefSimplePtrArithFixableGadget : public
FixableGadget {
}
};
-/// Scan the function and return a list of gadgets found with provided kits.
-static void findGadgets(const Stmt *S, ASTContext &Ctx,
-const UnsafeBufferU
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1986,112 +2360,119 @@ class DerefSimplePtrArithFixableGadget : public
FixableGadget {
}
};
-/// Scan the function and return a list of gadgets found with provided kits.
-static void findGadgets(const Stmt *S, ASTContext &Ctx,
-const UnsafeBufferU
@@ -1986,112 +2360,119 @@ class DerefSimplePtrArithFixableGadget : public
FixableGadget {
}
};
-/// Scan the function and return a list of gadgets found with provided kits.
-static void findGadgets(const Stmt *S, ASTContext &Ctx,
-const UnsafeBufferU
@@ -2672,7 +3053,7 @@ static inline std::optional
createDataFixit(const ASTContext &Ctx,
// `DRE.data()`
std::optional
UPCStandalonePointerGadget::getFixits(const FixitStrategy &S) const {
- const auto VD = cast(Node->getDecl());
ilya-biryukov wrote:
NIT: we
@@ -1986,112 +2360,119 @@ class DerefSimplePtrArithFixableGadget : public
FixableGadget {
}
};
-/// Scan the function and return a list of gadgets found with provided kits.
-static void findGadgets(const Stmt *S, ASTContext &Ctx,
-const UnsafeBufferU
https://github.com/ilya-biryukov commented:
@ivanaivanovska I only have NITs so far, could you address them?
I will make sure to finish the few other functions I have left "note to self"
for, but I don't actually expect much to pop up there.
As soon as the comments are addressed, I think we are
@@ -1948,31 +2294,59 @@ class DerefSimplePtrArithFixableGadget : public
FixableGadget {
const IntegerLiteral *Offset = nullptr;
public:
- DerefSimplePtrArithFixableGadget(const MatchFinder::MatchResult &Result)
+ DerefSimplePtrArithFixableGadget(const MatchResult &Result)
@@ -1948,31 +2294,59 @@ class DerefSimplePtrArithFixableGadget : public
FixableGadget {
const IntegerLiteral *Offset = nullptr;
public:
- DerefSimplePtrArithFixableGadget(const MatchFinder::MatchResult &Result)
+ DerefSimplePtrArithFixableGadget(const MatchResult &Result)
@@ -1460,30 +1694,32 @@ class UnsafeBufferUsageAttrGadget : public
WarningGadget {
DeclUseList getClaimedVarUseSites() const override { return {}; }
};
-/// A call of a constructor that performs unchecked buffer operations
-/// over one of its pointer parameters, or constru
@@ -1901,28 +2238,37 @@ class UUCAddAssignGadget : public FixableGadget {
const Expr *Offset = nullptr;
public:
- UUCAddAssignGadget(const MatchFinder::MatchResult &Result)
+ UUCAddAssignGadget(const MatchResult &Result)
: FixableGadget(Kind::UUCAddAssign),
-
@@ -1505,23 +1741,51 @@ class DataInvocationGadget : public WarningGadget {
const ExplicitCastExpr *Op;
public:
- DataInvocationGadget(const MatchFinder::MatchResult &Result)
+ DataInvocationGadget(const MatchResult &Result)
: WarningGadget(Kind::DataInvocation),
-
@@ -1636,24 +1914,33 @@ class ULCArraySubscriptGadget : public FixableGadget {
const ArraySubscriptExpr *Node;
public:
- ULCArraySubscriptGadget(const MatchFinder::MatchResult &Result)
+ ULCArraySubscriptGadget(const MatchResult &Result)
: FixableGadget(Kind::ULCArr
@@ -1505,23 +1741,51 @@ class DataInvocationGadget : public WarningGadget {
const ExplicitCastExpr *Op;
public:
- DataInvocationGadget(const MatchFinder::MatchResult &Result)
+ DataInvocationGadget(const MatchResult &Result)
: WarningGadget(Kind::DataInvocation),
-
@@ -1636,24 +1914,33 @@ class ULCArraySubscriptGadget : public FixableGadget {
const ArraySubscriptExpr *Node;
public:
- ULCArraySubscriptGadget(const MatchFinder::MatchResult &Result)
+ ULCArraySubscriptGadget(const MatchResult &Result)
: FixableGadget(Kind::ULCArr
@@ -1711,25 +2008,38 @@ class PointerDereferenceGadget : public FixableGadget {
const UnaryOperator *Op = nullptr;
public:
- PointerDereferenceGadget(const MatchFinder::MatchResult &Result)
+ PointerDereferenceGadget(const MatchResult &Result)
: FixableGadget(Kind::
@@ -1505,23 +1741,51 @@ class DataInvocationGadget : public WarningGadget {
const ExplicitCastExpr *Op;
public:
- DataInvocationGadget(const MatchFinder::MatchResult &Result)
+ DataInvocationGadget(const MatchResult &Result)
: WarningGadget(Kind::DataInvocation),
-
@@ -1561,56 +1825,70 @@ class UnsafeLibcFunctionCallGadget : public
WarningGadget {
} WarnedFunKind = OTHERS;
public:
- UnsafeLibcFunctionCallGadget(const MatchFinder::MatchResult &Result)
+ UnsafeLibcFunctionCallGadget(const MatchResult &Result)
: WarningGadget(Ki
@@ -1505,23 +1741,51 @@ class DataInvocationGadget : public WarningGadget {
const ExplicitCastExpr *Op;
public:
- DataInvocationGadget(const MatchFinder::MatchResult &Result)
+ DataInvocationGadget(const MatchResult &Result)
: WarningGadget(Kind::DataInvocation),
-
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1238,27 +1413,36 @@ class SpanTwoParamConstructorGadget : public
WarningGadget {
const CXXConstructExpr *Ctor; // the span constructor expression
public:
- SpanTwoParamConstructorGadget(const MatchFinder::MatchResult &Result)
+ SpanTwoParamConstructorGadget(const Matc
@@ -1460,30 +1694,32 @@ class UnsafeBufferUsageAttrGadget : public
WarningGadget {
DeclUseList getClaimedVarUseSites() const override { return {}; }
};
-/// A call of a constructor that performs unchecked buffer operations
-/// over one of its pointer parameters, or constru
@@ -1460,30 +1694,32 @@ class UnsafeBufferUsageAttrGadget : public
WarningGadget {
DeclUseList getClaimedVarUseSites() const override { return {}; }
};
-/// A call of a constructor that performs unchecked buffer operations
-/// over one of its pointer parameters, or constru
https://github.com/ilya-biryukov commented:
I had another round, left a few more comments, mostly NITs.
I feel we should definitely get rid of `getQualifiedNameAsString` calls, though.
There's a few more lines left, I'll get to them soon.
https://github.com/llvm/llvm-project/pull/125492
___
https://github.com/ilya-biryukov deleted
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -294,63 +426,81 @@ isInUnspecifiedPointerContext(internal::Matcher
InnerMatcher) {
// 4. the operand of a pointer subtraction operation
//(i.e., computing the distance between two pointers); or ...
- // clang-format off
- auto CallArgMatcher = callExpr(
+ if (au
@@ -1060,19 +1210,24 @@ class IncrementGadget : public WarningGadget {
const UnaryOperator *Op;
public:
- IncrementGadget(const MatchFinder::MatchResult &Result)
+ IncrementGadget(const MatchResult &Result)
: WarningGadget(Kind::Increment),
-Op(Result.Nodes.
@@ -839,17 +988,22 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
// second is an integer, it is a snprintf:
const Expr *UnsafeArg;
- if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 2, Ctx, false))
-return UnsafeStringArgMatcher.matches(*UnsafeArg, F
@@ -1189,29 +1353,40 @@ class PointerArithmeticGadget : public WarningGadget {
const Expr *Ptr; // the pointer expression in `PA`
public:
- PointerArithmeticGadget(const MatchFinder::MatchResult &Result)
+ PointerArithmeticGadget(const MatchResult &Result)
---
@@ -1139,26 +1299,30 @@ class ArraySubscriptGadget : public WarningGadget {
const ArraySubscriptExpr *ASE;
public:
- ArraySubscriptGadget(const MatchFinder::MatchResult &Result)
+ ArraySubscriptGadget(const MatchResult &Result)
: WarningGadget(Kind::ArraySubscript),
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ilya-biryukov commented:
I am still working on going through the commit very thoroughly.
It all looks great, just a few minor comments, and I've also left a few notes
to not forget where I stopped
https://github.com/llvm/llvm-project/pull/125492
__
@@ -186,106 +212,212 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void setASTContext(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void setHandler(const UnsafeBufferUsageHa
@@ -787,9 +933,9 @@ AST_MATCHER(FunctionDecl, isNormalPrintfFunc) {
// Then if the format string is a string literal, this matcher matches when at
// least one string argument is unsafe. If the format is not a string literal,
// this matcher matches when at least one pointer ty
@@ -186,106 +212,212 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void setASTContext(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void setHandler(const UnsafeBufferUsageHa
ivanaivanovska wrote:
> @ivanaivanovska @ilya-biryukov One PR is fine. I will look at it.
@ziqingluo-90 Thank you!
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/
ilya-biryukov wrote:
I am definitely happy to review the code very carefully one more time and also
commit to quickly revert or fix any discrepancies if anything happens
downstream (would only need reproducers and such if we do miss something).
We definitely wants to enable this warning and are
https://github.com/ivanaivanovska updated
https://github.com/llvm/llvm-project/pull/125492
>From 54c7b3c1fb149b82c26927d0fd831d8786f70ac3 Mon Sep 17 00:00:00 2001
From: Ivana Ivanovska
Date: Mon, 2 Dec 2024 14:17:06 +
Subject: [PATCH 1/2] Optimize -Wunsafe-buffer-usage.
---
clang/lib/Anal
jkorous-apple wrote:
I believe we should be able to remove dependency on AST Matchers dylib as AFAIK
the only use was in UnsafeBufferUsage.cpp.
https://github.com/llvm/llvm-project/blob/main/clang/lib/Analysis/CMakeLists.txt#L40
https://github.com/llvm/llvm-project/pull/125492
_
jkorous-apple wrote:
Hi @ilya-biryukov and @ivanaivanovska!
Unfortunately I currently don't have bandwidth for but I like this direction a
lot and appreciate the effort you put into optimizing the analysis!
https://github.com/llvm/llvm-project/pull/125492
___
ziqingluo-90 wrote:
> > I added @ziqingluo-90 @jkorous-apple as reviewers since you've approved
> > recent changes to this warning. Please let us know if you're the right
> > reviewers for this and feel free to loop in more people if necessary.
>
> Thank you @ivanaivanovska, the profiling and
ilya-biryukov wrote:
I added @ziqingluo-90 @jkorous-apple as reviewers since you've approved recent
changes to this warning.
Please let us know if you're the right reviewers for this and feel free to loop
in more people if necessary.
https://github.com/llvm/llvm-project/pull/125492
___
@@ -1048,7 +1197,10 @@ class FixableGadget : public Gadget {
}
};
-static auto toSupportedVariable() { return to(varDecl()); }
+static auto toSupportedVariable(const DeclRefExpr &Node) {
ilya-biryukov wrote:
NIT: replace `auto` with `bool`, rename to `isSup
@@ -1189,29 +1355,40 @@ class PointerArithmeticGadget : public WarningGadget {
const Expr *Ptr; // the pointer expression in `PA`
public:
- PointerArithmeticGadget(const MatchFinder::MatchResult &Result)
+ PointerArithmeticGadget(const MatchResult &Result)
@@ -2672,7 +3055,7 @@ static inline std::optional
createDataFixit(const ASTContext &Ctx,
// `DRE.data()`
std::optional
UPCStandalonePointerGadget::getFixits(const FixitStrategy &S) const {
- const auto VD = cast(Node->getDecl());
+ const auto *const VD = cast(Node->getDecl()
@@ -68,32 +70,59 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
if (StParents.size() > 1)
return "unavailable due to multiple parents";
-if (StParents.size() == 0)
+if (StParents.empty())
break;
St = StParents.begin()->get();
@@ -68,32 +70,59 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
if (StParents.size() > 1)
return "unavailable due to multiple parents";
-if (StParents.size() == 0)
+if (StParents.empty())
break;
St = StParents.begin()->get();
@@ -186,106 +214,212 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void setASTContext(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void setHandler(const UnsafeBufferUsageHa
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ilya-biryukov commented:
We really want this to enable the `Wunsafe-buffer-usage` more broadly in our
codebase.
Right now the performance impact is just way too high for us.
So thanks a lot for working on this!
Obviously, we need to add people who own the particular piece of
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Ivana Ivanovska (ivanaivanovska)
Changes
The Clang disgnostic `-Wunsafe-buffer-usage` was adding up to +15% compilation
time when used. Profiling showed that most of the overhead comes from the use
of ASTMatchers.
This change replaces th
https://github.com/ivanaivanovska ready_for_review
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ivanaivanovska edited
https://github.com/llvm/llvm-project/pull/125492
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ivanaivanovska created
https://github.com/llvm/llvm-project/pull/125492
None
>From 54c7b3c1fb149b82c26927d0fd831d8786f70ac3 Mon Sep 17 00:00:00 2001
From: Ivana Ivanovska
Date: Mon, 2 Dec 2024 14:17:06 +
Subject: [PATCH] Optimize -Wunsafe-buffer-usage.
---
clang/lib/An
71 matches
Mail list logo
