[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

chrish_ericsson_atx added inline comments. Herald added a subscriber: manas. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:763-765 + if (const auto *ER = dyn_cast(R)) { +R = StateMgr.getStoreManager().castRegion(ER, CastTy); +return

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:765 +R = StateMgr.getStoreManager().castRegion(ER, CastTy); +return loc::MemRegionVal(R); + } bjope wrote: > This caused some problems with

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

bjope added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:765 +R = StateMgr.getStoreManager().castRegion(ER, CastTy); +return loc::MemRegionVal(R); + } This caused some problems with assertion failures, see

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

This revision was automatically updated to reflect the committed changes. Closed by commit rGb30521c28a4d: [analyzer] Wrong type cast occurs during pointer dereferencing after type… (authored by ASDenysPetrov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. This looks like the fix is in the right place and it looks more or less how I expected it to look. Our casting procedure hopefully became more correct and now we have more correct analysis

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov added a comment. @NoQ, @steakhal, @vsavchenko I think we have met all the conditions with previous patchs to make this patch acceptable. If you think it's OK, could you, please, approve it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89055/new/

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

steakhal added a comment. I'm looking forward to this patch. Comment at: clang/test/Analysis/string.c:367-373 +void *func_strcpy_no_assertion(); +char ***ptr_strcpy_no_assertion; +void strcpy_no_assertion() { + *(unsigned char **)ptr_strcpy_no_assertion = (unsigned char

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov updated this revision to Diff 321693. ASDenysPetrov added a comment. Updated. Rolled the fix over the evalCast refactoring. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89055/new/ https://reviews.llvm.org/D89055 Files: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov added a comment. @NoQ > Ugh, sorry, no, that's `evalCast()`. Like `evalBinOp()` etc. My bad. Can we > also use `evalCast()`? Finally :-) Now we can use `evalCast()` instead of `CastRetrievedVal()`. It would be nice if you could look at D96090 .

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov added a comment. In D89055#2336709 , @NoQ wrote: > Ugh, sorry, no, that's `evalCast()`. Like `evalBinOp()` etc. My bad. Can we > also use `evalCast()`? I dived into `evalCast()`. Initially I had to figure it out and rework it to find the

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov added a comment. @NoQ > The new code should obviously be restricted into evalCastFromLoc() because if > it's a region it's a Loc. The first I tryed was `evalCastFromLoc()`, but it turned out that `SVal` which binds to a pointer can be `NonLoc` as well through violation of

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

NoQ added a comment. Aha, great, sounds like all casting can be made more correct, not just casting on store retrieve! Maybe it's worth celebrating through extra unittests on `evalCast()`. Thank you for looking into this. The new code should obviously be restricted into `evalCastFromLoc()`

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov updated this revision to Diff 298388. ASDenysPetrov added a comment. Updated. Moved `castRegion()` to `SValBuilder`. Actually it wasn't so hard as I thought :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89055/new/ https://reviews.llvm.org/D89055 Files:

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov added a comment. @NoQ > Can we collapse this function further towards this goal? Say, why not pump > every region unconditionally through castRegion()? Does dispatchCast() use > castRegion() internally - and if it does, why are there so many branches in > this function? I've

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov updated this revision to Diff 297822. ASDenysPetrov added a comment. Updated. Removed a new test file, moved the test to an existing file instead. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89055/new/ https://reviews.llvm.org/D89055 Files:

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov updated this revision to Diff 297579. ASDenysPetrov added a comment. Updat patch due to suggestions and fixed formating. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89055/new/ https://reviews.llvm.org/D89055 Files: clang/lib/StaticAnalyzer/Core/Store.cpp

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

steakhal added a comment. If you don't mind I would also request some credit in the summary since I've spent some time on this as well. Besides that I don't have much objection about this patch. It's defenately not my expertiese. Good job coming up with the fix though, I had to focus on other

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov added a comment. @NoQ BTW, what you think we should do with D77062 and D88477 then? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89055/new/

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov added a comment. @NoQ > Can we collapse this function further towards this goal? Say, why not pump > every region //unconditionally// through `castRegion()`? Does > `dispatchCast()` use `castRegion()` internally - and if it does, why are > there so many branches in this

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

NoQ added a comment. This actually looks correct to me but i suggest a bit more investigation. Architecturally, it doesn't make sense to me that `CastRetrievedVal()` does anything beyond `svalBuilder.dispatchCast(V, castTy)`. After all, there's only one way to cast a value from one type to

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

ASDenysPetrov created this revision. ASDenysPetrov added reviewers: steakhal, NoQ, martong, vsavchenko. ASDenysPetrov added a project: clang. Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware,