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

[Tom Honermann via cfe-commits]

Author: Tom Honermann
Date: 2022-10-10T15:45:02-04:00
New Revision: 0982db188b661d6744b06244fda64d43dd80206e

URL: https://github.com/llvm/llvm-project/commit/0982db188b661d6744b06244fda64d43dd80206e
DIFF: https://github.com/llvm/llvm-project/commit/0982db188b661d6744b06244fda64d43dd80206e.diff

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

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

Reviewed By: erichkeane, aaron.ballman

Differential Revision: https://reviews.llvm.org/D135500

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Basic/DiagnosticCommonKinds.td
    clang/lib/Sema/SemaStmtAsm.cpp
    clang/test/Parser/ms-inline-asm.c

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 29550f5376a7f..9803243e61967 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -140,6 +140,25 @@ code bases.
       *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

diff  --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 39dee7e683ff9..e0408e9abd2e7 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -279,6 +279,9 @@ let CategoryName = "Inline Assembly Issue" in {
   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">>;

diff  --git a/clang/lib/Sema/SemaStmtAsm.cpp b/clang/lib/Sema/SemaStmtAsm.cpp
index 715b5e314d90b..d19f4d6c78356 100644
--- a/clang/lib/Sema/SemaStmtAsm.cpp
+++ b/clang/lib/Sema/SemaStmtAsm.cpp
@@ -932,13 +932,24 @@ StmtResult Sema::ActOnMSAsmStmt(SourceLocation AsmLoc, SourceLocation LBraceLoc,
   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,

diff  --git a/clang/test/Parser/ms-inline-asm.c b/clang/test/Parser/ms-inline-asm.c
index 44f84a542f3fb..b5cb5ba4ee5c4 100644
--- a/clang/test/Parser/ms-inline-asm.c
+++ b/clang/test/Parser/ms-inline-asm.c
@@ -62,6 +62,18 @@ void t14(void) {
   __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}}