From 55fb6ba168b55e3b832293b312ae6a5899f23af3 Mon Sep 17 00:00:00 2001 From: Andres Madrid Ucha Date: Wed, 10 Dec 2025 13:35:57 +0100 Subject: [PATCH] Clang: Fix mismatch in elementtype When processing thunk functions and elementtypes with inheritance, there were logic errors in the direct base checking that could lead to incorrect function types being retained. The issue occurred in the following case: 1. Initial comparison: CurrentThisStructTy (ios_base) == NewThisTy (ios_base) - Both types are the same, so NewIsDirectBase is incorrectly set to true - ThunkFn type: void (%class._ZSt8ios_base*) - ThunkFnTy: (%class._ZSt8ios_base*) - Since types match, no update occurs 2. Second comparison: CurrentThisStructTy != NewThisTy - ThunkFn then gets updated to void (%class._ZSt9basic_iosIcSt11char_traitsIcEE*) - This is correct behavior 3. Third comparison: Same types again (ios_base == ios_base) - NewIsDirectBase is again set to true, which will then affect updating the ThunkFn - Current ThunkFn: void (%class._ZSt9basic_iosIcSt11char_traitsIcEE*) but the one we want to update to is void (%class._ZSt8ios_base*) - Because NewIsDirectBase is true, the update is skipped eventhough ThunkFn != ThunkFnTy. This leaves ThunkFn with the wrong type --- clang/lib/CodeGen/CGVTables.cpp | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index 05903e86937d..b208907286a0 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -586,31 +586,37 @@ llvm::Constant *CodeGenVTables::maybeEmitThunk(GlobalDecl GD, // CHEERP: If there are virtual bases it is possible that the type of `this` mismatches // between the existing and new think, but one is directbase of the other. // We keep the thunk with the most basic `this` type + + if (!ThunkFn->hasParamAttribute(thisIndex, llvm::Attribute::ElementType)) + ThunkFn->addParamAttr(thisIndex, llvm::Attribute::get(CGM.getLLVMContext(), llvm::Attribute::ElementType, CurrentThisTy)); + bool NewIsDirectBase = false; bool CurrentIsDirectBase = false; if (!CGM.getTarget().isByteAddressable()) { - // Geth the `this` type. It can be the first or second parameter (if there is a + // Get the `this` type. It can be the first or second parameter (if there is a // struct return pointer) llvm::StructType* CurrentThisStructTy = cast(CurrentThisTy); llvm::StructType* NewThisTy = cast(ThunkFn->getParamAttribute(thisIndex, llvm::Attribute::ElementType).getValueAsType()); - - // Check if one of the types is directbase of the other (check all the directbase hierarchy) - for (llvm::StructType* I = NewThisTy; I != nullptr; I = I->getDirectBase()) { - if (I == CurrentThisTy) { - NewIsDirectBase = true; - break; + // If the attribute type is the same as current, or doesn't exist, skip the directbase check + if (NewThisTy && NewThisTy != CurrentThisTy) { + // Check if one of the types is directbase of the other (check all the directbase hierarchy) + for (llvm::StructType* I = NewThisTy; I != nullptr; I = I->getDirectBase()) { + if (I == CurrentThisTy) { + NewIsDirectBase = true; + break; + } } - } - for (llvm::StructType* I = CurrentThisStructTy; I != nullptr; I = I->getDirectBase()) { - if (I == NewThisTy) { - CurrentIsDirectBase = true; - break; + for (llvm::StructType* I = CurrentThisStructTy; I != nullptr; I = I->getDirectBase()) { + if (I == NewThisTy) { + CurrentIsDirectBase = true; + break; + } } } } + if (ThunkFn->getFunctionType() != ThunkFnTy && !NewIsDirectBase) { llvm::GlobalValue *OldThunkFn = ThunkFn; - // If the types mismatch then we have to rewrite the definition. assert((OldThunkFn->isDeclaration() || CurrentIsDirectBase) && "Shouldn't replace non-declaration"); @@ -634,6 +640,7 @@ llvm::Constant *CodeGenVTables::maybeEmitThunk(GlobalDecl GD, OldThunkFn->eraseFromParent(); } + bool ABIHasKeyFunctions = CGM.getTarget().getCXXABI().hasKeyFunctions(); bool UseAvailableExternallyLinkage = ForVTable && ABIHasKeyFunctions;