[CodeGen] Fix crash when a function taking transparent union is redeclared.

When a function taking transparent union is declared as taking one of
union members earlier in the translation unit, clang would hit an
"Invalid cast" assertion during EmitFunctionProlog. This case
corresponds to function f1 in test/CodeGen/transparent-union-redecl.c.
We decided to cast i32 to union because after merging function
declarations function parameter type becomes int,
CGFunctionInfo::ArgInfo type matches with ABIArgInfo type, so we decide
it is a trivial case. But these types should also be castable to
parameter declaration type which is not the case here.

The fix is in checking for the trivial case if ABIArgInfo type matches with
parameter declaration type. It exposed inconsistency that we check
hasScalarEvaluationKind for different types in EmitParmDecl and
EmitFunctionProlog, and comment says they should match.

Additional tests in Sema/transparent-union.c capture current behavior and make
sure there are no regressions.

rdar://problem/34949329

Reviewers: rjmccall, rafael

Reviewed By: rjmccall

Subscribers: aemerson, cfe-commits, kristof.beyls

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

llvm-svn: 321296

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 38d7344..8d2fc5d 100644 (file)
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2317,7 +2317,7 @@ void CodeGenFunction::EmitFunctionProlog(const CGFunctionInfo &FI,
 
       // If we have the trivial case, handle it with no muss and fuss.
       if (!isa<llvm::StructType>(ArgI.getCoerceToType()) &&
-          ArgI.getCoerceToType() == ConvertType(Ty) &&
+          ArgI.getCoerceToType() == ConvertType(Arg->getType()) &&
           ArgI.getDirectOffset() == 0) {
         assert(NumIRArgs == 1);
         llvm::Value *V = FnArgs[FirstIRArg];
@@ -2414,8 +2414,8 @@ void CodeGenFunction::EmitFunctionProlog(const CGFunctionInfo &FI,
         break;
       }
 
-      Address Alloca = CreateMemTemp(Ty, getContext().getDeclAlign(Arg),
-                                     Arg->getName());
+      Address Alloca = CreateMemTemp(
+          Arg->getType(), getContext().getDeclAlign(Arg), Arg->getName());
 
       // Pointer to store into.
       Address Ptr = emitAddressAtOffset(*this, Alloca, ArgI);
@@ -2461,9 +2461,9 @@ void CodeGenFunction::EmitFunctionProlog(const CGFunctionInfo &FI,
       }
 
       // Match to what EmitParmDecl is expecting for this type.
-      if (CodeGenFunction::hasScalarEvaluationKind(Ty)) {
+      if (CodeGenFunction::hasScalarEvaluationKind(Arg->getType())) {
         llvm::Value *V =
-          EmitLoadOfScalar(Alloca, false, Ty, Arg->getLocStart());
+            EmitLoadOfScalar(Alloca, false, Arg->getType(), Arg->getLocStart());
         if (isPromoted)
           V = emitArgumentDemotion(*this, Arg, V);
         ArgVals.push_back(ParamValue::forDirect(V));

diff --git a/clang/test/CodeGen/kr-func-promote.c b/clang/test/CodeGen/kr-func-promote.c
index 8e55dc9..615fe94 100644 (file)
--- a/clang/test/CodeGen/kr-func-promote.c
+++ b/clang/test/CodeGen/kr-func-promote.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i386-unknown-unknown %s -emit-llvm -o - | FileCheck %s
-// CHECK: i32 @a(i32)
+// CHECK: i32 @a(i32
 
 int a();
 int a(x) short x; {return x;}

diff --git a/clang/test/CodeGen/transparent-union-redecl.c b/clang/test/CodeGen/transparent-union-redecl.c
new file mode 100644 (file)
index 0000000..c0619f0
--- /dev/null
+++ b/clang/test/CodeGen/transparent-union-redecl.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -Werror -triple i386-linux -emit-llvm -o - %s | FileCheck %s
+
+// Test that different order of declarations is acceptable and that
+// implementing different redeclarations is acceptable.
+// rdar://problem/34949329
+
+typedef union {
+  int i;
+  float f;
+} TU __attribute__((transparent_union));
+
+// CHECK-LABEL: define void @f0(i32 %tu.coerce)
+// CHECK: %tu = alloca %union.TU, align 4
+// CHECK: %coerce.dive = getelementptr inbounds %union.TU, %union.TU* %tu, i32 0, i32 0
+// CHECK: store i32 %tu.coerce, i32* %coerce.dive, align 4
+void f0(TU tu) {}
+void f0(int i);
+
+// CHECK-LABEL: define void @f1(i32 %tu.coerce)
+// CHECK: %tu = alloca %union.TU, align 4
+// CHECK: %coerce.dive = getelementptr inbounds %union.TU, %union.TU* %tu, i32 0, i32 0
+// CHECK: store i32 %tu.coerce, i32* %coerce.dive, align 4
+void f1(int i);
+void f1(TU tu) {}
+
+// CHECK-LABEL: define void @f2(i32 %i)
+// CHECK: %i.addr = alloca i32, align 4
+// CHECK: store i32 %i, i32* %i.addr, align 4
+void f2(TU tu);
+void f2(int i) {}
+
+// CHECK-LABEL: define void @f3(i32 %i)
+// CHECK: %i.addr = alloca i32, align 4
+// CHECK: store i32 %i, i32* %i.addr, align 4
+void f3(int i) {}
+void f3(TU tu);

diff --git a/clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp b/clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
index 5bed69f..d51c4be 100644 (file)
--- a/clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
+++ b/clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
@@ -88,8 +88,11 @@ void ChildOverride::right() {
 // ChildOverride::right gets 'this' cast to Right* in ECX (i.e. this+4) so we
 // need to adjust 'this' before use.
 //
+// CHECK: %[[THIS_STORE:.*]] = alloca %struct.ChildOverride*, align 4
 // CHECK: %[[THIS_ADDR:.*]] = alloca %struct.ChildOverride*, align 4
-// CHECK: %[[THIS_INIT:.*]] = bitcast i8* %[[ECX:.*]] to %struct.ChildOverride*
+// CHECK: %[[COERCE_VAL:.*]] = bitcast i8* %[[ECX:.*]] to %struct.ChildOverride*
+// CHECK: store %struct.ChildOverride* %[[COERCE_VAL]], %struct.ChildOverride** %[[THIS_STORE]], align 4
+// CHECK: %[[THIS_INIT:.*]] = load %struct.ChildOverride*, %struct.ChildOverride** %[[THIS_STORE]], align 4
 // CHECK: store %struct.ChildOverride* %[[THIS_INIT]], %struct.ChildOverride** %[[THIS_ADDR]], align 4
 // CHECK: %[[THIS_RELOAD:.*]] = load %struct.ChildOverride*, %struct.ChildOverride** %[[THIS_ADDR]]
 // CHECK: %[[THIS_i8:.*]] = bitcast %struct.ChildOverride* %[[THIS_RELOAD]] to i8*
@@ -132,8 +135,11 @@ struct GrandchildOverride : ChildOverride {
 void GrandchildOverride::right() {
 // CHECK-LABEL: define x86_thiscallcc void @"\01?right@GrandchildOverride@@UAEXXZ"(i8*
 //
+// CHECK: %[[THIS_STORE:.*]] = alloca %struct.GrandchildOverride*, align 4
 // CHECK: %[[THIS_ADDR:.*]] = alloca %struct.GrandchildOverride*, align 4
-// CHECK: %[[THIS_INIT:.*]] = bitcast i8* %[[ECX:.*]] to %struct.GrandchildOverride*
+// CHECK: %[[COERCE_VAL:.*]] = bitcast i8* %[[ECX:.*]] to %struct.GrandchildOverride*
+// CHECK: store %struct.GrandchildOverride* %[[COERCE_VAL]], %struct.GrandchildOverride** %[[THIS_STORE]], align 4
+// CHECK: %[[THIS_INIT:.*]] = load %struct.GrandchildOverride*, %struct.GrandchildOverride** %[[THIS_STORE]], align 4
 // CHECK: store %struct.GrandchildOverride* %[[THIS_INIT]], %struct.GrandchildOverride** %[[THIS_ADDR]], align 4
 // CHECK: %[[THIS_RELOAD:.*]] = load %struct.GrandchildOverride*, %struct.GrandchildOverride** %[[THIS_ADDR]]
 // CHECK: %[[THIS_i8:.*]] = bitcast %struct.GrandchildOverride* %[[THIS_RELOAD]] to i8*

diff --git a/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp b/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp
index 2f141b2..377a670 100644 (file)
--- a/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp
+++ b/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp
@@ -25,6 +25,7 @@ D::D() {}  // Forces vftable emission.
 
 // CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@D@@$4PPPPPPPM@A@AEXXZ"
 // Note that the vtordisp is applied before really adjusting to D*.
+// CHECK: %[[COERCE_LOAD:.*]] = load %struct.D*, %struct.D** %{{.*}}
 // CHECK: %[[ECX:.*]] = load %struct.D*, %struct.D** %{{.*}}
 // CHECK: %[[ECX_i8:.*]] = bitcast %struct.D* %[[ECX]] to i8*
 // CHECK: %[[VTORDISP_PTR_i8:.*]] = getelementptr inbounds i8, i8* %[[ECX_i8]], i32 -4
@@ -36,6 +37,7 @@ D::D() {}  // Forces vftable emission.
 // CHECK: ret void
 
 // CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@D@@$4PPPPPPPI@3AEXXZ"
+// CHECK: %[[COERCE_LOAD:.*]] = load %struct.D*, %struct.D** %{{.*}}
 // CHECK: %[[ECX:.*]] = load %struct.D*, %struct.D** %{{.*}}
 // CHECK: %[[ECX_i8:.*]] = bitcast %struct.D* %[[ECX]] to i8*
 // CHECK: %[[VTORDISP_PTR_i8:.*]] = getelementptr inbounds i8, i8* %[[ECX_i8]], i32 -8
@@ -64,7 +66,8 @@ struct G : virtual F, virtual E {
 
 G::G() {}  // Forces vftable emission.
 
-// CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@E@@$R4BA@M@PPPPPPPM@7AEXXZ"(i8*)
+// CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@E@@$R4BA@M@PPPPPPPM@7AEXXZ"(i8*
+// CHECK: %[[COERCE_LOAD:.*]] = load %struct.E*, %struct.E** %{{.*}}
 // CHECK: %[[ECX:.*]] = load %struct.E*, %struct.E** %{{.*}}
 // CHECK: %[[ECX_i8:.*]] = bitcast %struct.E* %[[ECX]] to i8*
 // CHECK: %[[VTORDISP_PTR_i8:.*]] = getelementptr inbounds i8, i8* %[[ECX_i8]], i32 -4

diff --git a/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp b/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
index c9dd1dd..2eca906 100644 (file)
--- a/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
+++ b/clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
@@ -115,9 +115,15 @@ void B::foo() {
 // B::foo gets 'this' cast to VBase* in ECX (i.e. this+8) so we
 // need to adjust 'this' before use.
 //
-// Store initial this:
+// Coerce this to correct type:
+// CHECK:   %[[THIS_STORE:.*]] = alloca %struct.B*
 // CHECK:   %[[THIS_ADDR:.*]] = alloca %struct.B*
-// CHECK:   store %struct.B* %{{.*}}, %struct.B** %[[THIS_ADDR]], align 4
+// CHECK:   %[[COERCE_VAL:.*]] = bitcast i8* %{{.*}} to %struct.B*
+// CHECK:   store %struct.B* %[[COERCE_VAL]], %struct.B** %[[THIS_STORE]], align 4
+//
+// Store initial this:
+// CHECK:   %[[THIS_INIT:.*]] = load %struct.B*, %struct.B** %[[THIS_STORE]]
+// CHECK:   store %struct.B* %[[THIS_INIT]], %struct.B** %[[THIS_ADDR]], align 4
 //
 // Reload and adjust the this parameter:
 // CHECK:   %[[THIS_RELOAD:.*]] = load %struct.B*, %struct.B** %[[THIS_ADDR]]

diff --git a/clang/test/Sema/transparent-union.c b/clang/test/Sema/transparent-union.c
index 0481127..7967535 100644 (file)
--- a/clang/test/Sema/transparent-union.c
+++ b/clang/test/Sema/transparent-union.c
@@ -43,6 +43,35 @@ void fi(int i) {} // expected-error{{conflicting types}}
 void fvpp(TU); // expected-note{{previous declaration is here}}
 void fvpp(void **v) {} // expected-error{{conflicting types}}
 
+/* Test redeclaring a function taking a transparent_union arg more than twice.
+   Merging different declarations depends on their order, vary order too. */
+
+void f_triple0(TU tu) {}
+void f_triple0(int *); // expected-note{{previous declaration is here}}
+void f_triple0(float *f); // expected-error{{conflicting types}}
+
+void f_triple1(int *);
+void f_triple1(TU tu) {} // expected-note{{previous definition is here}}
+void f_triple1(float *f); // expected-error{{conflicting types}}
+
+void f_triple2(int *); // expected-note{{previous declaration is here}}
+void f_triple2(float *f); // expected-error{{conflicting types}}
+void f_triple2(TU tu) {}
+
+/* Test calling redeclared function taking a transparent_union arg. */
+
+void f_callee(TU);
+void f_callee(int *i) {} // expected-note{{passing argument to parameter 'i' here}}
+
+void caller(void) {
+  TU tu;
+  f_callee(tu); // expected-error{{passing 'TU' to parameter of incompatible type 'int *'}}
+
+  int *i;
+  f_callee(i);
+}
+
+
 /* FIXME: we'd like to just use an "int" here and align it differently
    from the normal "int", but if we do so we lose the alignment
    information from the typedef within the compiler. */