[Clang] reject bit-fields as instruction operands in Microsoft style inline asm blocks.

[tahonermann]

Summary

MSVC allows bit-fields to be specified as instruction operands in inline asm blocks. Such references are resolved to the address of the allocation unit that the named bitfield is a part of. The result is that reads and writes of such operands will read or mutate nearby bit-fields stored in the same allocation unit. This is a surprising behavior for which MSVC issues warning C4401, "'<identifier>': member is bit field". Intel's icc compiler also allows such bit-field references, but does not issue a diagnostic.

Prior to this change, Clang fails the following assertion when a bit-field is referenced in such instructions:

clang/lib/CodeGen/CGValue.h:338: llvm::Value* clang::CodeGen::LValue::getPointer(clang::CodeGen::CodeGenFunction&) const: Assertion `isSimple()' failed.

In non-assert enabled builds, Clang's behavior appears to match the behavior of the MSVC and icc compilers, though it is unknown if that is by design or happenstance.

Following this change, attempts to use a bit-field as an instruction operand in Microsoft style asm blocks is diagnosed as an error due to the risk of unintentionally reading or writing nearby bit-fields.

Fixes https://github.com/llvm/llvm-project/issues/57791

[erichkeane]

1 nit, otherwise LGTM.

clang/lib/Sema/SemaStmtAsm.cpp

936–937

There is enough repetition of Exprs[I] I suspect there is value to splitting them up as:

Expr *CurExpr = Exprs[I];

for readability purposes.

[aaron.ballman]

LGTM as well with a diagnostic wording suggestion; please also add a release note for the fix.

clang/include/clang/Basic/DiagnosticCommonKinds.td

283

(Not strongly tied to the new wording, but we try to write diagnostics in singular form when possible and that led me to this rewording.)

[tahonermann]

Rebased and addressed code review feedback.

[tahonermann]

clang/lib/Sema/SemaStmtAsm.cpp

936–937

Thank you, this was a good suggestion!

clang/docs/ReleaseNotes.rst

- Clang now diagnoses indirection of ``void *`` in C++ mode as a warning which
   an error-only diagnostic in the next Clang release.
 
   .. code-block:: c++
 
     void func(void *p) {
       *p; // Now diagnosed as a warning-as-error.
     }
 
+- Clang now diagnoses use of a bit-field as an instruction operand in Microsoft
+  style inline asm blocks as an error. Previously, a bit-field operand yielded
+  the address of the allocation unit the bit-field was stored within; reads or
+  writes therefore had the potential to read or write nearby bit-fields. This
+  change fixes `issue 57791 <https://github.com/llvm/llvm-project/issues/57791>`_.
+
+  .. code-block:: c++
+
+    typedef struct S {
+      unsigned bf:1;
+    } S;
+    void f(S s) {
+      __asm {
+        mov eax, s.bf // Now diagnosed as an error.
+        mov s.bf, eax // Now diagnosed as an error.
+      }
+    }
+
+
 What's New in Clang |release|?
 ==============================
 Some of the major new features and improvements to Clang are listed
 here. Generic improvements to Clang as a whole or to its underlying
 infrastructure are described first, followed by language-specific
 sections with improvements to Clang's support for those languages.
 
 Major New Features

clang/include/clang/Basic/DiagnosticCommonKinds.td

 // Sema && Frontend
 let CategoryName = "Inline Assembly Issue" in {
   def err_asm_invalid_type_in_input : Error<
     "invalid type %0 in asm input for constraint '%1'">;
 
   def err_asm_invalid_type : Error<
     "invalid type %0 in asm %select{input|output}1">;
 
+  def err_ms_asm_bitfield_unsupported : Error<
+    "an inline asm block cannot have an operand which is a bit-field">;
+
   def warn_stack_clash_protection_inline_asm : Warning<
     "Unable to protect inline asm that clobbers stack pointer against stack clash">,
     InGroup<DiagGroup<"stack-protector">>;
 
   def warn_slh_does_not_support_asm_goto
       : Warning<"Speculative load hardening does not protect functions with "
                 "asm goto">,
         InGroup<DiagGroup<"slh-asm-goto">>;

clang/lib/Sema/SemaStmtAsm.cpp

StmtResult Sema::ActOnMSAsmStmt(SourceLocation AsmLoc, SourceLocation LBraceLoc,
                                 unsigned NumOutputs, unsigned NumInputs,
                                 ArrayRef<StringRef> Constraints,
                                 ArrayRef<StringRef> Clobbers,
                                 ArrayRef<Expr*> Exprs,
                                 SourceLocation EndLoc) {
   bool IsSimple = (NumOutputs != 0 || NumInputs != 0);
   setFunctionHasBranchProtectedScope();
 
+  bool InvalidOperand = false;
   for (uint64_t I = 0; I < NumOutputs + NumInputs; ++I) {
-    if (Exprs[I]->getType()->isBitIntType())
-      return StmtError(
-          Diag(Exprs[I]->getBeginLoc(), diag::err_asm_invalid_type)
-          << Exprs[I]->getType() << (I < NumOutputs)
-          << Exprs[I]->getSourceRange());
+    Expr *E = Exprs[I];
+    if (E->getType()->isBitIntType()) {
+      InvalidOperand = true;
+      Diag(E->getBeginLoc(), diag::err_asm_invalid_type)
+          << E->getType() << (I < NumOutputs)
+          << E->getSourceRange();
+    } else if (E->refersToBitField()) {
+      InvalidOperand = true;
+      FieldDecl *BitField = E->getSourceBitField();
+      Diag(E->getBeginLoc(), diag::err_ms_asm_bitfield_unsupported)
+          << E->getSourceRange();
+      Diag(BitField->getLocation(), diag::note_bitfield_decl);
     }
+  }
+  if (InvalidOperand)
+    return StmtError();
 
   MSAsmStmt *NS =
     new (Context) MSAsmStmt(Context, AsmLoc, LBraceLoc, IsSimple,
                             /*IsVolatile*/ true, AsmToks, NumOutputs, NumInputs,
                             Constraints, Exprs, AsmString,
                             Clobbers, EndLoc);
   return NS;
 }

clang/test/Parser/ms-inline-asm.c

void t13(void) {
   __asm m{o}v eax, ebx // expected-error {{unknown token in expression}}
 }
 
 void t14(void) {
   enum { A = 1, B };
   __asm mov eax, offset A // expected-error {{offset operator cannot yet handle constants}}
 }
 
+// GH57791
+typedef struct S {
+  unsigned bf1:1; // expected-note {{bit-field is declared here}}
+  unsigned bf2:1; // expected-note {{bit-field is declared here}}
+} S;
+void t15(S s) {
+  __asm {
+    mov eax, s.bf1 // expected-error {{an inline asm block cannot have an operand which is a bit-field}}
+    mov s.bf2, eax // expected-error {{an inline asm block cannot have an operand which is a bit-field}}
+  }
+}
+
 int t_fail(void) { // expected-note {{to match this}}
   __asm 
   __asm { // expected-error 3 {{expected}} expected-note {{to match this}}