tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
tahonermann requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135500

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/Parser/ms-inline-asm.c


Index: clang/test/Parser/ms-inline-asm.c
===================================================================
--- clang/test/Parser/ms-inline-asm.c
+++ clang/test/Parser/ms-inline-asm.c
@@ -62,6 +62,18 @@
   __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 {{bit-fields are not supported as 
operands in inline asm blocks}}
+    mov s.bf2, eax // expected-error {{bit-fields are not supported as 
operands in inline asm blocks}}
+  }
+}
+
 int t_fail(void) { // expected-note {{to match this}}
   __asm 
   __asm { // expected-error 3 {{expected}} expected-note {{to match this}}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===================================================================
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -932,13 +932,23 @@
   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)
+    if (Exprs[I]->getType()->isBitIntType()) {
+      InvalidOperand = true;
+      Diag(Exprs[I]->getBeginLoc(), diag::err_asm_invalid_type)
           << Exprs[I]->getType() << (I < NumOutputs)
-          << Exprs[I]->getSourceRange());
+          << Exprs[I]->getSourceRange();
+    } else if (Exprs[I]->refersToBitField()) {
+      InvalidOperand = true;
+      FieldDecl *BitField = Exprs[I]->getSourceBitField();
+      Diag(Exprs[I]->getBeginLoc(), diag::err_ms_asm_bitfield_unsupported)
+          << Exprs[I]->getSourceRange();
+      Diag(BitField->getLocation(), diag::note_bitfield_decl);
+    }
   }
+  if (InvalidOperand)
+    return StmtError();
 
   MSAsmStmt *NS =
     new (Context) MSAsmStmt(Context, AsmLoc, LBraceLoc, IsSimple,
Index: clang/include/clang/Basic/DiagnosticCommonKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -279,6 +279,9 @@
   def err_asm_invalid_type : Error<
     "invalid type %0 in asm %select{input|output}1">;
 
+  def err_ms_asm_bitfield_unsupported : Error<
+    "bit-fields are not supported as operands in inline asm blocks">;
+
   def warn_stack_clash_protection_inline_asm : Warning<
     "Unable to protect inline asm that clobbers stack pointer against stack 
clash">,
     InGroup<DiagGroup<"stack-protector">>;


Index: clang/test/Parser/ms-inline-asm.c
===================================================================
--- clang/test/Parser/ms-inline-asm.c
+++ clang/test/Parser/ms-inline-asm.c
@@ -62,6 +62,18 @@
   __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 {{bit-fields are not supported as operands in inline asm blocks}}
+    mov s.bf2, eax // expected-error {{bit-fields are not supported as operands in inline asm blocks}}
+  }
+}
+
 int t_fail(void) { // expected-note {{to match this}}
   __asm 
   __asm { // expected-error 3 {{expected}} expected-note {{to match this}}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===================================================================
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -932,13 +932,23 @@
   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)
+    if (Exprs[I]->getType()->isBitIntType()) {
+      InvalidOperand = true;
+      Diag(Exprs[I]->getBeginLoc(), diag::err_asm_invalid_type)
           << Exprs[I]->getType() << (I < NumOutputs)
-          << Exprs[I]->getSourceRange());
+          << Exprs[I]->getSourceRange();
+    } else if (Exprs[I]->refersToBitField()) {
+      InvalidOperand = true;
+      FieldDecl *BitField = Exprs[I]->getSourceBitField();
+      Diag(Exprs[I]->getBeginLoc(), diag::err_ms_asm_bitfield_unsupported)
+          << Exprs[I]->getSourceRange();
+      Diag(BitField->getLocation(), diag::note_bitfield_decl);
+    }
   }
+  if (InvalidOperand)
+    return StmtError();
 
   MSAsmStmt *NS =
     new (Context) MSAsmStmt(Context, AsmLoc, LBraceLoc, IsSimple,
Index: clang/include/clang/Basic/DiagnosticCommonKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -279,6 +279,9 @@
   def err_asm_invalid_type : Error<
     "invalid type %0 in asm %select{input|output}1">;
 
+  def err_ms_asm_bitfield_unsupported : Error<
+    "bit-fields are not supported as operands in inline asm blocks">;
+
   def warn_stack_clash_protection_inline_asm : Warning<
     "Unable to protect inline asm that clobbers stack pointer against stack clash">,
     InGroup<DiagGroup<"stack-protector">>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to