<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"></blockquote>Hi Lenny,<br><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote>On Dec 15, 2010, at 8:51 AM, Leonard Maiorani wrote:<br><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><blockquote type="cite" style="color: rgb(0, 0, 0); "><div>Attached is my first foray into Clang Static Analyzer contributions. Please review for style and correctness. Take punches at it if you will, I am tough enough. Unit tests are included.<br><br>strncpy() was not being checked at all, so this should solve that problem by ensuring that the 'n' parameter to strncpy() is not larger than the size of the buffer passed in because that would indicate a logic error and potential bus error.<br></div></blockquote><div><div><br></div></div></font><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote> BuiltinBug *BT;<br><blockquote type="cite"></blockquote> if (IsDestination) {<br><blockquote type="cite"></blockquote> if (!BT_BoundsWrite) {<br><blockquote type="cite"></blockquote>- BT_BoundsWrite = new BuiltinBug("Out-of-bound array access",<br><blockquote type="cite"></blockquote>- "Byte string function overflows destination buffer");<br><blockquote type="cite"></blockquote>+ if (IsDefinedLength) {<br><blockquote type="cite"></blockquote>+ BT_BoundsWrite = new BuiltinBug("Out-of-bound array access",<br><blockquote type="cite"></blockquote>+ "Byte string function length parameter overflows destination buffer");<br><blockquote type="cite"></blockquote>+ } else {<br><blockquote type="cite"></blockquote>+ BT_BoundsWrite = new BuiltinBug("Out-of-bound array access",<br><blockquote type="cite"></blockquote>+ "Byte string function overflows destination buffer");<br><blockquote type="cite"></blockquote>+ }<br><blockquote type="cite"></blockquote> }<br><blockquote type="cite"></blockquote> BT = static_cast<BuiltinBug*>(BT_BoundsWrite);<br><blockquote type="cite"></blockquote> } else {<br><blockquote type="cite"></blockquote> if (!BT_Bounds) {<br><blockquote type="cite"></blockquote>- BT_Bounds = new BuiltinBug("Out-of-bound array access",<br><blockquote type="cite"></blockquote>- "Byte string function accesses out-of-bound array element");<br><blockquote type="cite"></blockquote>+ if (IsDefinedLength) {<br><blockquote type="cite"></blockquote>+ BT_BoundsWrite = new BuiltinBug("Out-of-bound array access",<br><blockquote type="cite"></blockquote>+ "Byte string function length parameter accesses out-of-bound element");<br><blockquote type="cite"></blockquote>+ } else {<br><blockquote type="cite"></blockquote>+ BT_Bounds = new BuiltinBug("Out-of-bound array access",<br><blockquote type="cite"></blockquote>+ "Byte string function accesses out-of-bound array element");<br><blockquote type="cite"></blockquote>+ }<br><blockquote type="cite"></blockquote> }<br><blockquote type="cite"></blockquote> BT = static_cast<BuiltinBug*>(BT_Bounds);<br><blockquote type="cite"></blockquote> }<br><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote>BT_BoundsWrite/BT_Bounds is created once and reused, so this change sets the bug string for all strcpy and strncpy bugs in the same function,<br><blockquote type="cite"></blockquote>depending on what occured first.<br><blockquote type="cite"></blockquote>You probably meant to use new BuiltinBugs (e.g. BT_BoundsWrite_strncpy) but I think it's adequate to leave it as is and use the same message;<br><blockquote type="cite"></blockquote>then the IsDefinedLength parameter won't be needed.<br><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote>void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,<br><blockquote type="cite"></blockquote>- bool returnEnd) {<br><blockquote type="cite"></blockquote>+ bool returnEnd, bool specifiedLen) {<br><blockquote type="cite"></blockquote> const GRState *state = C.getState();<br><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote> // Check that the destination is non-null<br><blockquote type="cite"></blockquote>@@ -828,6 +847,12 @@<br><blockquote type="cite"></blockquote> // Get the string length of the source.<br><blockquote type="cite"></blockquote> SVal strLength = getCStringLength(C, state, srcExpr, srcVal);<br><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote>+ if (specifiedLen) {<br><blockquote type="cite"></blockquote>+ // Check if the number of bytes to copy is less than the size of the src<br><blockquote type="cite"></blockquote>+ const Expr *lenExpr = CE->getArg(2);<br><blockquote type="cite"></blockquote>+ strLength = state->getSVal(lenExpr);<br><blockquote type="cite"></blockquote>+ }<br><blockquote type="cite"></blockquote>+<br><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote>Maybe rename specifiedLen to isStrncpy to make it more clear that this affects what kind of CallExpr we got ?<br><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote>Other than that, it looks good, thanks for working in it.<br><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote>As an aside, the CStringChecker probably needs some refactoring and improvement to warn for non null-terminated strings,<br><blockquote type="cite"></blockquote>but this would require some infrastructure changes.<br><blockquote type="cite"></blockquote><font class="Apple-style-span" color="#144FAE"><br></font><blockquote type="cite"></blockquote>-Argiris<br></div><br></body></html>