Hi Lenny,
On Dec 15, 2010, at 8:51 AM, Leonard Maiorani wrote:
> 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.
>
> 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.
BuiltinBug *BT;
if (IsDestination) {
if (!BT_BoundsWrite) {
- BT_BoundsWrite = new BuiltinBug("Out-of-bound array access",
- "Byte string function overflows destination buffer");
+ if (IsDefinedLength) {
+ BT_BoundsWrite = new BuiltinBug("Out-of-bound array access",
+ "Byte string function length parameter overflows destination
buffer");
+ } else {
+ BT_BoundsWrite = new BuiltinBug("Out-of-bound array access",
+ "Byte string function overflows destination buffer");
+ }
}
BT = static_cast<BuiltinBug*>(BT_BoundsWrite);
} else {
if (!BT_Bounds) {
- BT_Bounds = new BuiltinBug("Out-of-bound array access",
- "Byte string function accesses out-of-bound array element");
+ if (IsDefinedLength) {
+ BT_BoundsWrite = new BuiltinBug("Out-of-bound array access",
+ "Byte string function length parameter accesses out-of-bound
element");
+ } else {
+ BT_Bounds = new BuiltinBug("Out-of-bound array access",
+ "Byte string function accesses out-of-bound array element");
+ }
}
BT = static_cast<BuiltinBug*>(BT_Bounds);
}
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,
depending on what occured first.
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;
then the IsDefinedLength parameter won't be needed.
void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
- bool returnEnd) {
+ bool returnEnd, bool specifiedLen) {
const GRState *state = C.getState();
// Check that the destination is non-null
@@ -828,6 +847,12 @@
// Get the string length of the source.
SVal strLength = getCStringLength(C, state, srcExpr, srcVal);
+ if (specifiedLen) {
+ // Check if the number of bytes to copy is less than the size of the src
+ const Expr *lenExpr = CE->getArg(2);
+ strLength = state->getSVal(lenExpr);
+ }
+
Maybe rename specifiedLen to isStrncpy to make it more clear that this affects
what kind of CallExpr we got ?
Other than that, it looks good, thanks for working in it.
As an aside, the CStringChecker probably needs some refactoring and improvement
to warn for non null-terminated strings,
but this would require some infrastructure changes.
-Argiris
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
