[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

This revision was automatically updated to reflect the committed changes. Closed by commit rL332463: [analyzer] Improve the modeling of memset(). (authored by henrywong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thanks! This looks great now. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1055 +std::tie(StateNullChar, StateNonNullChar) = +assumeZero(C, State,

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC updated this revision to Diff 146816. MTC added a comment. - According to NoQ's suggestion, use `assumeZero()` instead of `isZeroConstant()` to determine whether the value is 0. - Add test `memset26_upper_UCHAR_MAX()` and `memset27_symbol()` - Since `void *memset( void *dest, int ch, size_t

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1037 + +if (StateWholeReg && !StateNotWholeReg && CharVal.isZeroConstant()) { + // If the 'memset()' acts on the whole region of destination buffer and NoQ wrote: > I

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

NoQ added a comment. Thank you! Looks good overall. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1013-1014 + + // Get the offset and the base region to figure out whether the offset of + // buffer is 0. + RegionOffset Offset = MR->getAsOffset();

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. ping. Repository: rC Clang https://reviews.llvm.org/D44934 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC updated this revision to Diff 145361. MTC added a comment. - Since there is no perfect way to handle the default binding of non-zero character, remove the default binding of non-zero character. Use `bindDefaulrZero()` instead of `overwriteRegion()` to bind the zero character. - Reuse

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. In https://reviews.llvm.org/D44934#1088771, @NoQ wrote: > Hmm, ok, it seems that i've just changed the API in > https://reviews.llvm.org/D46368, and i should have thought about this use > case. Well, at least i have some background understanding of these problems > now.

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

NoQ added a comment. Hmm, ok, it seems that i've just changed the API in https://reviews.llvm.org/D46368, and i should have thought about this use case. Well, at least i have some background understanding of these problems now. Sorry for not keeping eye on this problem. In

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC updated this revision to Diff 145019. MTC added a comment. - fix typos - code refactoring, add auxiliary method `memsetAux()` - according to a.sidorin's suggestions, remove the useless state splitting. - make `StoreManager::overwriteRegion()` pure virtual Repository: rC Clang

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. Sorry for the long delay, I have just finished my holiday. Thanks a lot for the review, I will fix the typo in next update. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2100 +std::tie(StateNullChar, StateNonNullChar) = +

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

a.sidorin added a comment. Hi Henry. I had a quick look at the patch, here are some remarks. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2092 + // 'invalidateRegions()', which will remove the pair + // in CStringLength map. So calls

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. ping^2 Repository: rC Clang https://reviews.llvm.org/D44934 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. Kindly ping! Repository: rC Clang https://reviews.llvm.org/D44934 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC updated this revision to Diff 140629. MTC added a comment. > Thank you for your reminding, I overlooked this point. However for > non-concrete character, the symbol value, if we just invalidate the region, > the constraint information of the non-concrete character will be lost. Do we >

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC updated this revision to Diff 140405. MTC added a comment. According to @NoQ's suggestion, remove the duplicated code. Repository: rC Clang https://reviews.llvm.org/D44934 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. Thanks for your review, NoQ! > Why do you need separate code for null and non-null character? The function's > semantics doesn't seem to care. Thank you for your advice, I will remove the duplicate code！ > I'd rather consider the case of non-concrete character separately.

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

NoQ added a comment. > In addition, `memset` can bind anything to the region, so > `getBindingForDerivedDefaultValue()`'s logic needs some adjustment. **The > solution in this patch is certainly not correct.** Yeah, i guess you meant here the thing that i was saying about concrete bindings.

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

NoQ added a comment. Why do you need separate code for null and non-null character? The function's semantics doesn't seem to care. I'd rather consider the case of non-concrete character separately. Because wiping a region with a symbol is not something we currently support; a symbolic default

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC created this revision. MTC added reviewers: dcoughlin, NoQ, xazax.hun, a.sidorin. Herald added subscribers: cfe-commits, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. This patch originates from https://reviews.llvm.org/D31868. There are two key points in this patch: - Add