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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8845-8846
 let CategoryName = "Inline Assembly Issue" in {
+  def err_asm_pmf_in_input
+    : Error<"cannot pass pointer-to-member through a register on this ABI">;
   def err_asm_invalid_lvalue_in_output : Error<"invalid lvalue in asm output">;
----------------
No idea why Phab won't let me suggest this as an edit, but:
```
  def err_asm_pmf_in_input : Error<
    "cannot pass pointer-to-member through a register on this ABI">;
```


================
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)
----------------
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!


================
Comment at: clang/test/Sema/gnu-asm-pmf.cpp:1-34
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -std=c++2b -fsyntax-only -verify %s -DMICROSOFT_ABI
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-itanium -std=c++2b -fsyntax-only -verify %s -DITANIUM_ABI
+
+#if defined(MICROSOFT_ABI)
+// expected-no-diagnostics
+#endif
+
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138514