This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342832: [CStringSyntaxChecker] Check strlcat sizeof check
(authored by devnexen, committed by ).
Repository:
rC Clang
MaskRay accepted this revision.
MaskRay added a comment.
LG
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
devnexen updated this revision to Diff 166628.
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index: test/Analysis/cstring-syntax.c
===
---
devnexen added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:275
+os << "sizeof(" << DstName << ")";
+ else
+os << "sizeof()";
MaskRay wrote:
> devnexen wrote:
> > MaskRay wrote:
> > > Why can't this `else
MaskRay added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:275
+os << "sizeof(" << DstName << ")";
+ else
+os << "sizeof()";
devnexen wrote:
> MaskRay wrote:
> > Why can't this `else if` case be folded
devnexen updated this revision to Diff 166626.
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index: test/Analysis/cstring-syntax.c
===
---
devnexen added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:275
+os << "sizeof(" << DstName << ")";
+ else
+os << "sizeof()";
MaskRay wrote:
> Why can't this `else if` case be folded into the `strlcpy`
MaskRay added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:275
+os << "sizeof(" << DstName << ")";
+ else
+os << "sizeof()";
Why can't this `else if` case be folded into the `strlcpy` case? There are lots
devnexen added a comment.
ping :)
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
devnexen updated this revision to Diff 165604.
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index: test/Analysis/cstring-syntax.c
===
---
MaskRay added inline comments.
Comment at: test/Analysis/cstring-syntax.c:49
+ strlcat(dest, "0123456789", badlen / 2);
+ strlcat(dest, "0123456789", badlen); // expected-warning {{The third
argument allows to potentially copy more bytes than it should. Replace with the
devnexen added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:199
+if (Append)
+ RemainingBufferLen -= 1;
+if (RemainingBufferLen < ILRawVal)
MaskRay wrote:
> `RemainingBufferLen` is `uint64_t`. Can the
MaskRay added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164
+ // - sizeof(dst)
+ if (Append && isSizeof(LenArg, DstArg))
+return true;
george.karpenkov wrote:
> george.karpenkov wrote:
> > george.karpenkov wrote:
> >
devnexen updated this revision to Diff 164355.
devnexen added a comment.
- Correcting misleading message and advising proper fix.
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index:
devnexen added inline comments.
Comment at: test/Analysis/cstring-syntax.c:49
+ strlcat(dest, "0123456789", badlen / 2);
+ strlcat(dest, "0123456789", badlen); // expected-warning {{The third
argument allows to potentially copy more bytes than it should. Replace with the
NoQ added inline comments.
Comment at: test/Analysis/cstring-syntax.c:49
+ strlcat(dest, "0123456789", badlen / 2);
+ strlcat(dest, "0123456789", badlen); // expected-warning {{The third
argument allows to potentially copy more bytes than it should. Replace with the
value
devnexen added a comment.
ping @george.karpenkov after that I won t bother you for a long time :)
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
devnexen added a comment.
ping
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
devnexen added a comment.
ping
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
devnexen updated this revision to Diff 162139.
devnexen added a comment.
- Returns immediately for both case when sizeof destination.
- Adding few more cases.
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index:
george.karpenkov reopened this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.
This is not correct, `strlcat(dest, "mystr", sizeof(dest))` is perfectly fine,
as can be seen in many examples including strlcat man page.
Repository:
rL LLVM
NoQ added inline comments.
Comment at: cfe/trunk/test/Analysis/cstring-syntax.c:45
+ strlcpy(dest, "012345678", sizeof(dest));
+ strlcat(dest, "910", sizeof(dest)); // expected-warning {{The third argument
allows to potentially copy more bytes than it should. Replace with the
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339641: [CStringSyntaxChecker] Check strlcat sizeof check
(authored by devnexen, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
devnexen updated this revision to Diff 160272.
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index: test/Analysis/cstring-syntax.c
===
---
george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164
+ // - sizeof(dst)
+ if (Append && isSizeof(LenArg, DstArg))
+
george.karpenkov added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164
+ // - sizeof(dst)
+ if (Append && isSizeof(LenArg, DstArg))
+return true;
george.karpenkov wrote:
> I am confused on what is happening in this
devnexen added a comment.
ping but will be for 8.0 :)
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
devnexen added a comment.
ping :)
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
devnexen updated this revision to Diff 158062.
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index: test/Analysis/cstring-syntax.c
===
---
george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:154
-bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) {
+bool
devnexen added a comment.
ping :)
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
devnexen updated this revision to Diff 157381.
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index: test/Analysis/cstring-syntax.c
===
---
george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.
This has too much duplication with respect to your previous patch, and even
Phabricator highlights it in yellow.
Repository:
rC Clang
devnexen added a comment.
Hopefully will try to push it before the freeze just announced, that s my last
change in this area (except potential fixes) :)
Repository:
rC Clang
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
devnexen created this revision.
devnexen added reviewers: george.karpenkov, NoQ.
devnexen created this object with visibility "All Users".
Herald added a subscriber: cfe-commits.
- Assuming strlcat is used with strlcpy we check as we can if the last argument
does not equal os not larger than the
35 matches
Mail list logo
