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

Author: Andres Madrid Ucha

clang/lib/CodeGen/CGVTables.cpp

@@ -577,57 +577,44 @@ llvm::Constant *CodeGenVTables::maybeEmitThunk(GlobalDecl GD,
 
   int thisIndex = ThunkFn->hasStructRetAttr() && !FnInfo.getReturnInfo().isSRetAfterThis() ? 1 : 0;
 
-  // CurrentThisTy is the value used for elementtype attribute
-  // Extract the pointee type from the actual parameter to ensure consistency
-  // between the function signature and the elementtype attribute
-  llvm::Type* CurrentThisTy = nullptr;
-  
-  if (thisIndex < ThunkFn->arg_size()) {
-    llvm::Argument* ThisArg = ThunkFn->getArg(thisIndex);
-    llvm::Type* ActualParamType = ThisArg->getType();
-    auto* PtrTy = dyn_cast<llvm::PointerType>(ActualParamType);
-    // The getPointerElementType function is depricated for Opaque pointers
-    if (PtrTy && !PtrTy->isOpaque())
-      CurrentThisTy = PtrTy->getPointerElementType();
-  }
-
-  if (CurrentThisTy == nullptr)
-    CurrentThisTy = CGM.getTypes().ConvertTypeForMem(cast<PointerType>(FnInfo.arg_begin()->type)->getPointeeType());
-
-
-  if (!ThunkFn->hasParamAttribute(thisIndex, llvm::Attribute::ElementType))
-    ThunkFn->addParamAttr(thisIndex, llvm::Attribute::get(CGM.getLLVMContext(), llvm::Attribute::ElementType, CurrentThisTy));
+  llvm::Type* CurrentThisTy = CGM.getTypes().ConvertTypeForMem(cast<PointerType>(FnInfo.arg_begin()->type)->getPointeeType());
 
   // If the type of the underlying GlobalValue is wrong, we'll have to replace
   // it. It should be a declaration.
   // 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<llvm::StructType>(CurrentThisTy);
     llvm::StructType* NewThisTy = cast<llvm::StructType>(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");
@@ -651,6 +638,7 @@ llvm::Constant *CodeGenVTables::maybeEmitThunk(GlobalDecl GD,
     OldThunkFn->eraseFromParent();
   }
 
+
   bool ABIHasKeyFunctions = CGM.getTarget().getCXXABI().hasKeyFunctions();
   bool UseAvailableExternallyLinkage = ForVTable && ABIHasKeyFunctions;