tejohnson created this revision. tejohnson added a reviewer: evgeny777. Herald added a subscriber: Prazek. Herald added a project: clang.
The MicrosoftCXXABI uses a separate mechanism for emitting vtable type metadata, and thus didn't pick up the change from D71907 <https://reviews.llvm.org/D71907> to emit the vcall_visibility metadata under -fwhole-program-vtables. I believe this is the cause of a Windows bot failure when I committed follow on change D71913 <https://reviews.llvm.org/D71913> that required a revert. The failure occurred in a CFI test that was expecting to not abort because it expected a devirtualization to occur, and without the necessary vcall_visibility metadata we would not get devirtualization. Note in the equivalent code in CodeGenModule::EmitVTableTypeMetadata (used by the ItaniumCXXABI), we also emit the vcall_visibility metadata when Virtual Function Elimination is enabled. Since I am not as familiar with the details of that optimization, I have marked that as a TODO and am only inserting under -fwhole-program-vtables. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D73418 Files: clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/test/CodeGenCXX/vcall-visibility-metadata.cpp Index: clang/test/CodeGenCXX/vcall-visibility-metadata.cpp =================================================================== --- clang/test/CodeGenCXX/vcall-visibility-metadata.cpp +++ clang/test/CodeGenCXX/vcall-visibility-metadata.cpp @@ -1,5 +1,6 @@ // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm -fvirtual-function-elimination -fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VFE // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm -fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOVFE +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -emit-llvm -fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK-MS --check-prefix=CHECK-NOVFE // Check that in ThinLTO we also get vcall_visibility summary entries in the bitcode // RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux -emit-llvm-bc -fwhole-program-vtables -o - %s | llvm-dis -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOVFE --check-prefix=CHECK-SUMMARY @@ -87,6 +88,15 @@ return new G(); } +// CHECK-MS: comdat($"??_7A@?A0xE24FC2D5@@6B@"){{.*}} !vcall_visibility [[VIS_TU:![0-9]+]] +// CHECK-MS: comdat($"??_7B@@6B@"){{.*}} !vcall_visibility [[VIS_DSO:![0-9]+]] +// CHECK-MS: comdat($"??_7C@@6B@"){{.*}} !vcall_visibility [[VIS_DSO]] +// CHECK-MS: comdat($"??_7E@@6B@"){{.*}} !vcall_visibility [[VIS_DSO]] +// CHECK-MS: comdat($"??_7F@?A0xE24FC2D5@@6B@"){{.*}} !vcall_visibility [[VIS_DSO]] +// CHECK-MS: comdat($"??_7G@?A0xE24FC2D5@@6B@"){{.*}} !vcall_visibility [[VIS_DSO]] +// CHECK-MS-DAG: [[VIS_TU]] = !{i64 2} +// CHECK-MS-DAG: [[VIS_DSO]] = !{i64 1} + // CHECK-DAG: [[VIS_DSO]] = !{i64 1} // CHECK-DAG: [[VIS_TU]] = !{i64 2} // CHECK-VFE-DAG: !{i32 1, !"Virtual Function Elim", i32 1} Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp =================================================================== --- clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -1621,6 +1621,15 @@ if (!CGM.getCodeGenOpts().LTOUnit) return; + // TODO: Should VirtualFunctionElimination also be supported here? + // See similar handling in CodeGenModule::EmitVTableTypeMetadata. + if (CGM.getCodeGenOpts().WholeProgramVTables) { + llvm::GlobalObject::VCallVisibility TypeVis = + CGM.GetVCallVisibilityLevel(RD); + if (TypeVis != llvm::GlobalObject::VCallVisibilityPublic) + VTable->setVCallVisibilityMetadata(TypeVis); + } + // The location of the first virtual function pointer in the virtual table, // aka the "address point" on Itanium. This is at offset 0 if RTTI is // disabled, or sizeof(void*) if RTTI is enabled.
Index: clang/test/CodeGenCXX/vcall-visibility-metadata.cpp =================================================================== --- clang/test/CodeGenCXX/vcall-visibility-metadata.cpp +++ clang/test/CodeGenCXX/vcall-visibility-metadata.cpp @@ -1,5 +1,6 @@ // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm -fvirtual-function-elimination -fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VFE // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm -fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOVFE +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -emit-llvm -fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK-MS --check-prefix=CHECK-NOVFE // Check that in ThinLTO we also get vcall_visibility summary entries in the bitcode // RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux -emit-llvm-bc -fwhole-program-vtables -o - %s | llvm-dis -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOVFE --check-prefix=CHECK-SUMMARY @@ -87,6 +88,15 @@ return new G(); } +// CHECK-MS: comdat($"??_7A@?A0xE24FC2D5@@6B@"){{.*}} !vcall_visibility [[VIS_TU:![0-9]+]] +// CHECK-MS: comdat($"??_7B@@6B@"){{.*}} !vcall_visibility [[VIS_DSO:![0-9]+]] +// CHECK-MS: comdat($"??_7C@@6B@"){{.*}} !vcall_visibility [[VIS_DSO]] +// CHECK-MS: comdat($"??_7E@@6B@"){{.*}} !vcall_visibility [[VIS_DSO]] +// CHECK-MS: comdat($"??_7F@?A0xE24FC2D5@@6B@"){{.*}} !vcall_visibility [[VIS_DSO]] +// CHECK-MS: comdat($"??_7G@?A0xE24FC2D5@@6B@"){{.*}} !vcall_visibility [[VIS_DSO]] +// CHECK-MS-DAG: [[VIS_TU]] = !{i64 2} +// CHECK-MS-DAG: [[VIS_DSO]] = !{i64 1} + // CHECK-DAG: [[VIS_DSO]] = !{i64 1} // CHECK-DAG: [[VIS_TU]] = !{i64 2} // CHECK-VFE-DAG: !{i32 1, !"Virtual Function Elim", i32 1} Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp =================================================================== --- clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -1621,6 +1621,15 @@ if (!CGM.getCodeGenOpts().LTOUnit) return; + // TODO: Should VirtualFunctionElimination also be supported here? + // See similar handling in CodeGenModule::EmitVTableTypeMetadata. + if (CGM.getCodeGenOpts().WholeProgramVTables) { + llvm::GlobalObject::VCallVisibility TypeVis = + CGM.GetVCallVisibilityLevel(RD); + if (TypeVis != llvm::GlobalObject::VCallVisibilityPublic) + VTable->setVCallVisibilityMetadata(TypeVis); + } + // The location of the first virtual function pointer in the virtual table, // aka the "address point" on Itanium. This is at offset 0 if RTTI is // disabled, or sizeof(void*) if RTTI is enabled.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits