[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 14 13:50:57 PDT 2018


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:
> > > I am confused on what is happening in this branch and why is it bad. Could we add a commend?
> > Sorry I'm still confused about this branch: could you clarify?
> Ah, OK, I see it needs one more byte due to null-termination.
I think the `if (isSizeof(LenArg, DstArg))` check is fine so it returns `false`.

`strlcat(dst, src, sizeof dst)` is good.


================
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:199
+        if (Append)
+          RemainingBufferLen -= 1;
+        if (RemainingBufferLen < ILRawVal)
----------------
`RemainingBufferLen` is `uint64_t`. Can the `-= 1` overflow?


https://reviews.llvm.org/D49722





More information about the cfe-commits mailing list