RedDocMD added inline comments.
================ 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; ---------------- steakhal wrote: > RedDocMD wrote: > > steakhal wrote: > > > RedDocMD wrote: > > > > steakhal wrote: > > > > > 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. > > > > The member access on A is definitely UB, I guess I will do what I > > > > proposed in the `Some` case. > > > > I don't think the other one is. Consider the following: > > > > ``` > > > > struct A {}; > > > > struct B : public A {}; > > > > struct C { > > > > int field; > > > > }; > > > > struct D : public C {}; > > > > struct E : public B, public D {}; > > > > struct F : public E {}; > > > > > > > > int main() { > > > > int F::* ff = &F::field; > > > > int C::* cf1 = static_cast<int C::*>(ff); > > > > int C::* cf2 = reinterpret_cast<int C::*>(ff); > > > > C c; > > > > c.*cf1 = 10; > > > > c.*cf2 = 10; > > > > return 0; > > > > } > > > > ``` > > > > `cf1` and `cf2` are the same thing, except that they are declared > > > > differently (one via `static_cast`, other via `reinterpret_cast`). If > > > > we look at the AST (truncated to the first three lines of main): > > > > ``` > > > > CompoundStmt 0x1a4fe18 <col:12, line:18:1> > > > > |-DeclStmt 0x1a4f3a8 <line:11:3, col:26> > > > > | `-VarDecl 0x1a21078 <col:3, col:21> col:12 used ff 'int F::*' > > > > cinit > > > > | `-ImplicitCastExpr 0x1a4f378 <col:17, col:21> 'int F::*' > > > > <BaseToDerivedMemberPointer (E -> D -> C)> > > > > | `-UnaryOperator 0x1a4f360 <col:17, col:21> 'int C::*' prefix > > > > '&' cannot overflow > > > > | `-DeclRefExpr 0x1a4f2f8 <col:18, col:21> 'int' lvalue Field > > > > 0x1a207d0 'field' 'int' > > > > |-DeclStmt 0x1a4f560 <line:12:3, col:43> > > > > | `-VarDecl 0x1a4f428 <col:3, col:42> col:12 used cf1 'int C::*' > > > > cinit > > > > | `-CXXStaticCastExpr 0x1a4f518 <col:18, col:42> 'int C::*' > > > > static_cast<int struct C::*> <DerivedToBaseMemberPointer (E -> D -> C)> > > > > | `-ImplicitCastExpr 0x1a4f500 <col:40> 'int F::*' > > > > <LValueToRValue> part_of_explicit_cast > > > > | `-DeclRefExpr 0x1a4f498 <col:40> 'int F::*' lvalue Var > > > > 0x1a21078 'ff' 'int F::*' > > > > |-DeclStmt 0x1a4f6e8 <line:13:3, col:48> > > > > | `-VarDecl 0x1a4f5c8 <col:3, col:47> col:12 used cf2 'int C::*' > > > > cinit > > > > | `-CXXReinterpretCastExpr 0x1a4f6b8 <col:18, col:47> 'int C::*' > > > > reinterpret_cast<int struct C::*> <ReinterpretMemberPointer> > > > > | `-ImplicitCastExpr 0x1a4f6a0 <col:45> 'int F::*' > > > > <LValueToRValue> part_of_explicit_cast > > > > | `-DeclRefExpr 0x1a4f638 <col:45> 'int F::*' lvalue Var > > > > 0x1a21078 'ff' 'int F::*' > > > > ``` > > > > Notice how the `static_cast` figures out the path to the correct > > > > subobject. This is how member-pointers are handled as far as I can tell. > > > > Unfortunately, Stroustrup is surprisingly scant when it comes to this > > > > topic. I am trying to dig through the Standard. > > > I tried to highlight that the classes could have non-static data members > > > which could cause your assumption about `cf1 == cf2` not to hold anymore. > > > See my previously attached godbolt link. > > > > > > `static_cast<T>(p)` makes sure that the //offset// that the member > > > pointer `p` represents is updated accordingly to match the //offset// of > > > the `field` member within the new type `T`. Usually, it involves some > > > addition/subtraction to accommodate this. > > > While `static_cast<T>(p)` does not update the value, just reinterprets it > > > in a different way - which usually results in an invalid pointer though > > > and a bunch of subtle rules kick in such as the type similarity, pointer > > > interchangeability, which in general known as //strict-aliasing//. > > > While static_cast<T>(p) does not update the value, just reinterprets it > > > in a different way - which usually results in an invalid pointer though > > > and a bunch of subtle rules kick in such as the type similarity, pointer > > > interchangeability, which in general known as strict-aliasing. > > Presumably you mean `reinterpret_cast`. > > > > Ah, I see. This makes it glaringly obvious: [[ https://godbolt.org/z/f6vs77 > > | Another Godbolt link ]] > > So I guess handling `reinterpret_cast` for pointer-to-members pointless. I > > guess the idea will be to simply not analyze pointer-to-members which have > > been obtained through `reinterpret_cast.` > > Presumably you mean reinterpret_cast. > Yes, sorry for copy-pasting too much xD > > > So I guess handling reinterpret_cast for pointer-to-members pointless. > I don't know if it's really pointless, but hard to model precisely for sure. > > > I guess the idea will be to simply not analyze pointer-to-members which > > have been obtained through `reinterpret_cast`. > I guess, you could just return unknown? I don't know. This area is not my > expertise, I just wanted to get you going with my observations. > I guess, you could just return unknown? If I return `nullptr` as `PointerToMemberData`, then the analyzer will mark the pointer-to-member as unknown [this is done by `handleLVectorSplat`] >I don't know if it's really pointless, but hard to model precisely for sure. Really hard to model and has too many cases and ifs and buts. Like although the member-pointer obtained by `reinterpret_cast` probably has the wrong offset, we still don't get seg-fault. Curious, but seems to happen for both GCC and Clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96976/new/ https://reviews.llvm.org/D96976 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits