[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-10-10 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. In D88477#2319959 , @ASDenysPetrov wrote: > @NoQ @steakhal > > Hi, guys. > > I've just uploaded a patch for solving this and related D77062 > . Welcome to review D89055 >

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-10-08 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. @NoQ @steakhal Hi, guys. I've just uploaded a patch for solving this and related D77062 . Welcome to review D89055 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-10-05 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. > i'm pretty worried about our ability to actually achieve that in the near > future This whole cast problem that you're looking into in D85528 is definitely a part of it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-10-01 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. > A value of an expression should have the same type and value-kind as the > expression. F13121967: roflbot.jpg Unfortunately i'm pretty worried about our ability to actually achieve that in the near future; as of now we don't even

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-10-01 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. No, you got it all wrong again. I don't want to explain this one more time so let's talk about some basics: //A value of an expression should have the same type and value-kind as the expression//. Can we get there? How? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-10-01 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. In D88477#2304708 , @NoQ wrote: > I'm trying to say that the value produced by the load should not be the same > as the stored value, because these two values are of different types. When > exactly does the first value change

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-09-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. (in the latest message by "load" i mean the first load that produces a pointer (i.e., an `ElementRegion`) as the result) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88477/new/ https://reviews.llvm.org/D88477

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-09-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. I'm trying to say that the value produced by the load should not be the same as the stored value, because these two values are of different types. When exactly does the first value change into the second value is a different story; the current grand vision around which the

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-09-30 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. I'm getting lost :D In D88477#2304230 , @NoQ wrote: > And I believe that this part is already incorrect. Like, regardless of how we > do the dereference (the implicit lvalue-to-rvalue cast), or *whether* we do > it at all

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-09-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D88477#2303376 , @steakhal wrote: > In the Summary's example, at location `#2` the value of `**p` is > `{SymRegion{reg_$0},0 S64b,unsigned char}` - which is > exactly what we stored at line `#1`. > > void test(int *a, char

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-09-30 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. In D88477#2301641 , @NoQ wrote: > A load from a region of type `T` should always yield a value of type `T` > because that's what a load is. > > I'd rather have it as an assertion: if the region is typed, the type > shouldn't be

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-09-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. A load from a region of type `T` should always yield a value of type `T` because that's what a load is. I'd rather have it as an assertion: if the region is typed, the type shouldn't be specified. If such assertion is hard to satisfy, we could allow the same canonical

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-09-29 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. In D88477#2300687 , @martong wrote: >> In this example, it cast to the `unsigned char` (which is the type of the >> stored value of `**b`, stored at `#1`) instead of the static type of the >> access (the type of `**b` is

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-09-29 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. > In this example, it cast to the `unsigned char` (which is the type of the > stored value of `**b`, stored at `#1`) instead of the static type of the > access (the type of `**b` is `char`at `#2`). Possible typo in the summary. At `#2` the type should be `char *`

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

2020-09-29 Thread Balázs Benics via Phabricator via cfe-commits
steakhal created this revision. steakhal added reviewers: NoQ, vsavchenko, baloghadamsoftware, xazax.hun. Herald added subscribers: cfe-commits, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity. Herald added a reviewer: Szelethus. Herald