Hendrik =?utf-8?q?Hübner?= <[email protected]> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/[email protected]>
llvmorg-github-actions[bot] wrote: <!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Hendrik Hübner (HendrikHuebner) <details> <summary>Changes</summary> Ivars can have negative offsets, for example if an ivar from a derived class is placed inside the super classes padding. Currently, the offset is computed as an `uint64_t`, which results in an integer wraparound for negative offsets. The wrapped offset is then written to a global value to be parsed by libobjc2. When libobjc2 reads the value as a truncated signed `int`, it is again implicitly converted correct negative number. However, with assertions enabled we hit an assertion failure in LLVM when creating the global value. See https://godbolt.org/z/Yb7cvWTqP. --- Full diff: https://github.com/llvm/llvm-project/pull/215753.diff 2 Files Affected: - (modified) clang/lib/CodeGen/CGObjCGNU.cpp (+7-5) - (modified) clang/test/CodeGenObjC/gnustep2-ivar-offset.m (+19) ``````````diff diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp index 32a1afe310629..24e7a2e8f7ebb 100644 --- a/clang/lib/CodeGen/CGObjCGNU.cpp +++ b/clang/lib/CodeGen/CGObjCGNU.cpp @@ -1885,8 +1885,9 @@ class CGObjCGNUstep2 : public CGObjCGNUstep { ivarBuilder.add(MakeConstantString(TypeStr)); // int *offset; uint64_t BaseOffset = ComputeIvarBaseOffset(CGM, OID, IVD); - uint64_t Offset = BaseOffset - superInstanceSize; - llvm::Constant *OffsetValue = llvm::ConstantInt::get(IntTy, Offset); + int64_t Offset = static_cast<int64_t>(BaseOffset) - superInstanceSize; + llvm::Constant *OffsetValue = + llvm::ConstantInt::getSigned(IntTy, Offset); std::string OffsetName = GetIVarOffsetVariableName(classDecl, IVD); llvm::GlobalVariable *OffsetVar = TheModule.getGlobalVariable(OffsetName); if (OffsetVar) @@ -3804,11 +3805,12 @@ void CGObjCGNU::GenerateClass(const ObjCImplementationDecl *OID) { Context.getTypeSize(IVD->getType()))); // Get the offset uint64_t BaseOffset = ComputeIvarBaseOffset(CGM, OID, IVD); - uint64_t Offset = BaseOffset; + int64_t Offset = static_cast<int64_t>(BaseOffset); if (CGM.getLangOpts().ObjCRuntime.isNonFragile()) { - Offset = BaseOffset - superInstanceSize; + Offset = static_cast<int64_t>(BaseOffset) - superInstanceSize; } - llvm::Constant *OffsetValue = llvm::ConstantInt::get(IntTy, Offset); + llvm::Constant *OffsetValue = + llvm::ConstantInt::getSigned(IntTy, Offset); // Create the direct offset value std::string OffsetName = "__objc_ivar_offset_value_" + ClassName +"." + IVD->getNameAsString(); diff --git a/clang/test/CodeGenObjC/gnustep2-ivar-offset.m b/clang/test/CodeGenObjC/gnustep2-ivar-offset.m index dd133ba04e307..c238edcf79380 100644 --- a/clang/test/CodeGenObjC/gnustep2-ivar-offset.m +++ b/clang/test/CodeGenObjC/gnustep2-ivar-offset.m @@ -28,3 +28,22 @@ @implementation ANObject @end // CHECK: @.objc_ivar_list = private global { i32, i64, [4 x { ptr, ptr, ptr, i32, i32 }] } { i32 4, i64 32, // Check that we emit 1 as the size of _Bool, not 0. // CHECK-SAME: @__objc_ivar_offset_ANObject.boolIvar.B, i32 1, i32 4 + + +// The derived class reuses tail padding in the base class. +// Its ivar is at offset 12, while the superclass instance size is 16, so the relative offset is -4. +@interface Base { + long long a; + char b; +} +@end + +@interface Derived : Base { + int c; +} +@end + +@implementation Base @end +@implementation Derived @end + +// CHECK: @__objc_ivar_offset_Derived.c.i = global i32 -4 `````````` </details> https://github.com/llvm/llvm-project/pull/215753 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
