kernigh created this revision. kernigh added reviewers: brad, nemanjai, efriedma. Herald added subscribers: cfe-commits, shchenz, kbarton. Herald added a project: clang. kernigh requested review of this revision.
In the PPC32 SVR4 ABI, a va_list has copies of registers from the function call. va_arg looked in the wrong registers for (the pointer representation of) an object in Objective-C. Fix va_arg to look in the general-purpose registers, not the floating-point registers. Anthony Richardby found the problem. Eli Friedman suggested to call hasPointerRepresentation(). Fixes https://bugs.llvm.org/show_bug.cgi?id=47921 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D90329 Files: clang/lib/CodeGen/TargetInfo.cpp clang/test/CodeGenObjC/ppc32-varargs-id.m Index: clang/test/CodeGenObjC/ppc32-varargs-id.m =================================================================== --- /dev/null +++ clang/test/CodeGenObjC/ppc32-varargs-id.m @@ -0,0 +1,33 @@ +// REQUIRES: powerpc-registered-target +// RUN: %clang_cc1 -triple powerpc-unknown-openbsd -fblocks -emit-llvm -o - %s | FileCheck %s + +#include <stdarg.h> + +id testObject(va_list ap) { + return va_arg(ap, id); +} +// CHECK: @testObject +// CHECK: using_regs: +// CHECK-NEXT: getelementptr inbounds %struct.__va_list_tag, %struct.__va_list_tag* %{{[0-9]+}}, i32 0, i32 4 +// CHECK-NEXT: load i8*, i8** %{{[0-9]+}}, align 4 +// CHECK-NEXT: mul i8 %numUsedRegs, 4 +// CHECK-NEXT: getelementptr inbounds i8, i8* %{{[0-9]+}}, i8 %{{[0-9]+}} +// CHECK-NEXT: bitcast i8* %{{[0-9]+}} to i8** +// CHECK-NEXT: add i8 %numUsedRegs, 1 +// CHECK-NEXT: store i8 %{{[0-9]+}}, i8* %gpr, align 4 +// CHECK-NEXT: br label %cont + +typedef void (^block)(void); +block testBlock(va_list ap) { + return va_arg(ap, block); +} +// CHECK: @testBlock +// CHECK: using_regs: +// CHECK-NEXT: getelementptr inbounds %struct.__va_list_tag, %struct.__va_list_tag* %{{[0-9]+}}, i32 0, i32 4 +// CHECK-NEXT: load i8*, i8** %{{[0-9]+}}, align 4 +// CHECK-NEXT: mul i8 %numUsedRegs, 4 +// CHECK-NEXT: getelementptr inbounds i8, i8* %{{[0-9]+}}, i8 %{{[0-9]+}} +// CHECK-NEXT: bitcast i8* %{{[0-9]+}} to void ()** +// CHECK-NEXT: add i8 %numUsedRegs, 1 +// CHECK-NEXT: store i8 %{{[0-9]+}}, i8* %gpr, align 4 +// CHECK-NEXT: br label %cont Index: clang/lib/CodeGen/TargetInfo.cpp =================================================================== --- clang/lib/CodeGen/TargetInfo.cpp +++ clang/lib/CodeGen/TargetInfo.cpp @@ -4719,8 +4719,8 @@ // }; bool isI64 = Ty->isIntegerType() && getContext().getTypeSize(Ty) == 64; - bool isInt = - Ty->isIntegerType() || Ty->isPointerType() || Ty->isAggregateType(); + bool isInt = Ty->isIntegerType() || Ty->hasPointerRepresentation() || + Ty->isAggregateType(); bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64; // All aggregates are passed indirectly? That doesn't seem consistent
Index: clang/test/CodeGenObjC/ppc32-varargs-id.m =================================================================== --- /dev/null +++ clang/test/CodeGenObjC/ppc32-varargs-id.m @@ -0,0 +1,33 @@ +// REQUIRES: powerpc-registered-target +// RUN: %clang_cc1 -triple powerpc-unknown-openbsd -fblocks -emit-llvm -o - %s | FileCheck %s + +#include <stdarg.h> + +id testObject(va_list ap) { + return va_arg(ap, id); +} +// CHECK: @testObject +// CHECK: using_regs: +// CHECK-NEXT: getelementptr inbounds %struct.__va_list_tag, %struct.__va_list_tag* %{{[0-9]+}}, i32 0, i32 4 +// CHECK-NEXT: load i8*, i8** %{{[0-9]+}}, align 4 +// CHECK-NEXT: mul i8 %numUsedRegs, 4 +// CHECK-NEXT: getelementptr inbounds i8, i8* %{{[0-9]+}}, i8 %{{[0-9]+}} +// CHECK-NEXT: bitcast i8* %{{[0-9]+}} to i8** +// CHECK-NEXT: add i8 %numUsedRegs, 1 +// CHECK-NEXT: store i8 %{{[0-9]+}}, i8* %gpr, align 4 +// CHECK-NEXT: br label %cont + +typedef void (^block)(void); +block testBlock(va_list ap) { + return va_arg(ap, block); +} +// CHECK: @testBlock +// CHECK: using_regs: +// CHECK-NEXT: getelementptr inbounds %struct.__va_list_tag, %struct.__va_list_tag* %{{[0-9]+}}, i32 0, i32 4 +// CHECK-NEXT: load i8*, i8** %{{[0-9]+}}, align 4 +// CHECK-NEXT: mul i8 %numUsedRegs, 4 +// CHECK-NEXT: getelementptr inbounds i8, i8* %{{[0-9]+}}, i8 %{{[0-9]+}} +// CHECK-NEXT: bitcast i8* %{{[0-9]+}} to void ()** +// CHECK-NEXT: add i8 %numUsedRegs, 1 +// CHECK-NEXT: store i8 %{{[0-9]+}}, i8* %gpr, align 4 +// CHECK-NEXT: br label %cont Index: clang/lib/CodeGen/TargetInfo.cpp =================================================================== --- clang/lib/CodeGen/TargetInfo.cpp +++ clang/lib/CodeGen/TargetInfo.cpp @@ -4719,8 +4719,8 @@ // }; bool isI64 = Ty->isIntegerType() && getContext().getTypeSize(Ty) == 64; - bool isInt = - Ty->isIntegerType() || Ty->isPointerType() || Ty->isAggregateType(); + bool isInt = Ty->isIntegerType() || Ty->hasPointerRepresentation() || + Ty->isAggregateType(); bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64; // All aggregates are passed indirectly? That doesn't seem consistent
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits