Fix for PR14269: Clang crashes when a bit field is used as inline assembler input / output with memory constraint

[andreybokhanko]

Summary

Fix for PR14269 (https://llvm.org/bugs/show_bug.cgi?id=14269) -- clang simply crashes when a bit field is used as inline assembler input / output with memory constraint.

One generally can't get address of a bit field, so the general solution is to error on such cases. GCC does the same.

[andreybokhanko]

Ping

[mcrosier]

I think it's acceptable to reference a PR# in the test case, but in general we don't include these in the source code unless there's an associated FIXME. Please remove the PR#s from SemaStmtAsm.cpp. Otherwise, seems reasonable to me.

[andreybokhanko]

Chad, thanks for the review. I removed mentioning of PRs; patch updated.

[echristo]

One inline comment and then LGTM.

Thanks!

-eric

lib/Sema/SemaStmtAsm.cpp

241

"Bit field" -> "Bitfield" please.

[andreybokhanko]

Eric, thanks!

Patch updated: "bit field" changed to "bitfield" in comments; "bit field" changed to "bit-field" in messages, as "bit-field" seems to be the prevailing spelling in other messages.

I don't have commit access; Alexander [Musman], could you, please, commit the patch?

Andrey

[rsmith]

cfe/trunk/lib/Sema/SemaStmtAsm.cpp

158 ↗

(On Diff #27199)

What about other weird kinds of lvalues, like the result of __real / __imag, vector indexing, and global register variables? Those have the same problem; CGStmt.cpp blindly calls LValue::getAddress without checking for those cases.

Testcase:

typedef __attribute__((vector_size(16))) int vi4;
void test(vi4 v) {
  __asm__("" : "=rm"(v[2]));
}

[andreybokhanko]

Richard, I will take a look.

Yours,
Andrey

include/clang/Basic/DiagnosticSemaKinds.td

def err_asm_invalid_input_size : Error<
     "invalid input size for constraint '%0'">;
   def err_asm_invalid_output_size : Error<
     "invalid output size for constraint '%0'">;
   def err_invalid_asm_cast_lvalue : Error<
     "invalid use of a cast in a inline asm context requiring an l-value: "
     "remove the cast or build with -fheinous-gnu-extensions">;
   def err_invalid_asm_value_for_constraint
       : Error <"value '%0' out of range for constraint '%1'">;
+  def err_asm_bitfield_in_memory_constraint
+      : Error <"reference to a bit field in asm "
+      "%select{input|output}0 with a memory constraint '%1'">;
 
   def warn_asm_label_on_auto_decl : Warning<
     "ignored asm label '%0' on automatic variable">;
   def warn_invalid_asm_cast_lvalue : Warning<
     "invalid use of a cast in an inline asm context requiring an l-value: "
     "accepted due to -fheinous-gnu-extensions, but clang may remove support "
     "for this in the future">;
   def warn_asm_mismatched_size_modifier : Warning<

lib/Sema/SemaStmtAsm.cpp

for (unsigned i = 0; i != NumOutputs; i++) {
 
     // Check that the output exprs are valid lvalues.
     Expr *OutputExpr = Exprs[i];
 
     // Referring to parameters is not allowed in naked functions.
     if (CheckNakedParmReference(OutputExpr, *this))
       return StmtError();
 
+    // Bit field can't be referenced with a pointer.
+    if (Info.allowsMemory() && OutputExpr->refersToBitField())
+      return StmtError(Diag(OutputExpr->getLocStart(),
+                            diag::err_asm_bitfield_in_memory_constraint)
+                       << 1
+                       << Info.getConstraintStr()
+                       << OutputExpr->getSourceRange());
+
     OutputConstraintInfos.push_back(Info);
 
     // If this is dependent, just continue.
     if (OutputExpr->isTypeDependent())
       continue;
 
     Expr::isModifiableLvalueResult IsLV =
         OutputExpr->isModifiableLvalue(Context, /*Loc=*/nullptr);
for (unsigned i = NumOutputs, e = NumOutputs + NumInputs; i != e; i++) {
     Exprs[i] = ER.get();
 
     Expr *InputExpr = Exprs[i];
 
     // Referring to parameters is not allowed in naked functions.
     if (CheckNakedParmReference(InputExpr, *this))
       return StmtError();
 
+    // Bit field can't be referenced with a pointer.
+    if (Info.allowsMemory() && InputExpr->refersToBitField())
+      return StmtError(Diag(InputExpr->getLocStart(),
+                            diag::err_asm_bitfield_in_memory_constraint)
+                       << 0
+                       << Info.getConstraintStr()
+                       << InputExpr->getSourceRange());
+
     // Only allow void types for memory constraints.
     if (Info.allowsMemory() && !Info.allowsRegister()) {
       if (CheckAsmLValue(InputExpr, *this))
         return StmtError(Diag(InputExpr->getLocStart(),
                               diag::err_asm_invalid_lvalue_in_input)
                          << Info.getConstraintStr()
                          << InputExpr->getSourceRange());
     } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {

test/Sema/asm.c

 }
 
 void fn6() {
     int a;
   __asm__(""
             : "=rm"(a), "=rm"(a)
             : "11m"(a)) // expected-error {{invalid input constraint '11m' in asm}}
 }
+
+// PR14269
+typedef struct test16_foo {
+  unsigned int field1 : 1;
+  unsigned int field2 : 2;
+  unsigned int field3 : 3;
+} test16_foo;
+test16_foo x;
+void test16()
+{
+  __asm__("movl $5, %0"
+          : "=rm" (x.field2)); // expected-error {{reference to a bit field in asm output with a memory constraint '=rm'}}
+  __asm__("movl $5, %0"
+          :
+          : "m" (x.field3)); // expected-error {{reference to a bit field in asm input with a memory constraint 'm'}}
+}
+