This revision was automatically updated to reflect the committed changes.
Closed by commit rG99b94f29ac5d: [analyzer] LoopUnrolling: fix crash when a
parameter is a loop counter. (authored by dergachev.a).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org
AbbasSabra added a comment.
Great! Can someone take care of merging it? I believe I don't have access.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80171/new/
https://reviews.llvm.org/D80171
___
cfe-c
vsavchenko accepted this revision.
vsavchenko added a comment.
LGTM! Thanks again!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80171/new/
https://reviews.llvm.org/D80171
___
cfe-commits mailing list
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks great, thanks!
This is probably not the precise solution (i.e., we might be able to see where
exactly is the reference taken and proceed from there) but neither is the
original code - it's al
AbbasSabra updated this revision to Diff 265253.
AbbasSabra added a comment.
Fix code review 2
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80171/new/
https://reviews.llvm.org/D80171
Files:
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
clan
AbbasSabra marked 2 inline comments as done.
AbbasSabra added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:167
+ const bool isParm = VD->getKind() == Decl::ParmVar;
+ // Reference parameters are assumed as escaped variables.
vs
vsavchenko added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:167
+ const bool isParm = VD->getKind() == Decl::ParmVar;
+ // Reference parameters are assumed as escaped variables.
`getKind` function is only an implementation de
AbbasSabra updated this revision to Diff 265095.
AbbasSabra added a comment.
update
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80171/new/
https://reviews.llvm.org/D80171
Files:
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
clang/test/Anal
AbbasSabra requested review of this revision.
AbbasSabra added a comment.
What you both said makes sense. Now, reference parameters are escaped and
value parameters are treated as the local variables.
Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.ll
AbbasSabra updated this revision to Diff 265090.
AbbasSabra added a comment.
Fix code review
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80171/new/
https://reviews.llvm.org/D80171
Files:
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
clang/
vsavchenko added a comment.
Thanks for taking your time to find and resolve the bug.
I agree that this solution seems a bit too harsh. Technically there is no
difference between local variable declaration and a parameter declaration (if
we are talking about value types as pointed out by @xazax.
xazax.hun added inline comments.
Comment at: clang/test/Analysis/loop-unrolling.cpp:503
+
+void arg_as_loop_counter(int i) {
+ for (i = 0; i < 10; ++i) {
nit: we usually use `arg` for actual arguments at the call site, and use
`param` for formal parameters. The
xazax.hun added reviewers: NoQ, vsavchenko.
xazax.hun added a comment.
Thanks for finding this bug! Adding some reviewers.
I think it would be perfectly fine to unroll loops where the loop counter is a
pass-by-value parameter. That should not be considered escaped.
I think in case of parameters
AbbasSabra created this revision.
AbbasSabra added a reviewer: szepet.
Herald added subscribers: ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy,
Szelethus, mikhail.ramalho, a.sidorin, zzheng, rnkovacs, baloghadamsoftware,
xazax.hun.
Herald added a project: clang.
When loop counter is a fun
14 matches
Mail list logo
