[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

ASDenysPetrov added a comment. @steakhal > Till then, I recommend you to follow my effort at D88477 > . I'm aleady on this way. I'm debugging the Store. I think we load a wrong type because we store a wrong type. CHANGES SINCE LAST ACTION

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

steakhal added a comment. In D77062#2305856 , @ASDenysPetrov wrote: > @steakhal > >> @ASDenysPetrov Do you still want to rework the API of the `assumeZero`? > > This patch more looks like NFC, being just refactored. Fine. > Actually I see that if we

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

ASDenysPetrov added a comment. @steakhal > @ASDenysPetrov Do you still want to rework the API of the `assumeZero`? This patch more looks like NFC, being just refactored. Actually I see that if we find and fix the root cause, we can freely refuse this patch. Another thing I see is that this

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

steakhal added a comment. In D77062#2298663 , @ASDenysPetrov wrote: > @steakhal > You told that you suppose a potential fix. It would be nice, if you share the > patch to review. I mean, I already told you my suggestion, there was no concrete fix in my

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

NoQ added a comment. Nice, very interesting! The contract of RegionStore with respect to type punning is that it stores the value //as written//, even if its type doesn't match the storage type, but then it casts the value to the correct type upon reading by invoking `CastRetrievedVal()` on

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

ASDenysPetrov added a comment. @steakhal > If we get the value of `**b`, we get a //NonLoc// of type //unsigned char//. > The dump of `**b` confirms this: `reg_$4 Element{SymRegion{reg_$0},0 S64b,unsigned char}>`, which is a > `NonLoc` in deed. Exactly. That's what I've been trying to explaine

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

steakhal added a comment. In D77062#2294516 , @martong wrote: > Though, the fix probably will not be simple, because the issue itself always > requires a 3x indirection. The code that is presented by @steakhal is the > least minimal example to get this

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

martong added a comment. Beware, Phabricator ruins the visual experience of this nice analysis. E.g `//char ***//` is visible as an italic `char *`. > I think we should have a symbolic cast back to the static type before doing > anything with the SVal (iff the BaseKind differs). > If we do

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

steakhal added a comment. And of course, repro: ./bin/clang -cc1 -analyze -setup-static-analyzer -analyzer-checker=core example.c Assertion `op == BO_Add' failed #0 0x7f5bea743904 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int)

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

steakhal added a comment. Finally, I made my investigations and I come up with this code: void strcpy(char *, char *); void test(int *a, char ***b) { *(unsigned char **)b = (unsigned char*)a; // #1 if (**b == nullptr) // will-crash ; } So, this issue does not relate to

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

ASDenysPetrov added a comment. @steakhal > I would not accept this patch unless this investigation is done. However, I'm > not inherently against this change. Actually I've done the investigation. You can find it here https://reviews.llvm.org/D77062#1977613 The bug is somewhere deep in the

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

steakhal added a comment. In D77062#1976414 , @Szelethus wrote: > I think what what be great to see here (and this seems to be the thing @NoQ > is fishing for) is not to change an if branch and avoid running into the > crash, but rather find out why

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

ASDenysPetrov added a comment. One more notification. Please, somebody look at this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77062/new/ https://reviews.llvm.org/D77062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

ASDenysPetrov added a comment. A gentle notification :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77062/new/ https://reviews.llvm.org/D77062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

ASDenysPetrov added a comment. One more ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77062/new/ https://reviews.llvm.org/D77062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

ASDenysPetrov added a comment. @NoQ just one more ping. You accepted it and then I just revised it again. Could you, please, take a minute and look at it. I'd close it with a great pleasure. It's been hanging too long. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77062/new/

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

ASDenysPetrov added a comment. Herald added a subscriber: steakhal. Ping! Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:199 // Utility methods - std::pair - static assumeZero(CheckerContext , -ProgramStateRef state, SVal V, QualType