[PATCH] D47007: [Sanitizer] CStringChecker fix for strlcpy when no bytes are copied to the dest buffer

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

I've been thinking if we could de-duplicate this whole set of branches that 
computes the return value so that we didn't have to fix every bug twice. Maybe 
move it to an auxiliary function.




Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1795
 // copied element, or a pointer to the start of the destination buffer.
 Result = (returnEnd ? UnknownVal() : DstVal);
   } else {

Do we need to consider `returnEnd` on the short path as well?



Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1869-1875
   if (returnPtr) {
 // If this is a stpcpy-style copy, but we were unable to check for a buffer
 // overflow, we still need a result. Conjure a return value.
 if (returnEnd && Result.isUnknown()) {
   Result = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
 }
   }

Do we need to do that on the short path as well?


Repository:
  rC Clang

https://reviews.llvm.org/D47007



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47007: [Sanitizer] CStringChecker fix for strlcpy when no bytes are copied to the dest buffer

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

In https://reviews.llvm.org/D47007#1103551, @george.karpenkov wrote:

> Is it a fix for https://bugs.llvm.org/show_bug.cgi?id=37503 ?


Nope. more for last NoQ comment. Will try for this one once I finish setting it 
up.


Repository:
  rC Clang

https://reviews.llvm.org/D47007



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47007: [Sanitizer] CStringChecker fix for strlcpy when no bytes are copied to the dest buffer

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

Is it a fix for https://bugs.llvm.org/show_bug.cgi?id=37503 ?


Repository:
  rC Clang

https://reviews.llvm.org/D47007



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47007: [Sanitizer] CStringChecker fix for strlcpy when no bytes are copied to the dest buffer

[David CARLIER via Phabricator via cfe-commits]

devnexen created this revision.
devnexen added reviewers: NoQ, george.karpenkov.
devnexen created this object with visibility "All Users".
Herald added a subscriber: cfe-commits.

Again strlc* does not return a pointer so the zero size case does not fit.


Repository:
  rC Clang

https://reviews.llvm.org/D47007

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/bsd-string.c


Index: test/Analysis/bsd-string.c
===
--- test/Analysis/bsd-string.c
+++ test/Analysis/bsd-string.c
@@ -38,3 +38,8 @@
   size_t len = strlcat(buf, "defg", 4);
   clang_analyzer_eval(len == 7); // expected-warning{{TRUE}}
 }
+
+int f7() {
+  char buf[8];
+  return strlcpy(buf, "1234567", 0); // no-crash
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1652,7 +1652,11 @@
 
 // If the size is known to be zero, we're done.
 if (StateZeroSize && !StateNonZeroSize) {
-  StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
+  if (returnPtr) {
+StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
+  } else {
+StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, *lenValNL);
+  }
   C.addTransition(StateZeroSize);
   return;
 }


Index: test/Analysis/bsd-string.c
===
--- test/Analysis/bsd-string.c
+++ test/Analysis/bsd-string.c
@@ -38,3 +38,8 @@
   size_t len = strlcat(buf, "defg", 4);
   clang_analyzer_eval(len == 7); // expected-warning{{TRUE}}
 }
+
+int f7() {
+  char buf[8];
+  return strlcpy(buf, "1234567", 0); // no-crash
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1652,7 +1652,11 @@
 
 // If the size is known to be zero, we're done.
 if (StateZeroSize && !StateNonZeroSize) {
-  StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
+  if (returnPtr) {
+StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal);
+  } else {
+StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, *lenValNL);
+  }
   C.addTransition(StateZeroSize);
   return;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits