caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[lldb] Change ObjectValueDictionary to use a StringMap

Browse Source 
llvm has a structure for maps where the key's type is a string. Using
that also means that the keys for OptionValueDictionary don't stick
around forever in ConstString's StringPool (even after they are gone).

The only thing we lose here is ordering: iterating over the map where the keys
are ConstStrings guarantees that we iterate in alphabetical order.
StringMap makes no guarantees about the ordering when you iterate over
the entire map.

Differential Revision: https://reviews.llvm.org/D149482
This commit is contained in:
Alex Langford
2023-04-28 13:23:55 -07:00
parent 2a333e9104
commit e53e1de57e
10 changed files with 52 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
lldb/include/lldb/Interpreter/OptionValueDictionary.h
View File

@@ -9,11 +9,11 @@
#ifndef LLDB_INTERPRETER_OPTIONVALUEDICTIONARY_H
#define LLDB_INTERPRETER_OPTIONVALUEDICTIONARY_H
#include <map>
#include "lldb/Interpreter/OptionValue.h"
#include "lldb/lldb-private-types.h"
#include "llvm/ADT/StringMap.h"
namespace lldb_private {
class OptionValueDictionary
@@ -58,7 +58,7 @@ public:
size_t GetNumValues() const { return m_values.size(); }
lldb::OptionValueSP GetValueForKey(ConstString key) const;
lldb::OptionValueSP GetValueForKey(llvm::StringRef key) const;
lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
llvm::StringRef name, bool will_modify,
@@ -67,21 +67,19 @@ public:
Status SetSubValue(const ExecutionContext *exe_ctx, VarSetOperationType op,
llvm::StringRef name, llvm::StringRef value) override;
bool SetValueForKey(ConstString key,
const lldb::OptionValueSP &value_sp,
bool SetValueForKey(llvm::StringRef key, const lldb::OptionValueSP &value_sp,
bool can_replace = true);
bool DeleteValueForKey(ConstString key);
bool DeleteValueForKey(llvm::StringRef key);
size_t GetArgs(Args &args) const;
Status SetArgs(const Args &args, VarSetOperationType op);
protected:
typedef std::map<ConstString, lldb::OptionValueSP> collection;
uint32_t m_type_mask;
OptionEnumValues m_enum_values;
collection m_values;
llvm::StringMap<lldb::OptionValueSP> m_values;
bool m_raw_value_dump;
};

14
lldb/source/Core/Disassembler.cpp
View File

@@ -755,7 +755,7 @@ OptionValueSP Instruction::ReadDictionary(FILE *in_file, Stream *out_stream) {
char buffer[1024];
auto option_value_sp = std::make_shared<OptionValueDictionary>();
static ConstString encoding_key("data_encoding");
static constexpr llvm::StringLiteral encoding_key("data_encoding");
OptionValue::Type data_type = OptionValue::eTypeInvalid;
while (!done) {
@@ -802,7 +802,6 @@ OptionValueSP Instruction::ReadDictionary(FILE *in_file, Stream *out_stream) {
return option_value_sp;
}
ConstString const_key(key.c_str());
// Check value to see if it's the start of an array or dictionary.
lldb::OptionValueSP value_sp;
@@ -838,15 +837,14 @@ OptionValueSP Instruction::ReadDictionary(FILE *in_file, Stream *out_stream) {
value_sp = std::make_shared<OptionValueString>(value.c_str(), "");
}
if (const_key == encoding_key) {
if (key == encoding_key) {
// A 'data_encoding=..." is NOT a normal key-value pair; it is meta-data
// indicating the
// data type of an upcoming array (usually the next bit of data to be
// read in).
if (strcmp(value.c_str(), "uint32_t") == 0)
// indicating the data type of an upcoming array (usually the next bit
// of data to be read in).
if (llvm::StringRef(value) == "uint32_t")
data_type = OptionValue::eTypeUInt64;
} else
option_value_sp->GetAsDictionary()->SetValueForKey(const_key, value_sp,
option_value_sp->GetAsDictionary()->SetValueForKey(key, value_sp,
false);
}
}

45
lldb/source/Interpreter/OptionValueDictionary.cpp
View File

@@ -33,20 +33,24 @@ void OptionValueDictionary::DumpValue(const ExecutionContext *exe_ctx,
if (dump_mask & eDumpOptionType)
strm.PutCString(" =");
collection::iterator pos, end = m_values.end();
if (!one_line)
strm.IndentMore();
for (pos = m_values.begin(); pos != end; ++pos) {
OptionValue *option_value = pos->second.get();
// m_values is not guaranteed to be sorted alphabetically, so for
// consistentcy we will sort them here before dumping
std::map<llvm::StringRef, OptionValue *> sorted_values;
for (const auto &value : m_values) {
sorted_values[value.first()] = value.second.get();
}
for (const auto &value : sorted_values) {
OptionValue *option_value = value.second;
if (one_line)
strm << ' ';
else
strm.EOL();
strm.Indent(pos->first.GetStringRef());
strm.Indent(value.first);
const uint32_t extra_dump_options = m_raw_value_dump ? eDumpOptionRaw : 0;
switch (dict_type) {
@@ -87,18 +91,17 @@ llvm::json::Value
OptionValueDictionary::ToJSON(const ExecutionContext *exe_ctx) {
llvm::json::Object dict;
for (const auto &value : m_values) {
dict.try_emplace(value.first.GetCString(), value.second->ToJSON(exe_ctx));
dict.try_emplace(value.first(), value.second->ToJSON(exe_ctx));
}
return dict;
}
size_t OptionValueDictionary::GetArgs(Args &args) const {
args.Clear();
collection::const_iterator pos, end = m_values.end();
for (pos = m_values.begin(); pos != end; ++pos) {
for (const auto &value : m_values) {
StreamString strm;
strm.Printf("%s=", pos->first.GetCString());
pos->second->DumpValue(nullptr, strm, eDumpOptionValue | eDumpOptionRaw);
strm.Printf("%s=", value.first().data());
value.second->DumpValue(nullptr, strm, eDumpOptionValue | eDumpOptionRaw);
args.AppendArgument(strm.GetString());
}
return args.GetArgumentCount();
@@ -178,7 +181,7 @@ Status OptionValueDictionary::SetArgs(const Args &args,
if (error.Fail())
return error;
m_value_was_set = true;
SetValueForKey(ConstString(key), enum_value, true);
SetValueForKey(key, enum_value, true);
} else {
lldb::OptionValueSP value_sp(CreateValueFromCStringForTypeMask(
value.str().c_str(), m_type_mask, error));
@@ -186,7 +189,7 @@ Status OptionValueDictionary::SetArgs(const Args &args,
if (error.Fail())
return error;
m_value_was_set = true;
SetValueForKey(ConstString(key), value_sp, true);
SetValueForKey(key, value_sp, true);
} else {
error.SetErrorString("dictionaries that can contain multiple types "
"must subclass OptionValueArray");
@@ -198,11 +201,11 @@ Status OptionValueDictionary::SetArgs(const Args &args,
case eVarSetOperationRemove:
if (argc > 0) {
for (size_t i = 0; i < argc; ++i) {
ConstString key(args.GetArgumentAtIndex(i));
llvm::StringRef key(args.GetArgumentAtIndex(i));
if (!DeleteValueForKey(key)) {
error.SetErrorStringWithFormat(
"no value found named '%s', aborting remove operation",
key.GetCString());
key.data());
break;
}
}
@@ -267,7 +270,7 @@ OptionValueDictionary::GetSubValue(const ExecutionContext *exe_ctx,
return nullptr;
}
value_sp = GetValueForKey(ConstString(key));
value_sp = GetValueForKey(key);
if (!value_sp) {
error.SetErrorStringWithFormat(
"dictionary does not contain a value for the key name '%s'",
@@ -297,22 +300,22 @@ Status OptionValueDictionary::SetSubValue(const ExecutionContext *exe_ctx,
}
lldb::OptionValueSP
OptionValueDictionary::GetValueForKey(ConstString key) const {
OptionValueDictionary::GetValueForKey(llvm::StringRef key) const {
lldb::OptionValueSP value_sp;
collection::const_iterator pos = m_values.find(key);
auto pos = m_values.find(key);
if (pos != m_values.end())
value_sp = pos->second;
return value_sp;
}
bool OptionValueDictionary::SetValueForKey(ConstString key,
bool OptionValueDictionary::SetValueForKey(llvm::StringRef key,
const lldb::OptionValueSP &value_sp,
bool can_replace) {
// Make sure the value_sp object is allowed to contain values of the type
// passed in...
if (value_sp && (m_type_mask & value_sp->GetTypeAsMask())) {
if (!can_replace) {
collection::const_iterator pos = m_values.find(key);
auto pos = m_values.find(key);
if (pos != m_values.end())
return false;
}
@@ -322,8 +325,8 @@ bool OptionValueDictionary::SetValueForKey(ConstString key,
return false;
}
bool OptionValueDictionary::DeleteValueForKey(ConstString key) {
collection::iterator pos = m_values.find(key);
bool OptionValueDictionary::DeleteValueForKey(llvm::StringRef key) {
auto pos = m_values.find(key);
if (pos != m_values.end()) {
m_values.erase(pos);
return true;

7
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
View File

@@ -18,7 +18,6 @@
#include "lldb/Interpreter/OptionValueDictionary.h"
#include "lldb/Symbol/UnwindPlan.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/Stream.h"
#include "Plugins/Process/Utility/ARMDefines.h"
@@ -14353,9 +14352,9 @@ bool EmulateInstructionARM::TestEmulation(Stream *out_stream, ArchSpec &arch,
return false;
}
static ConstString opcode_key("opcode");
static ConstString before_key("before_state");
static ConstString after_key("after_state");
static constexpr llvm::StringLiteral opcode_key("opcode");
static constexpr llvm::StringLiteral before_key("before_state");
static constexpr llvm::StringLiteral after_key("after_state");
OptionValueSP value_sp = test_data->GetValueForKey(opcode_key);

1
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
View File

@@ -11,7 +11,6 @@
#include "Plugins/Process/Utility/ARMDefines.h"
#include "lldb/Core/EmulateInstruction.h"
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/Status.h"
#include <optional>

13
lldb/source/Plugins/Instruction/ARM/EmulationStateARM.cpp
View File

@@ -264,8 +264,7 @@ bool EmulationStateARM::LoadRegistersStateFromDictionary(
for (int i = 0; i < num; ++i) {
sstr.Clear();
sstr.Printf("%c%d", kind, i);
OptionValueSP value_sp =
reg_dict->GetValueForKey(ConstString(sstr.GetString()));
OptionValueSP value_sp = reg_dict->GetValueForKey(sstr.GetString());
if (value_sp.get() == nullptr)
return false;
uint64_t reg_value = value_sp->GetUInt64Value();
@@ -277,8 +276,8 @@ bool EmulationStateARM::LoadRegistersStateFromDictionary(
bool EmulationStateARM::LoadStateFromDictionary(
OptionValueDictionary *test_data) {
static ConstString memory_key("memory");
static ConstString registers_key("registers");
static constexpr llvm::StringLiteral memory_key("memory");
static constexpr llvm::StringLiteral registers_key("registers");
if (!test_data)
return false;
@@ -288,8 +287,8 @@ bool EmulationStateARM::LoadStateFromDictionary(
// Load memory, if present.
if (value_sp.get() != nullptr) {
static ConstString address_key("address");
static ConstString data_key("data");
static constexpr llvm::StringLiteral address_key("address");
static constexpr llvm::StringLiteral data_key("data");
uint64_t start_address = 0;
OptionValueDictionary *mem_dict = value_sp->GetAsDictionary();
@@ -327,7 +326,7 @@ bool EmulationStateARM::LoadStateFromDictionary(
if (!LoadRegistersStateFromDictionary(reg_dict, 'r', dwarf_r0, 16))
return false;
static ConstString cpsr_name("cpsr");
static constexpr llvm::StringLiteral cpsr_name("cpsr");
value_sp = reg_dict->GetValueForKey(cpsr_name);
if (value_sp.get() == nullptr)
return false;

1
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
View File

@@ -12,7 +12,6 @@
#include "lldb/Core/PluginManager.h"
#include "lldb/Symbol/UnwindPlan.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/RegisterValue.h"
#include "lldb/Utility/Stream.h"

8
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
View File

@@ -300,20 +300,18 @@ size_t ObjectFilePECOFF::GetModuleSpecifications(
if (!module_env_option) {
// Step 2: Try with the file name in lowercase.
auto name_lower = name.GetStringRef().lower();
module_env_option =
map->GetValueForKey(ConstString(llvm::StringRef(name_lower)));
module_env_option = map->GetValueForKey(llvm::StringRef(name_lower));
}
if (!module_env_option) {
// Step 3: Try with the file name with ".debug" suffix stripped.
auto name_stripped = name.GetStringRef();
if (name_stripped.consume_back_insensitive(".debug")) {
module_env_option = map->GetValueForKey(ConstString(name_stripped));
module_env_option = map->GetValueForKey(name_stripped);
if (!module_env_option) {
// Step 4: Try with the file name in lowercase with ".debug" suffix
// stripped.
auto name_lower = name_stripped.lower();
module_env_option =
map->GetValueForKey(ConstString(llvm::StringRef(name_lower)));
module_env_option = map->GetValueForKey(llvm::StringRef(name_lower));
}
}
}

4
lldb/test/API/commands/settings/TestSettings.py
View File

@@ -60,7 +60,7 @@ class SettingsCommandTestCase(TestBase):
self.assertEqual(self.dbg.GetSelectedTarget().GetNumBreakpoints(), 1)
def test_append_target_env_vars(self):
"""Test that 'append target.run-args' works."""
"""Test that 'append target.env-vars' works."""
# Append the env-vars.
self.runCmd('settings append target.env-vars MY_ENV_VAR=YES')
# And add hooks to restore the settings during tearDown().
@@ -268,7 +268,7 @@ class SettingsCommandTestCase(TestBase):
found_env_var = True
break
self.assertTrue(found_env_var,
"MY_ENV_VAR was not set in LunchInfo object")
"MY_ENV_VAR was not set in LaunchInfo object")
self.assertEqual(launch_info.GetNumArguments(), 3)
self.assertEqual(launch_info.GetArgumentAtIndex(0), "A")

4
lldb/unittests/Interpreter/TestOptionValue.cpp
View File

@@ -146,12 +146,12 @@ TEST(TestProperties, DeepCopy) {
ASSERT_TRUE(dict_copy_ptr->OptionWasSet());
ASSERT_EQ(dict_copy_ptr->GetNumValues(), 2U);
auto value_ptr = dict_copy_ptr->GetValueForKey(ConstString("A"));
auto value_ptr = dict_copy_ptr->GetValueForKey("A");
ASSERT_TRUE(value_ptr);
ASSERT_EQ(value_ptr->GetParent().get(), dict_copy_ptr);
ASSERT_EQ(value_ptr->GetUInt64Value(), 1U);
value_ptr = dict_copy_ptr->GetValueForKey(ConstString("B"));
value_ptr = dict_copy_ptr->GetValueForKey("B");
ASSERT_TRUE(value_ptr);
ASSERT_EQ(value_ptr->GetParent().get(), dict_copy_ptr);
ASSERT_EQ(value_ptr->GetUInt64Value(), 2U);