ilya-biryukov wrote:
FYI: this PR was taking too long to pick changes after rebase, so the
discussions continued in #125492.
https://github.com/llvm/llvm-project/pull/124554
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.or
https://github.com/ivanaivanovska closed
https://github.com/llvm/llvm-project/pull/124554
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1986,112 +2362,117 @@ 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:
This generally looks good, just one more minor comment from me.
Could we put this out of the draft state and send to upstream owners of the
check to start a discussion about landing it?
https://github.com/llvm/llvm-project/pull/124554
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/124554
___
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/124554
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -68,32 +70,60 @@ 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,60 @@ 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();
https://github.com/ivanaivanovska updated
https://github.com/llvm/llvm-project/pull/124554
504 Gateway Time-out
The server didn't respond in time.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo
@@ -68,32 +70,60 @@ 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,60 @@ 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();
https://github.com/ilya-biryukov commented:
I only have a few suggestions left, overall this LG.
Could you also add a description for the PR that explains the motivation and
approach taken for the rewrite?
Benchmark numbers would also be needed there.
Thanks!
https://github.com/llvm/llvm-pro
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/124554
___
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/124554
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -186,106 +213,221 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void setASTContext(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void setHandler(const UnsafeBufferUsageHa
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
https://github.com/ivanaivanovska updated
https://github.com/llvm/llvm-project/pull/124554
>From 9890ff5dd8977a5d70538a2034517d19b8399536 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/Analysis
@@ -186,106 +213,221 @@ 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/124554
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/124554
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/124554
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -1986,112 +2327,142 @@ 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
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -79,21 +81,39 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
} // namespace
#endif /* NDEBUG */
-namespace clang::ast_matchers {
+class CustomMatcher {
+public:
+ virtual bool matches(const DynTypedNode &DynNode, ASTContext &Ctx,
+
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
ivanaivanovska wrote:
Done
@@ -79,21 +81,39 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
} // namespace
#endif /* NDEBUG */
-namespace clang::ast_matchers {
+class CustomMatcher {
ivanaivanovska wrote:
Done.
https://github.com/llvm/llvm-project/pull/124554
_
@@ -79,21 +81,39 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
} // namespace
#endif /* NDEBUG */
-namespace clang::ast_matchers {
+class CustomMatcher {
+public:
+ virtual bool matches(const DynTypedNode &DynNode, ASTContext &Ctx,
+
@@ -79,21 +81,39 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
} // namespace
#endif /* NDEBUG */
-namespace clang::ast_matchers {
+class CustomMatcher {
ivanaivanovska wrote:
Done.
https://github.com/llvm/llvm-project/pull/124554
_
@@ -79,21 +81,39 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
} // namespace
#endif /* NDEBUG */
-namespace clang::ast_matchers {
+class CustomMatcher {
+public:
+ virtual bool matches(const DynTypedNode &DynNode, ASTContext &Ctx,
+
@@ -31,27 +35,34 @@
#define FIXABLE_GADGET(name) GADGET(name)
#endif
+/// A subset of the safe gadgets that may return multiple results.
+#ifndef FIXABLE_GADGET_MULTY_RES
+#define FIXABLE_GADGET_MULTY_RES(name) GADGET(name)
+#endif
+
WARNING_GADGET(Increment)
WARNING_GADGET(
https://github.com/ivanaivanovska updated
https://github.com/llvm/llvm-project/pull/124554
>From 3cda247a87fc4f084b33edb69e33f6d3797d0d92 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/Analysis
@@ -1986,112 +2327,142 @@ 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
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -79,21 +81,39 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
} // namespace
#endif /* NDEBUG */
-namespace clang::ast_matchers {
+class CustomMatcher {
+public:
+ virtual bool matches(const DynTypedNode &DynNode, ASTContext &Ctx,
+
@@ -79,21 +81,39 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
} // namespace
#endif /* NDEBUG */
-namespace clang::ast_matchers {
+class CustomMatcher {
ilya-biryukov wrote:
NIT: maybe rename to `FastMatcher`?
We should definitely add
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/124554
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -31,27 +35,34 @@
#define FIXABLE_GADGET(name) GADGET(name)
#endif
+/// A subset of the safe gadgets that may return multiple results.
+#ifndef FIXABLE_GADGET_MULTY_RES
+#define FIXABLE_GADGET_MULTY_RES(name) GADGET(name)
+#endif
+
WARNING_GADGET(Increment)
WARNING_GADGET(
@@ -79,21 +81,39 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
} // namespace
#endif /* NDEBUG */
-namespace clang::ast_matchers {
+class CustomMatcher {
+public:
+ virtual bool matches(const DynTypedNode &DynNode, ASTContext &Ctx,
+
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -79,21 +81,39 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
} // namespace
#endif /* NDEBUG */
-namespace clang::ast_matchers {
+class CustomMatcher {
ilya-biryukov wrote:
this class must go into an anonymous `namespace {}` to avoid O
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void set_handler(const UnsafeBufferUsag
@@ -186,106 +205,202 @@ class MatchDescendantVisitor : public
DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void set_ast_context(ASTContext &Context) { ActiveASTContext = &Context; }
ilya-biryukov wrote:
NIT:
@@ -79,21 +81,39 @@ static std::string getDREAncestorString(const DeclRefExpr
*DRE,
} // namespace
#endif /* NDEBUG */
-namespace clang::ast_matchers {
+class CustomMatcher {
+public:
+ virtual bool matches(const DynTypedNode &DynNode, ASTContext &Ctx,
+
https://github.com/ilya-biryukov commented:
Thanks a lot for this!
I have left quite a few comments, mostly NITs.
A few things that really stood out and I think we should fix:
- increasing the number of macros and signatures we use seems like a bad
trade-off. It's much better to have more param
https://github.com/ivanaivanovska created
https://github.com/llvm/llvm-project/pull/124554
Details of the optimization results will follow.
>From 1bfe3f3fcf2385c67057a19baf9ddf3b14aeeb6d Mon Sep 17 00:00:00 2001
From: Ivana Ivanovska
Date: Mon, 2 Dec 2024 14:17:06 +
Subject: [PATCH] Optimi
54 matches
Mail list logo
