jingham created this revision.
jingham added reviewers: clayborg, jasonmolenda.
Herald added subscribers: lldb-commits, abidh, atanasyan, kbarton, javed.absar,
nemanjai.
For reasons that are unclear to me, when the ABIXXX::CreateInstance function is
called to make a new ABI for a given process and ArchSpec, we would only make
the plugin once, with the initial process and architecture. Then if we got
called with a different process, we would check to see if we already made one
and if we did, hand back the one we had made - even though that was for a
different process.
If there were ever anything per-process that effected the ABI plugin's behavior
(for instance if it relied on a Process property) you could very well use the
wrong processes setting. Even worse, since the ABI's hold onto a process
through a weak pointer, if the initial process had gone away, you would not be
able to get to any process at all, and silently fall back on some default
behavior.
This caching goes back to prehistoric days in lldb, but I can't think of any
reason why we would do this. It seems clearly wrong, and ABI plugins are
really cheap to make - they pretty much just copy the process SP to a weak
pointer and that's about all. So this also seems like an unnecessary
optimization.
Greg or Jason, do you remember why we did this?
Repository:
rLLDB LLDB
https://reviews.llvm.org/D54460
Files:
source/Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.cpp
source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp
source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp
source/Plugins/ABI/SysV-hexagon/ABISysV_hexagon.cpp
source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
source/Plugins/ABI/SysV-mips/ABISysV_mips.cpp
source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp
source/Plugins/ABI/SysV-s390x/ABISysV_s390x.cpp
source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
Index: source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
===================================================================
--- source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
+++ source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
@@ -1091,11 +1091,8 @@
ABISP
ABISysV_x86_64::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
if (arch.GetTriple().getArch() == llvm::Triple::x86_64) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABISysV_x86_64(process_sp));
- return g_abi_sp;
+ return ABISP(new ABISysV_x86_64(process_sp));
}
return ABISP();
}
Index: source/Plugins/ABI/SysV-s390x/ABISysV_s390x.cpp
===================================================================
--- source/Plugins/ABI/SysV-s390x/ABISysV_s390x.cpp
+++ source/Plugins/ABI/SysV-s390x/ABISysV_s390x.cpp
@@ -202,11 +202,8 @@
ABISP
ABISysV_s390x::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
if (arch.GetTriple().getArch() == llvm::Triple::systemz) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABISysV_s390x(process_sp));
- return g_abi_sp;
+ return ABISP(new ABISysV_s390x(process_sp));
}
return ABISP();
}
Index: source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp
===================================================================
--- source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp
+++ source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp
@@ -220,11 +220,8 @@
ABISP
ABISysV_ppc::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
if (arch.GetTriple().getArch() == llvm::Triple::ppc) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABISysV_ppc(process_sp));
- return g_abi_sp;
+ return ABISP(new ABISysV_ppc(process_sp));
}
return ABISP();
}
Index: source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
===================================================================
--- source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
+++ source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
@@ -556,13 +556,10 @@
ABISP
ABISysV_mips64::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
if ((arch_type == llvm::Triple::mips64) ||
(arch_type == llvm::Triple::mips64el)) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABISysV_mips64(process_sp));
- return g_abi_sp;
+ return ABISP(new ABISysV_mips64(process_sp));
}
return ABISP();
}
Index: source/Plugins/ABI/SysV-mips/ABISysV_mips.cpp
===================================================================
--- source/Plugins/ABI/SysV-mips/ABISysV_mips.cpp
+++ source/Plugins/ABI/SysV-mips/ABISysV_mips.cpp
@@ -556,13 +556,10 @@
ABISP
ABISysV_mips::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
if ((arch_type == llvm::Triple::mips) ||
(arch_type == llvm::Triple::mipsel)) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABISysV_mips(process_sp));
- return g_abi_sp;
+ return ABISP(new ABISysV_mips(process_sp));
}
return ABISP();
}
Index: source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
===================================================================
--- source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
+++ source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
@@ -199,12 +199,9 @@
ABISP
ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
if (arch.GetTriple().getArch() == llvm::Triple::x86) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABISysV_i386(process_sp));
- return g_abi_sp;
+ return ABISP(new ABISysV_i386(process_sp));
}
}
return ABISP();
Index: source/Plugins/ABI/SysV-hexagon/ABISysV_hexagon.cpp
===================================================================
--- source/Plugins/ABI/SysV-hexagon/ABISysV_hexagon.cpp
+++ source/Plugins/ABI/SysV-hexagon/ABISysV_hexagon.cpp
@@ -1016,11 +1016,8 @@
ABISP
ABISysV_hexagon::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
if (arch.GetTriple().getArch() == llvm::Triple::hexagon) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABISysV_hexagon(process_sp));
- return g_abi_sp;
+ return ABISP(new ABISysV_hexagon(process_sp));
}
return ABISP();
}
Index: source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp
===================================================================
--- source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp
+++ source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp
@@ -1667,15 +1667,12 @@
ABISP
ABISysV_arm64::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
const llvm::Triple::VendorType vendor_type = arch.GetTriple().getVendor();
if (vendor_type != llvm::Triple::Apple) {
if (arch_type == llvm::Triple::aarch64) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABISysV_arm64(process_sp));
- return g_abi_sp;
+ return ABISP(new ABISysV_arm64(process_sp));
}
}
Index: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
===================================================================
--- source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
+++ source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
@@ -1324,16 +1324,13 @@
ABISP
ABISysV_arm::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
const llvm::Triple::VendorType vendor_type = arch.GetTriple().getVendor();
if (vendor_type != llvm::Triple::Apple) {
if ((arch_type == llvm::Triple::arm) ||
(arch_type == llvm::Triple::thumb)) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABISysV_arm(process_sp));
- return g_abi_sp;
+ return ABISP(new ABISysV_arm(process_sp));
}
}
Index: source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp
===================================================================
--- source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp
+++ source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp
@@ -710,13 +710,10 @@
ABISP
ABIMacOSX_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
(arch.GetTriple().isMacOSX() || arch.GetTriple().isiOS() ||
arch.GetTriple().isWatchOS())) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABIMacOSX_i386(process_sp));
- return g_abi_sp;
+ return ABISP(new ABIMacOSX_i386(process_sp));
}
return ABISP();
}
Index: source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
===================================================================
--- source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
+++ source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
@@ -1664,15 +1664,12 @@
ABISP
ABIMacOSX_arm64::CreateInstance(ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
const llvm::Triple::VendorType vendor_type = arch.GetTriple().getVendor();
if (vendor_type == llvm::Triple::Apple) {
if (arch_type == llvm::Triple::aarch64) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABIMacOSX_arm64(process_sp));
- return g_abi_sp;
+ return ABISP(new ABIMacOSX_arm64(process_sp));
}
}
Index: source/Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.cpp
===================================================================
--- source/Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.cpp
+++ source/Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.cpp
@@ -1323,16 +1323,13 @@
ABISP
ABIMacOSX_arm::CreateInstance(ProcessSP process_sp, const ArchSpec &arch) {
- static ABISP g_abi_sp;
const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
const llvm::Triple::VendorType vendor_type = arch.GetTriple().getVendor();
if (vendor_type == llvm::Triple::Apple) {
if ((arch_type == llvm::Triple::arm) ||
(arch_type == llvm::Triple::thumb)) {
- if (!g_abi_sp)
- g_abi_sp.reset(new ABIMacOSX_arm(process_sp));
- return g_abi_sp;
+ return ABISP(new ABIMacOSX_arm(process_sp));
}
}
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
