maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[lldb] Remove all the RegisterInfo name constification code 

RegisterInfo's `reg_name`/`reg_alt_name` fields are C-Strings and are supposed
to only be generated from a ConstString. The reason for that is that
`DynamicRegisterInfo::GetRegisterInfo` and
`RegInfoBasedABI::GetRegisterInfoByName` try to optimise finding registers by
name by only comparing the C string pointer values instead of the underlying
strings. This only works if both C strings involved in the comparison come from
a ConstString. If one of the two C strings doesn't come from a ConstString the
comparison won't work (and most likely will silently fail).

I added an assert in b0060c3a78 which checks that
both strings come from a ConstString. Apparently not all ABI plugins are
generating their register names via ConstString, so this code is now not just
silently failing but also asserting.

In D88375 we did a shady fix for the MIPS plugins by just copying the
ConstString setup code to that plugin, but we still need to fix ABISysV_arc,
ABISysV_ppc and ABISysV_ppc64 plugins.

I would say we just fix the remaining plugins by removing the whole requirement
to have the register names coming from ConstStrings. I really doubt that we
actually save any time with the whole ConstString search trick (searching ~50
strings that have <4 characters doesn't sound more expensive than calling the
really expensive ConstString constructor + comparing the same amount of pointer
values). Also whatever small percentage of LLDB's runtime is actually spend in
this function is anyway not worth the complexity of this approach.

This patch just removes all this and just does a normal string comparison.

Reviewed By: JDevlieghere, labath

Differential Revision: https://reviews.llvm.org/D88490
This commit is contained in:
Raphael Isemann 2020-10-13 17:10:10 +02:00
parent 4cd873c4bd
commit 24e07570cc
10 changed files with 21 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lldb/include/lldb/Target/ABI.h
View File

@ -159,7 +159,7 @@ public:
protected:
using ABI::ABI;
bool GetRegisterInfoByName(ConstString name, RegisterInfo &info);
bool GetRegisterInfoByName(llvm::StringRef name, RegisterInfo &info);
virtual const RegisterInfo *GetRegisterInfoArray(uint32_t &count) = 0;
};

17
lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
View File

