Skip to content

Commit de346cf

Browse files
JDevlieghereJDevlieghere
committed
[lldb] Redesign Target::GetUtilityFunctionForLanguage API
This patch redesigns the Target::GetUtilityFunctionForLanguage API: - Use a unique_ptr instead of a raw pointer for the return type. - Wrap the result in an llvm::Expected instead of using a Status object as an I/O parameter. - Combine the action of "getting" and "installing" the UtilityFunction as they always get called together. - Pass std::strings instead of const char* and std::move them where appropriate. There's more room for improvement but I think this tackles the most prevalent issues with the current API. Differential revision: https://reviews.llvm.org/D90011
1 parent 9df832d commit de346cf

File tree

18 files changed

+186
-275
lines changed

18 files changed

+186
-275
lines changed

‎lldb/include/lldb/Symbol/TypeSystem.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,9 @@ class TypeSystem : public PluginInterface {
465465
return nullptr;
466466
}
467467

468-
virtual UtilityFunction *GetUtilityFunction(const char *text,
469-
const char *name) {
470-
return nullptr;
468+
virtual std::unique_ptr<UtilityFunction>
469+
CreateUtilityFunction(std::string text, std::string name) {
470+
return {};
471471
}
472472

473473
virtual PersistentExpressionState *GetPersistentExpressionState() {

‎lldb/include/lldb/Target/Target.h

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,6 @@ class TargetProperties : public Properties {
221221

222222
void UpdateLaunchInfoFromProperties();
223223

224-
225224
private:
226225
// Callbacks for m_launch_info.
227226
void Arg0ValueChangedCallback();
@@ -1074,14 +1073,10 @@ class Target : public std::enable_shared_from_this<Target>,
10741073
const ValueList &arg_value_list,
10751074
const char *name, Status &error);
10761075

1077-
// Creates a UtilityFunction for the given language, the rest of the
1078-
// parameters have the same meaning as for the UtilityFunction constructor.
1079-
// Returns a new-ed object which the caller owns.
1080-
1081-
UtilityFunction *GetUtilityFunctionForLanguage(const char *expr,
1082-
lldb::LanguageType language,
1083-
const char *name,
1084-
Status &error);
1076+
/// Creates and installs a UtilityFunction for the given language.
1077+
llvm::Expected<std::unique_ptr<UtilityFunction>>
1078+
CreateUtilityFunction(std::string expression, std::string name,
1079+
lldb::LanguageType language, ExecutionContext &exe_ctx);
10851080

10861081
// Install any files through the platform that need be to installed prior to
10871082
// launching or attaching.

‎lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,29 +48,27 @@ ClangDynamicCheckerFunctions::~ClangDynamicCheckerFunctions() = default;
4848

4949
bool ClangDynamicCheckerFunctions::Install(
5050
DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx) {
51-
Status error;
52-
m_valid_pointer_check.reset(
53-
exe_ctx.GetTargetRef().GetUtilityFunctionForLanguage(
54-
g_valid_pointer_check_text, lldb::eLanguageTypeC,
55-
VALID_POINTER_CHECK_NAME, error));
56-
if (error.Fail())
51+
auto utility_fn_or_error = exe_ctx.GetTargetRef().CreateUtilityFunction(
52+
g_valid_pointer_check_text, VALID_POINTER_CHECK_NAME,
53+
lldb::eLanguageTypeC, exe_ctx);
54+
if (!utility_fn_or_error) {
55+
llvm::consumeError(utility_fn_or_error.takeError());
5756
return false;
57+
}
58+
m_valid_pointer_check = std::move(*utility_fn_or_error);
5859

59-
if (!m_valid_pointer_check->Install(diagnostic_manager, exe_ctx))
60-
return false;
61-
62-
Process *process = exe_ctx.GetProcessPtr();
63-
64-
if (process) {
60+
if (Process *process = exe_ctx.GetProcessPtr()) {
6561
ObjCLanguageRuntime *objc_language_runtime =
6662
ObjCLanguageRuntime::Get(*process);
6763

6864
if (objc_language_runtime) {
69-
m_objc_object_check.reset(objc_language_runtime->CreateObjectChecker(
70-
VALID_OBJC_OBJECT_CHECK_NAME));
71-
72-
if (!m_objc_object_check->Install(diagnostic_manager, exe_ctx))
65+
auto utility_fn_or_error = objc_language_runtime->CreateObjectChecker(
66+
VALID_OBJC_OBJECT_CHECK_NAME, exe_ctx);
67+
if (!utility_fn_or_error) {
68+
llvm::consumeError(utility_fn_or_error.takeError());
7369
return false;
70+
}
71+
m_objc_object_check = std::move(*utility_fn_or_error);
7472
}
7573
}
7674

‎lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp

Lines changed: 49 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -122,58 +122,60 @@ struct BufStruct {
122122
char contents[2048];
123123
};
124124

125-
UtilityFunction *AppleObjCRuntimeV1::CreateObjectChecker(const char *name) {
125+
llvm::Expected<std::unique_ptr<UtilityFunction>>
126+
AppleObjCRuntimeV1::CreateObjectChecker(std::string name,
127+
ExecutionContext &exe_ctx) {
126128
std::unique_ptr<BufStruct> buf(new BufStruct);
127129

128-
int strformatsize = snprintf(&buf->contents[0], sizeof(buf->contents),
129-
"struct __objc_class "
130-
" \n"
131-
"{ "
132-
" \n"
133-
" struct __objc_class *isa; "
134-
" \n"
135-
" struct __objc_class *super_class; "
136-
" \n"
137-
" const char *name; "
138-
" \n"
139-
" // rest of struct elided because unused "
140-
" \n"
141-
"}; "
142-
" \n"
143-
" "
144-
" \n"
145-
"struct __objc_object "
146-
" \n"
147-
"{ "
148-
" \n"
149-
" struct __objc_class *isa; "
150-
" \n"
151-
"}; "
152-
" \n"
153-
" "
154-
" \n"
155-
"extern \"C\" void "
156-
" \n"
157-
"%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector) "
158-
" \n"
159-
"{ "
160-
" \n"
161-
" struct __objc_object *obj = (struct "
162-
"__objc_object*)$__lldb_arg_obj; \n"
163-
" if ($__lldb_arg_obj == (void *)0) "
164-
" \n"
165-
" return; // nil is ok "
166-
" (int)strlen(obj->isa->name); "
167-
" \n"
168-
"} "
169-
" \n",
170-
name);
130+
int strformatsize =
131+
snprintf(&buf->contents[0], sizeof(buf->contents),
132+
"struct __objc_class "
133+
" \n"
134+
"{ "
135+
" \n"
136+
" struct __objc_class *isa; "
137+
" \n"
138+
" struct __objc_class *super_class; "
139+
" \n"
140+
" const char *name; "
141+
" \n"
142+
" // rest of struct elided because unused "
143+
" \n"
144+
"}; "
145+
" \n"
146+
" "
147+
" \n"
148+
"struct __objc_object "
149+
" \n"
150+
"{ "
151+
" \n"
152+
" struct __objc_class *isa; "
153+
" \n"
154+
"}; "
155+
" \n"
156+
" "
157+
" \n"
158+
"extern \"C\" void "
159+
" \n"
160+
"%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector) "
161+
" \n"
162+
"{ "
163+
" \n"
164+
" struct __objc_object *obj = (struct "
165+
"__objc_object*)$__lldb_arg_obj; \n"
166+
" if ($__lldb_arg_obj == (void *)0) "
167+
" \n"
168+
" return; // nil is ok "
169+
" (int)strlen(obj->isa->name); "
170+
" \n"
171+
"} "
172+
" \n",
173+
name.c_str());
171174
assert(strformatsize < (int)sizeof(buf->contents));
172175
(void)strformatsize;
173176

174-
Status error;
175-
return GetTargetRef().GetUtilityFunctionForLanguage(
176-
buf->contents, eLanguageTypeObjC, name, error);
177+
return GetTargetRef().CreateUtilityFunction(buf->contents, std::move(name),
178+
eLanguageTypeC, exe_ctx);
177179
}
178180

179181
AppleObjCRuntimeV1::ClassDescriptorV1::ClassDescriptorV1(

‎lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ class AppleObjCRuntimeV1 : public AppleObjCRuntime {
9797
Address &address,
9898
Value::ValueType &value_type) override;
9999

100-
UtilityFunction *CreateObjectChecker(const char *) override;
100+
llvm::Expected<std::unique_ptr<UtilityFunction>>
101+
CreateObjectChecker(std::string, ExecutionContext &exe_ctx) override;
101102

102103
// PluginInterface protocol
103104
ConstString GetPluginName() override;

‎lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Lines changed: 31 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,9 @@ AppleObjCRuntimeV2::CreateExceptionResolver(const BreakpointSP &bkpt,
840840
return resolver_sp;
841841
}
842842

843-
UtilityFunction *AppleObjCRuntimeV2::CreateObjectChecker(const char *name) {
843+
llvm::Expected<std::unique_ptr<UtilityFunction>>
844+
AppleObjCRuntimeV2::CreateObjectChecker(std::string name,
845+
ExecutionContext &exe_ctx) {
844846
char check_function_code[2048];
845847

846848
int len = 0;
@@ -861,7 +863,8 @@ UtilityFunction *AppleObjCRuntimeV2::CreateObjectChecker(const char *name) {
861863
if ($responds == (signed char) 0)
862864
*((volatile int *)0) = 'ocgc';
863865
}
864-
})", name);
866+
})",
867+
name.c_str());
865868
} else {
866869
len = ::snprintf(check_function_code, sizeof(check_function_code), R"(
867870
extern "C" void *gdb_class_getClass(void *);
@@ -881,15 +884,15 @@ UtilityFunction *AppleObjCRuntimeV2::CreateObjectChecker(const char *name) {
881884
if ($responds == (signed char) 0)
882885
*((volatile int *)0) = 'ocgc';
883886
}
884-
})", name);
887+
})",
888+
name.c_str());
885889
}
886890

