pengfei created this revision.
pengfei added reviewers: jyu2, epastor, ABataev, kbsmith1, LuoYuanke.
pengfei requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Clang will crash if we tie a structure with a small size type. This
patch adds check for this case to show the error in user-friendly way.
Note, both GCC and ICC support a more loose constraint that Clang.
https://godbolt.org/z/5ex9x3dvv
I don't find any document about GCC and ICC's behavior and the generated
code doesn't make much sense to me. Besides, if we change "=rm" to "=r"
or "=m", GCC and ICC will report error differently. Which gives me an
impression that the use of constraint is undefined behavior.
So I'd like to simply report error in Clang at present.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D107141
Files:
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Sema/SemaStmtAsm.cpp
clang/test/Sema/asm.c
Index: clang/test/Sema/asm.c
===================================================================
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -313,3 +313,23 @@
asm ("jne %l0":::);
asm goto ("jne %l0"::::lab);
}
+
+// FIXME: GCC and ICC support this case. We may need to follow them.
+typedef struct test19_foo {
+ int a;
+ char b;
+} test19_foo;
+
+typedef struct test19_bar {
+ int a;
+ int b;
+ int c;
+} test19_bar;
+
+void test19()
+{
+ test19_foo a;
+ test19_bar b;
+ asm ("" : "=rm" (a): "0" (1)); // expected-error {{unsupported inline asm:
input with type 'int' matching output with type 'test19_foo' (aka 'struct
test19_foo')}}
+ asm ("" : "=rm" (b): "0" (1)); // expected-error {{unsupported inline asm:
input with type 'int' matching output with type 'test19_bar' (aka 'struct
test19_bar')}}
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===================================================================
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -666,7 +666,8 @@
// output was a register, just extend the shorter one to the size of the
// larger one.
if (!SmallerValueMentioned && InputDomain != AD_Other &&
- OutputConstraintInfos[TiedTo].allowsRegister())
+ OutputConstraintInfos[TiedTo].allowsRegister() &&
+ (OutputDomain != AD_Other || OutSize <= InSize))
continue;
// Either both of the operands were mentioned or the smaller one was
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2467,7 +2467,8 @@
// that is usually cheaper, but LLVM IR should really get an anyext
someday.
if (Info.hasTiedOperand()) {
unsigned Output = Info.getTiedOperand();
- QualType OutputType = S.getOutputExpr(Output)->getType();
+ const Expr *OutputExpr = S.getOutputExpr(Output);
+ QualType OutputType = OutputExpr->getType();
QualType InputTy = InputExpr->getType();
if (getContext().getTypeSize(OutputType) >
@@ -2480,10 +2481,14 @@
Arg = Builder.CreateZExt(Arg, OutputTy);
else if (isa<llvm::PointerType>(OutputTy))
Arg = Builder.CreateZExt(Arg, IntPtrTy);
- else {
- assert(OutputTy->isFloatingPointTy() && "Unexpected output type");
+ else if (OutputTy->isFloatingPointTy())
Arg = Builder.CreateFPExt(Arg, OutputTy);
- }
+ else
+ // FIXME: GCC and ICC can handle structure type that have more than 1
+ // element. We may need to match the same behavior.
+ CGM.getDiags().Report(S.getAsmLoc(), diag::err_asm_invalid_type)
+ << OutputExpr->getType() << 1 /*Output*/
+ << OutputExpr->getSourceRange();
}
// Deal with the tied operands' constraint code in adjustInlineAsmType.
ReplaceConstraint = OutputConstraints[Output];
Index: clang/test/Sema/asm.c
===================================================================
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -313,3 +313,23 @@
asm ("jne %l0":::);
asm goto ("jne %l0"::::lab);
}
+
+// FIXME: GCC and ICC support this case. We may need to follow them.
+typedef struct test19_foo {
+ int a;
+ char b;
+} test19_foo;
+
+typedef struct test19_bar {
+ int a;
+ int b;
+ int c;
+} test19_bar;
+
+void test19()
+{
+ test19_foo a;
+ test19_bar b;
+ asm ("" : "=rm" (a): "0" (1)); // expected-error {{unsupported inline asm: input with type 'int' matching output with type 'test19_foo' (aka 'struct test19_foo')}}
+ asm ("" : "=rm" (b): "0" (1)); // expected-error {{unsupported inline asm: input with type 'int' matching output with type 'test19_bar' (aka 'struct test19_bar')}}
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===================================================================
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -666,7 +666,8 @@
// output was a register, just extend the shorter one to the size of the
// larger one.
if (!SmallerValueMentioned && InputDomain != AD_Other &&
- OutputConstraintInfos[TiedTo].allowsRegister())
+ OutputConstraintInfos[TiedTo].allowsRegister() &&
+ (OutputDomain != AD_Other || OutSize <= InSize))
continue;
// Either both of the operands were mentioned or the smaller one was
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2467,7 +2467,8 @@
// that is usually cheaper, but LLVM IR should really get an anyext someday.
if (Info.hasTiedOperand()) {
unsigned Output = Info.getTiedOperand();
- QualType OutputType = S.getOutputExpr(Output)->getType();
+ const Expr *OutputExpr = S.getOutputExpr(Output);
+ QualType OutputType = OutputExpr->getType();
QualType InputTy = InputExpr->getType();
if (getContext().getTypeSize(OutputType) >
@@ -2480,10 +2481,14 @@
Arg = Builder.CreateZExt(Arg, OutputTy);
else if (isa<llvm::PointerType>(OutputTy))
Arg = Builder.CreateZExt(Arg, IntPtrTy);
- else {
- assert(OutputTy->isFloatingPointTy() && "Unexpected output type");
+ else if (OutputTy->isFloatingPointTy())
Arg = Builder.CreateFPExt(Arg, OutputTy);
- }
+ else
+ // FIXME: GCC and ICC can handle structure type that have more than 1
+ // element. We may need to match the same behavior.
+ CGM.getDiags().Report(S.getAsmLoc(), diag::err_asm_invalid_type)
+ << OutputExpr->getType() << 1 /*Output*/
+ << OutputExpr->getSourceRange();
}
// Deal with the tied operands' constraint code in adjustInlineAsmType.
ReplaceConstraint = OutputConstraints[Output];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
