[PATCH] D138514: Sema: diagnose PMFs passed through registers to inline assembly

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.


================
Comment at: clang/lib/Sema/SemaStmtAsm.cpp:381
+    if (!Context.getTargetInfo().getCXXABI().isMicrosoft())
+      if (const auto *UO = dyn_cast<UnaryOperator>(InputExpr))
+        if (UO->getOpcode() == UO_AddrOf)
----------------
aaron.ballman wrote:
> rnk wrote:
> > This is too narrow, there are lots of other ways to do this:
> > ```
> > struct Foo { void method(); };
> > void f() {
> >   auto pmf = &Foo::method;
> >   asm volatile ("" : : "r"(pmf));
> > }
> > ```
> > 
> > I think it makes sense to check for:
> > * An expression with a member pointer type
> > * Where the size of the type is larger than the size of a pointer, or word, or whatever proxy we normally use for the size of a general purpose register
> > 
> > In the Microsoft ABI, member function pointers are only sometimes pointer-sized. If the class uses the multiple inheritance model, it will be bigger and include the this-adjuster field. See the inheritance keyword docs to learn more:
> > https://learn.microsoft.com/en-us/cpp/cpp/inheritance-keywords?view=msvc-170
> > 
> > This also handles large pointers to data members in the MS ABI, which also has a wacky aggregate representation.
> That sounds like a less fragile approach to me, thanks for that suggestion!
Thanks @rnk for pointing out the oversight on handling a PMF through a VarDecl and the pointer to the MSDN docs on how to form the adjusted reference.  This actually is more interesting.  Consider now the following:

```
struct s {
    __UINTPTR_TYPE__ i, j;
};

void f() {
    __asm__ __volatile__("" : : "r"(s{0,0}));
}

struct u { virtual void f(); };
struct v { virtual void operator()(); };
struct w: u, v {
};

void g() {
    __asm__ __volatile__("" : : "r"(&w::operator()));
}
```

GCC does accept the input, but clang fails due to the backend failing to form the indirect passing for the aggregate (`Don't know how to handle indirect register inputs yet for constraint 'r'`).  Interestingly enough, when targeting `x86_64-unknown-windows-msvc` instead of `x86_64-unknown-linux-gnu`, we represent the PMF as an aggregate and trigger the same type of aggregate passing failure.  On Linux though we attempt to lower the aggregate passing and then fail to select the copy due to the aggregate being lowered into a single register rather than being passed as indirect.  One of the issues with the indirect passing is that GCC will also splat the "indirect" value but not give you the register that the remaining structure is splatted into.  So ultimately, I think that filtering out any attempt to pass the PMF here is something that we can/should diagnose.  But should we do that for any aggregate type is questionable, and there is still the question of how you identify that the representation that the PMF will be lowered to is an aggregate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138514