@ -34,7 +34,7 @@
using namespace lldb;
using namespace lldb_private;
static RegisterInfo g_register_infos[] = {
static const RegisterInfo g_register_infos[] = {
// NAME ALT SZ OFF ENCODING FORMAT EH_FRAME
// DWARF GENERIC PROCESS PLUGIN
// LLDB NATIVE
@ -1292,24 +1292,9 @@ static RegisterInfo g_register_infos[] = {
static const uint32_t k_num_register_infos =
llvm::array_lengthof(g_register_infos);
static bool g_register_info_names_constified = false;
const lldb_private::RegisterInfo *
ABIMacOSX_arm::GetRegisterInfoArray(uint32_t &count) {
// Make the C-string names and alt_names for the register infos into const
// C-string values by having the ConstString unique the names in the global
// constant C-string pool.
if (!g_register_info_names_constified) {
g_register_info_names_constified = true;
for (uint32_t i = 0; i < k_num_register_infos; ++i) {
if (g_register_infos[i].name)
g_register_infos[i].name =
ConstString(g_register_infos[i].name).GetCString();
if (g_register_infos[i].alt_name)
g_register_infos[i].alt_name =
ConstString(g_register_infos[i].alt_name).GetCString();
}
}
count = k_num_register_infos;
return g_register_infos;
}

17
lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
View File

@ -36,7 +36,7 @@ using namespace lldb_private;
LLDB_PLUGIN_DEFINE(ABISysV_arm)
static RegisterInfo g_register_infos[] = {
static const RegisterInfo g_register_infos[] = {
// NAME ALT SZ OFF ENCODING FORMAT EH_FRAME
// DWARF GENERIC PROCESS PLUGIN
// LLDB NATIVE VALUE REGS INVALIDATE REGS
@ -1295,24 +1295,9 @@ static RegisterInfo g_register_infos[] = {
static const uint32_t k_num_register_infos =
llvm::array_lengthof(g_register_infos);
static bool g_register_info_names_constified = false;
const lldb_private::RegisterInfo *
ABISysV_arm::GetRegisterInfoArray(uint32_t &count) {
// Make the C-string names and alt_names for the register infos into const
// C-string values by having the ConstString unique the names in the global
// constant C-string pool.
if (!g_register_info_names_constified) {
g_register_info_names_constified = true;
for (uint32_t i = 0; i < k_num_register_infos; ++i) {
if (g_register_infos[i].name)
g_register_infos[i].name =
ConstString(g_register_infos[i].name).GetCString();
if (g_register_infos[i].alt_name)
g_register_infos[i].alt_name =
ConstString(g_register_infos[i].alt_name).GetCString();
}
}
count = k_num_register_infos;
return g_register_infos;
}

17
lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.cpp
View File

@ -34,7 +34,7 @@ using namespace lldb_private;
LLDB_PLUGIN_DEFINE_ADV(ABISysV_hexagon, ABIHexagon)
static RegisterInfo g_register_infos[] = {
static const RegisterInfo g_register_infos[] = {
// hexagon-core.xml
{"r00",
"",
@ -974,24 +974,9 @@ static RegisterInfo g_register_infos[] = {
static const uint32_t k_num_register_infos =
sizeof(g_register_infos) / sizeof(RegisterInfo);
static bool g_register_info_names_constified = false;
const lldb_private::RegisterInfo *
ABISysV_hexagon::GetRegisterInfoArray(uint32_t &count) {
// Make the C-string names and alt_names for the register infos into const
// C-string values by having the ConstString unique the names in the global
// constant C-string pool.
if (!g_register_info_names_constified) {
g_register_info_names_constified = true;
for (uint32_t i = 0; i < k_num_register_infos; ++i) {
if (g_register_infos[i].name)
g_register_infos[i].name =
ConstString(g_register_infos[i].name).GetCString();
if (g_register_infos[i].alt_name)
g_register_infos[i].alt_name =
ConstString(g_register_infos[i].alt_name).GetCString();
}
}
count = k_num_register_infos;
return g_register_infos;
}

17
lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
View File

@ -75,7 +75,7 @@ enum dwarf_regnums {
dwarf_pc
};
static RegisterInfo g_register_infos[] = {
static const RegisterInfo g_register_infos[] = {
// NAME ALT SZ OFF ENCODING FORMAT EH_FRAME
// DWARF GENERIC PROCESS PLUGINS
// LLDB NATIVE VALUE REGS INVALIDATE REGS
@ -542,24 +542,9 @@ static RegisterInfo g_register_infos[] = {
static const uint32_t k_num_register_infos =
llvm::array_lengthof(g_register_infos);
static bool g_register_info_names_constified = false;
const lldb_private::RegisterInfo *
ABISysV_mips::GetRegisterInfoArray(uint32_t &count) {
// Make the C-string names and alt_names for the register infos into const
// C-string values by having the ConstString unique the names in the global
// constant C-string pool.
if (!g_register_info_names_constified) {
g_register_info_names_constified = true;
for (uint32_t i = 0; i < k_num_register_infos; ++i) {
if (g_register_infos[i].name)
g_register_infos[i].name =
ConstString(g_register_infos[i].name).GetCString();
if (g_register_infos[i].alt_name)
g_register_infos[i].alt_name =
ConstString(g_register_infos[i].alt_name).GetCString();
}
}
count = k_num_register_infos;
return g_register_infos;
}

17
lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
View File

@ -75,7 +75,7 @@ enum dwarf_regnums {
dwarf_pc
};
static RegisterInfo g_register_infos_mips64[] = {
static const RegisterInfo g_register_infos_mips64[] = {
// NAME ALT SZ OFF ENCODING FORMAT EH_FRAME
// DWARF GENERIC PROCESS PLUGIN
// LLDB NATIVE
@ -542,24 +542,9 @@ static RegisterInfo g_register_infos_mips64[] = {
static const uint32_t k_num_register_infos =
llvm::array_lengthof(g_register_infos_mips64);
static bool g_register_info_names_constified = false;
const lldb_private::RegisterInfo *
ABISysV_mips64::GetRegisterInfoArray(uint32_t &count) {
// Make the C-string names and alt_names for the register infos into const
// C-string values by having the ConstString unique the names in the global
// constant C-string pool.
if (!g_register_info_names_constified) {
g_register_info_names_constified = true;
for (uint32_t i = 0; i < k_num_register_infos; ++i) {
if (g_register_infos_mips64[i].name)
g_register_infos_mips64[i].name =
ConstString(g_register_infos_mips64[i].name).GetCString();
if (g_register_infos_mips64[i].alt_name)
g_register_infos_mips64[i].alt_name =
ConstString(g_register_infos_mips64[i].alt_name).GetCString();
}
}
count = k_num_register_infos;
return g_register_infos_mips64;
}

17
lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp
View File

@ -118,7 +118,7 @@ enum dwarf_regnums {
nullptr, nullptr, nullptr, 0 \
}
static RegisterInfo g_register_infos[] = {
static const RegisterInfo g_register_infos[] = {
DEFINE_REG(r0, 8, nullptr, LLDB_INVALID_REGNUM),
DEFINE_REG(r1, 8, nullptr, LLDB_INVALID_REGNUM),
DEFINE_REG(r2, 8, "arg1", LLDB_REGNUM_GENERIC_ARG1),
@ -173,24 +173,9 @@ static RegisterInfo g_register_infos[] = {
static const uint32_t k_num_register_infos =
llvm::array_lengthof(g_register_infos);
static bool g_register_info_names_constified = false;
const lldb_private::RegisterInfo *
ABISysV_s390x::GetRegisterInfoArray(uint32_t &count) {
// Make the C-string names and alt_names for the register infos into const
// C-string values by having the ConstString unique the names in the global
// constant C-string pool.
if (!g_register_info_names_constified) {
g_register_info_names_constified = true;
for (uint32_t i = 0; i < k_num_register_infos; ++i) {
if (g_register_infos[i].name)
g_register_infos[i].name =
ConstString(g_register_infos[i].name).GetCString();
if (g_register_infos[i].alt_name)
g_register_infos[i].alt_name =
ConstString(g_register_infos[i].alt_name).GetCString();
}
}
count = k_num_register_infos;
return g_register_infos;
}

24
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
View File

@ -151,10 +151,8 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict,
const uint32_t msbyte = msbit / 8;
const uint32_t lsbyte = lsbit / 8;
ConstString containing_reg_name(reg_name_str);
const RegisterInfo *containing_reg_info =
GetRegisterInfo(containing_reg_name);
GetRegisterInfo(reg_name_str);
if (containing_reg_info) {
const uint32_t max_bit = containing_reg_info->byte_size * 8;
if (msbit < max_bit && lsbit < max_bit) {
@ -189,7 +187,7 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict,
}
} else {
printf("error: invalid concrete register \"%s\"\n",
containing_reg_name.GetCString());
reg_name_str.c_str());
}
} else {
printf("error: msbit (%u) must be greater than lsbit (%u)\n",
@ -217,7 +215,7 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict,
if (composite_reg_list->GetItemAtIndexAsString(
composite_idx, composite_reg_name, nullptr)) {
const RegisterInfo *composite_reg_info =
GetRegisterInfo(composite_reg_name);
GetRegisterInfo(composite_reg_name.GetStringRef());
if (composite_reg_info) {
composite_offset = std::min(composite_offset,
composite_reg_info->byte_offset);
@ -357,7 +355,7 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict,
if (invalidate_reg_list->GetItemAtIndexAsString(
idx, invalidate_reg_name)) {
const RegisterInfo *invalidate_reg_info =
GetRegisterInfo(invalidate_reg_name);
GetRegisterInfo(invalidate_reg_name.GetStringRef());
if (invalidate_reg_info) {
m_invalidate_regs_map[i].push_back(
invalidate_reg_info->kinds[eRegisterKindLLDB]);
@ -737,16 +735,10 @@ void DynamicRegisterInfo::Dump() const {
}
}
const lldb_private::RegisterInfo *DynamicRegisterInfo::GetRegisterInfo(
lldb_private::ConstString reg_name) const {
for (auto &reg_info : m_regs) {
// We can use pointer comparison since we used a ConstString to set the
// "name" member in AddRegister()
assert(ConstString(reg_info.name).GetCString() == reg_info.name &&
"reg_info.name not from a ConstString?");
if (reg_info.name == reg_name.GetCString()) {
const lldb_private::RegisterInfo *
DynamicRegisterInfo::GetRegisterInfo(llvm::StringRef reg_name) const {
for (auto &reg_info : m_regs)
if (reg_info.name == reg_name)
return &reg_info;
}
}
return nullptr;
}

2
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
View File

@ -75,7 +75,7 @@ protected:
typedef std::map<uint32_t, dwarf_opcode> dynamic_reg_size_map;
const lldb_private::RegisterInfo *
GetRegisterInfo(lldb_private::ConstString reg_name) const;
GetRegisterInfo(llvm::StringRef reg_name) const;
void MoveFrom(DynamicRegisterInfo &&info);

15
lldb/source/Target/ABI.cpp
View File

@ -42,27 +42,22 @@ ABI::FindPlugin(lldb::ProcessSP process_sp, const ArchSpec &arch) {
ABI::~ABI() = default;
bool RegInfoBasedABI::GetRegisterInfoByName(ConstString name, RegisterInfo &info) {
bool RegInfoBasedABI::GetRegisterInfoByName(llvm::StringRef name,
RegisterInfo &info) {
uint32_t count = 0;
const RegisterInfo *register_info_array = GetRegisterInfoArray(count);
if (register_info_array) {
const char *unique_name_cstr = name.GetCString();
uint32_t i;
for (i = 0; i < count; ++i) {
const char *reg_name = register_info_array[i].name;
assert(ConstString(reg_name).GetCString() == reg_name &&
"register_info_array[i].name not from a ConstString?");
if (reg_name == unique_name_cstr) {
if (reg_name == name) {
info = register_info_array[i];
return true;
}
}
for (i = 0; i < count; ++i) {
const char *reg_alt_name = register_info_array[i].alt_name;
assert((reg_alt_name == nullptr ||
ConstString(reg_alt_name).GetCString() == reg_alt_name) &&
"register_info_array[i].alt_name not from a ConstString?");
if (reg_alt_name == unique_name_cstr) {
if (reg_alt_name == name) {
info = register_info_array[i];
return true;
}
@ -224,7 +219,7 @@ void RegInfoBasedABI::AugmentRegisterInfo(RegisterInfo &info) {
return;
RegisterInfo abi_info;
if (!GetRegisterInfoByName(ConstString(info.name), abi_info))
if (!GetRegisterInfoByName(info.name, abi_info))
return;
if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)