[PATCH] D96976: [analyzer] Fix reinterpret_cast handling for pointer-to-member

[Balázs Benics via Phabricator via cfe-commits]

steakhal requested changes to this revision.
steakhal added a comment.
This revision now requires changes to proceed.

I have serious concerns inline.



================
Comment at: clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp:20
   int DoubleDerived::*ddf = &Base::field;
   int Base::*bf = reinterpret_cast<int Base::*>(reinterpret_cast<int Derived::*>(reinterpret_cast<int Base::*>(ddf)));
   Base base;
----------------
Please have a note describing why you are doing this roundtrip.


================
Comment at: clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp:29-31
+  Some some;
+  some.*sf = 14;
+  clang_analyzer_eval(some.*sf == 14); // expected-warning{{UNKNOWN}}
----------------
The assignment is actually UB.
TBH I don't know how to test such behavior xD
Same for the next example.

Shouldn't it return `undef` for reading via an invalid member pointer?


================
Comment at: clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp:43-50
+struct A {};
+struct B : public A {};
+struct C {
+  int field;
+};
+struct D : public C {};
+struct E : public B, public D {};
----------------
An ASCII art would help so much:
```
A   C(field)
|   |
B   D
 \ /
  E
  |
  F
```
However, I'm still missing a diamond-shaped inheritance.


================
Comment at: clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp:52-62
+void testMultiple() {
+  int F::*f = &F::field;
+  int A::*a = reinterpret_cast<int A::*>(f);
+  int C::*c = reinterpret_cast<int C::*>(f);
+  A aobj;
+  C cobj;
+  aobj.*a = 13;
----------------
Wait a minute. It's not how it works.
How I imagine member pointers, they are just offsets.
`&F::field` is notionally equivalent with `offsetof(F, field)`. That being said, You can not apply this member pointer to any object besides `F`.
Imagine if the classes of the inheritance tree would have other fields as well.
Then the `offsetof(T, field)` would be different for `F`, and `C`.

This example demonstrates that both of these member pointer dereferences are UB.
https://godbolt.org/z/15sMEP
It returns different values depending on the optimization level, which is a clear sign of UB.
BTW this issue is closely related to strict aliasing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96976/new/

https://reviews.llvm.org/D96976