887891
assert(len < (int)sizeof(check_function_code));
888892
UNUSED_IF_ASSERT_DISABLED(len);
889893

890-
Status error;
891-
return GetTargetRef().GetUtilityFunctionForLanguage(
892-
check_function_code, eLanguageTypeObjC, name, error);
894+
return GetTargetRef().CreateUtilityFunction(check_function_code, name,
895+
eLanguageTypeC, exe_ctx);
893896
}
894897

895898
size_t AppleObjCRuntimeV2::GetByteOffsetForIvar(CompilerType &parent_ast_type,
@@ -1340,8 +1343,6 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapDynamic(
13401343

13411344
Address function_address;
13421345

1343-
DiagnosticManager diagnostics;
1344-
13451346
const uint32_t addr_size = process->GetAddressByteSize();
13461347

13471348
Status err;
@@ -1363,28 +1364,16 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapDynamic(
13631364
FunctionCaller *get_class_info_function = nullptr;
13641365

13651366
if (!m_get_class_info_code) {
1366-
Status error;
1367-
m_get_class_info_code.reset(GetTargetRef().GetUtilityFunctionForLanguage(
1368-
g_get_dynamic_class_info_body, eLanguageTypeObjC,
1369-
g_get_dynamic_class_info_name, error));
1370-
if (error.Fail()) {
1371-
LLDB_LOGF(log,
1372-
"Failed to get Utility Function for implementation lookup: %s",
1373-
error.AsCString());
1374-
m_get_class_info_code.reset();
1375-
} else {
1376-
diagnostics.Clear();
1377-
1378-
if (!m_get_class_info_code->Install(diagnostics, exe_ctx)) {
1379-
if (log) {
1380-
LLDB_LOGF(log, "Failed to install implementation lookup");
1381-
diagnostics.Dump(log);
1382-
}
1383-
m_get_class_info_code.reset();
1384-
}
1385-
}
1386-
if (!m_get_class_info_code)
1367+
auto utility_fn_or_error = GetTargetRef().CreateUtilityFunction(
1368+
g_get_dynamic_class_info_body, g_get_dynamic_class_info_name,
1369+
eLanguageTypeC, exe_ctx);
1370+
if (!utility_fn_or_error) {
1371+
LLDB_LOG_ERROR(
1372+
log, utility_fn_or_error.takeError(),
1373+
"Failed to get utility function for implementation lookup: {0}");
13871374
return DescriptorMapUpdateResult::Fail();
1375+
}
1376+
m_get_class_info_code = std::move(*utility_fn_or_error);
13881377

13891378
// Next make the runner function for our implementation utility function.
13901379
Value value;
@@ -1398,6 +1387,7 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapDynamic(
13981387
arguments.PushValue(value);
13991388
arguments.PushValue(value);
14001389

1390+
Status error;
14011391
get_class_info_function = m_get_class_info_code->MakeFunctionCaller(
14021392
clang_uint32_t_type, arguments, thread_sp, error);
14031393

@@ -1410,17 +1400,13 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapDynamic(
14101400
} else {
14111401
get_class_info_function = m_get_class_info_code->GetFunctionCaller();
14121402
if (!get_class_info_function) {
1413-
if (log) {
1414-
LLDB_LOGF(log, "Failed to get implementation lookup function caller.");
1415-
diagnostics.Dump(log);
1416-
}
1417-
1403+
LLDB_LOGF(log, "Failed to get implementation lookup function caller.");
14181404
return DescriptorMapUpdateResult::Fail();
14191405
}
14201406
arguments = get_class_info_function->GetArgumentValues();
14211407
}
14221408

1423-
diagnostics.Clear();
1409+
DiagnosticManager diagnostics;
14241410

14251411
const uint32_t class_info_byte_size = addr_size + 4;
14261412
const uint32_t class_infos_byte_size = num_classes * class_info_byte_size;
@@ -1600,8 +1586,6 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapSharedCache() {
16001586

16011587
Address function_address;
16021588

1603-
DiagnosticManager diagnostics;
1604-
16051589
const uint32_t addr_size = process->GetAddressByteSize();
16061590

16071591
Status err;
@@ -1663,29 +1647,17 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapSharedCache() {
16631647

16641648
shared_class_expression += g_get_shared_cache_class_info_body;
16651649

1666-
m_get_shared_cache_class_info_code.reset(
1667-
GetTargetRef().GetUtilityFunctionForLanguage(
1668-
shared_class_expression.c_str(), eLanguageTypeObjC,
1669-
g_get_shared_cache_class_info_name, error));
1670-
if (error.Fail()) {
1671-
LLDB_LOGF(log,
1672-
"Failed to get Utility function for implementation lookup: %s.",
1673-
error.AsCString());
1674-
m_get_shared_cache_class_info_code.reset();
1675-
} else {
1676-
diagnostics.Clear();
1677-
1678-
if (!m_get_shared_cache_class_info_code->Install(diagnostics, exe_ctx)) {
1679-
if (log) {
1680-
LLDB_LOGF(log, "Failed to install implementation lookup.");
1681-
diagnostics.Dump(log);
1682-
}
1683-
m_get_shared_cache_class_info_code.reset();
1684-
}
1650+
auto utility_fn_or_error = exe_ctx.GetTargetRef().CreateUtilityFunction(
1651+
std::move(shared_class_expression), g_get_shared_cache_class_info_name,
1652+
eLanguageTypeC, exe_ctx);
1653+
if (!utility_fn_or_error) {
1654+
LLDB_LOG_ERROR(
1655+
log, utility_fn_or_error.takeError(),
1656+
"Failed to get utility function for implementation lookup: {0}");
1657+
return DescriptorMapUpdateResult::Fail();
16851658
}
16861659

1687-
if (!m_get_shared_cache_class_info_code)
1688-
return DescriptorMapUpdateResult::Fail();
1660+
m_get_shared_cache_class_info_code = std::move(*utility_fn_or_error);
16891661

16901662
// Next make the function caller for our implementation utility function.
16911663
Value value;
@@ -1714,7 +1686,7 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapSharedCache() {
17141686
arguments = get_shared_cache_class_info_function->GetArgumentValues();
17151687
}
17161688

1717-
diagnostics.Clear();
1689+
DiagnosticManager diagnostics;
17181690

17191691
const uint32_t class_info_byte_size = addr_size + 4;
17201692
const uint32_t class_infos_byte_size = num_classes * class_info_byte_size;

‎lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ class AppleObjCRuntimeV2 : public AppleObjCRuntime {
5353
Address &address,
5454
Value::ValueType &value_type) override;
5555

56-
UtilityFunction *CreateObjectChecker(const char *) override;
56+
llvm::Expected<std::unique_ptr<UtilityFunction>>
57+
CreateObjectChecker(std::string name, ExecutionContext &exe_ctx) override;
5758

5859
// PluginInterface protocol
5960
ConstString GetPluginName() override;

0 commit comments

Comments
 (0)