Skip to content

Commit 15cd71a

Browse files
labathkuilpd
labath
and
kuilpd
authored
[lldb] Make ValueObject::Dereference less aggressive (#137311)
The function was always trying to dereference both the synthetic and non-synthetic view of the object. This is wrong as the caller should be able to determine which view of the object it wants to access, as is done e.g. for child member access. This patch removes the nonsynthetic->synthetic fallback, which is the more surprising path, and fixes the callers to try both versions of the object (when appropriate). I also snuck in simplification of the member access code path because it was possible to use the same helper function for that, and I wanted to be sure I understand the logic correctly. I've left the synthetic->nonsynthetic fallback in place. I think we may want to keep that one as we often have synthetic child providers for pointer types. They usually don't provide an explicit dereference operation but I think users would expect that a dereference operation on those objects would work. What we may want to do is to try the *synthetic* operation first in this case, so that the nonsynthetic case is really a fallback. --------- Co-authored-by: Ilia Kuklin <[email protected]>
1 parent 679cc0a commit 15cd71a

File tree

2 files changed

+83
-141
lines changed

2 files changed

+83
-141
lines changed

‎lldb/source/Target/StackFrame.cpp‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -709,21 +709,17 @@ ValueObjectSP StackFrame::LegacyGetValueForVariableExpressionPath(
709709
// we have a synthetic dereference specified.
710710
if (!valobj_sp->IsPointerType() && valobj_sp->HasSyntheticValue()) {
711711
Status deref_error;
712-
if (valobj_sp->GetCompilerType().IsReferenceType()) {
713-
valobj_sp = valobj_sp->GetSyntheticValue()->Dereference(deref_error);
714-
if (!valobj_sp || deref_error.Fail()) {
715-
error = Status::FromErrorStringWithFormatv(
716-
"Failed to dereference reference type: {0}", deref_error);
717-
return ValueObjectSP();
718-
}
712+
if (ValueObjectSP synth_deref_sp =
713+
valobj_sp->GetSyntheticValue()->Dereference(deref_error);
714+
synth_deref_sp && deref_error.Success()) {
715+
valobj_sp = std::move(synth_deref_sp);
719716
}
720-
721-
valobj_sp = valobj_sp->Dereference(deref_error);
722717
if (!valobj_sp || deref_error.Fail()) {
723718
error = Status::FromErrorStringWithFormatv(
724719
"Failed to dereference synthetic value: {0}", deref_error);
725720
return ValueObjectSP();
726721
}
722+
727723
// Some synthetic plug-ins fail to set the error in Dereference
728724
if (!valobj_sp) {
729725
error =
@@ -1129,6 +1125,12 @@ ValueObjectSP StackFrame::LegacyGetValueForVariableExpressionPath(
11291125
if (valobj_sp) {
11301126
if (deref) {
11311127
ValueObjectSP deref_valobj_sp(valobj_sp->Dereference(error));
1128+
if (!deref_valobj_sp && !no_synth_child) {
1129+
if (ValueObjectSP synth_obj_sp = valobj_sp->GetSyntheticValue()) {
1130+
error.Clear();
1131+
deref_valobj_sp = synth_obj_sp->Dereference(error);
1132+
}
1133+
}
11321134
valobj_sp = deref_valobj_sp;
11331135
} else if (address_of) {
11341136
ValueObjectSP address_of_valobj_sp(valobj_sp->AddressOf(error));

‎lldb/source/ValueObject/ValueObject.cpp‎

Lines changed: 72 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -2202,6 +2202,45 @@ void ValueObject::GetExpressionPath(Stream &s,
22022202
}
22032203
}
22042204

2205+
// Return the alternate value (synthetic if the input object is non-synthetic
2206+
// and otherwise) this is permitted by the expression path options.
2207+
static ValueObjectSP GetAlternateValue(
2208+
ValueObject &valobj,
2209+
ValueObject::GetValueForExpressionPathOptions::SyntheticChildrenTraversal
2210+
synth_traversal) {
2211+
using SynthTraversal =
2212+
ValueObject::GetValueForExpressionPathOptions::SyntheticChildrenTraversal;
2213+
2214+
if (valobj.IsSynthetic()) {
2215+
if (synth_traversal == SynthTraversal::FromSynthetic ||
2216+
synth_traversal == SynthTraversal::Both)
2217+
return valobj.GetNonSyntheticValue();
2218+
} else {
2219+
if (synth_traversal == SynthTraversal::ToSynthetic ||
2220+
synth_traversal == SynthTraversal::Both)
2221+
return valobj.GetSyntheticValue();
2222+
}
2223+
return nullptr;
2224+
}
2225+
2226+
// Dereference the provided object or the alternate value, if permitted by the
2227+
// expression path options.
2228+
static ValueObjectSP DereferenceValueOrAlternate(
2229+
ValueObject &valobj,
2230+
ValueObject::GetValueForExpressionPathOptions::SyntheticChildrenTraversal
2231+
synth_traversal,
2232+
Status &error) {
2233+
error.Clear();
2234+
ValueObjectSP result = valobj.Dereference(error);
2235+
if (!result || error.Fail()) {
2236+
if (ValueObjectSP alt_obj = GetAlternateValue(valobj, synth_traversal)) {
2237+
error.Clear();
2238+
result = alt_obj->Dereference(error);
2239+
}
2240+
}
2241+
return result;
2242+
}
2243+
22052244
ValueObjectSP ValueObject::GetValueForExpressionPath(
22062245
llvm::StringRef expression, ExpressionPathScanEndReason *reason_to_stop,
22072246
ExpressionPathEndResultType *final_value_type,
@@ -2234,7 +2273,8 @@ ValueObjectSP ValueObject::GetValueForExpressionPath(
22342273
: dummy_final_task_on_target) ==
22352274
ValueObject::eExpressionPathAftermathDereference) {
22362275
Status error;
2237-
ValueObjectSP final_value = ret_val->Dereference(error);
2276+
ValueObjectSP final_value = DereferenceValueOrAlternate(
2277+
*ret_val, options.m_synthetic_children_traversal, error);
22382278
if (error.Fail() || !final_value.get()) {
22392279
if (reason_to_stop)
22402280
*reason_to_stop =
@@ -2348,144 +2388,45 @@ ValueObjectSP ValueObject::GetValueForExpressionPath_Impl(
23482388
temp_expression = temp_expression.drop_front(); // skip . or >
23492389

23502390
size_t next_sep_pos = temp_expression.find_first_of("-.[", 1);
2351-
if (next_sep_pos == llvm::StringRef::npos) // if no other separator just
2352-
// expand this last layer
2353-
{
2391+
if (next_sep_pos == llvm::StringRef::npos) {
2392+
// if no other separator just expand this last layer
23542393
llvm::StringRef child_name = temp_expression;
23552394
ValueObjectSP child_valobj_sp =
23562395
root->GetChildMemberWithName(child_name);
2357-
2358-
if (child_valobj_sp.get()) // we know we are done, so just return
2359-
{
2360-
*reason_to_stop =
2361-
ValueObject::eExpressionPathScanEndReasonEndOfString;
2362-
*final_result = ValueObject::eExpressionPathEndResultTypePlain;
2363-
return child_valobj_sp;
2364-
} else {
2365-
switch (options.m_synthetic_children_traversal) {
2366-
case GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
2367-
None:
2368-
break;
2369-
case GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
2370-
FromSynthetic:
2371-
if (root->IsSynthetic()) {
2372-
child_valobj_sp = root->GetNonSyntheticValue();
2373-
if (child_valobj_sp.get())
2374-
child_valobj_sp =
2375-
child_valobj_sp->GetChildMemberWithName(child_name);
2376-
}
2377-
break;
2378-
case GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
2379-
ToSynthetic:
2380-
if (!root->IsSynthetic()) {
2381-
child_valobj_sp = root->GetSyntheticValue();
2382-
if (child_valobj_sp.get())
2383-
child_valobj_sp =
2384-
child_valobj_sp->GetChildMemberWithName(child_name);
2385-
}
2386-
break;
2387-
case GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
2388-
Both:
2389-
if (root->IsSynthetic()) {
2390-
child_valobj_sp = root->GetNonSyntheticValue();
2391-
if (child_valobj_sp.get())
2392-
child_valobj_sp =
2393-
child_valobj_sp->GetChildMemberWithName(child_name);
2394-
} else {
2395-
child_valobj_sp = root->GetSyntheticValue();
2396-
if (child_valobj_sp.get())
2397-
child_valobj_sp =
2398-
child_valobj_sp->GetChildMemberWithName(child_name);
2399-
}
2400-
break;
2401-
}
2396+
if (!child_valobj_sp) {
2397+
if (ValueObjectSP altroot = GetAlternateValue(
2398+
*root, options.m_synthetic_children_traversal))
2399+
child_valobj_sp = altroot->GetChildMemberWithName(child_name);
24022400
}
2403-
2404-
// if we are here and options.m_no_synthetic_children is true,
2405-
// child_valobj_sp is going to be a NULL SP, so we hit the "else"
2406-
// branch, and return an error
2407-
if (child_valobj_sp.get()) // if it worked, just return
2408-
{
2401+
if (child_valobj_sp) {
24092402
*reason_to_stop =
24102403
ValueObject::eExpressionPathScanEndReasonEndOfString;
24112404
*final_result = ValueObject::eExpressionPathEndResultTypePlain;
24122405
return child_valobj_sp;
2413-
} else {
2414-
*reason_to_stop =
2415-
ValueObject::eExpressionPathScanEndReasonNoSuchChild;
2416-
*final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
2417-
return nullptr;
24182406
}
2419-
} else // other layers do expand
2420-
{
2421-
llvm::StringRef next_separator = temp_expression.substr(next_sep_pos);
2422-
llvm::StringRef child_name = temp_expression.slice(0, next_sep_pos);
2407+
*reason_to_stop = ValueObject::eExpressionPathScanEndReasonNoSuchChild;
2408+
*final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
2409+
return nullptr;
2410+
}
24232411

2424-
ValueObjectSP child_valobj_sp =
2425-
root->GetChildMemberWithName(child_name);
2426-
if (child_valobj_sp.get()) // store the new root and move on
2427-
{
2428-
root = child_valobj_sp;
2429-
remainder = next_separator;
2430-
*final_result = ValueObject::eExpressionPathEndResultTypePlain;
2431-
continue;
2432-
} else {
2433-
switch (options.m_synthetic_children_traversal) {
2434-
case GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
2435-
None:
2436-
break;
2437-
case GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
2438-
FromSynthetic:
2439-
if (root->IsSynthetic()) {
2440-
child_valobj_sp = root->GetNonSyntheticValue();
2441-
if (child_valobj_sp.get())
2442-
child_valobj_sp =
2443-
child_valobj_sp->GetChildMemberWithName(child_name);
2444-
}
2445-
break;
2446-
case GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
2447-
ToSynthetic:
2448-
if (!root->IsSynthetic()) {
2449-
child_valobj_sp = root->GetSyntheticValue();
2450-
if (child_valobj_sp.get())
2451-
child_valobj_sp =
2452-
child_valobj_sp->GetChildMemberWithName(child_name);
2453-
}
2454-
break;
2455-
case GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
2456-
Both:
2457-
if (root->IsSynthetic()) {
2458-
child_valobj_sp = root->GetNonSyntheticValue();
2459-
if (child_valobj_sp.get())
2460-
child_valobj_sp =
2461-
child_valobj_sp->GetChildMemberWithName(child_name);
2462-
} else {
2463-
child_valobj_sp = root->GetSyntheticValue();
2464-
if (child_valobj_sp.get())
2465-
child_valobj_sp =
2466-
child_valobj_sp->GetChildMemberWithName(child_name);
2467-
}
2468-
break;
2469-
}
2470-
}
2412+
llvm::StringRef next_separator = temp_expression.substr(next_sep_pos);
2413+
llvm::StringRef child_name = temp_expression.slice(0, next_sep_pos);
24712414

2472-
// if we are here and options.m_no_synthetic_children is true,
2473-
// child_valobj_sp is going to be a NULL SP, so we hit the "else"
2474-
// branch, and return an error
2475-
if (child_valobj_sp.get()) // if it worked, move on
2476-
{
2477-
root = child_valobj_sp;
2478-
remainder = next_separator;
2479-
*final_result = ValueObject::eExpressionPathEndResultTypePlain;
2480-
continue;
2481-
} else {
2482-
*reason_to_stop =
2483-
ValueObject::eExpressionPathScanEndReasonNoSuchChild;
2484-
*final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
2485-
return nullptr;
2486-
}
2415+
ValueObjectSP child_valobj_sp = root->GetChildMemberWithName(child_name);
2416+
if (!child_valobj_sp) {
2417+
if (ValueObjectSP altroot = GetAlternateValue(
2418+
*root, options.m_synthetic_children_traversal))
2419+
child_valobj_sp = altroot->GetChildMemberWithName(child_name);
24872420
}
2488-
break;
2421+
if (child_valobj_sp) {
2422+
root = child_valobj_sp;
2423+
remainder = next_separator;
2424+
*final_result = ValueObject::eExpressionPathEndResultTypePlain;
2425+
continue;
2426+
}
2427+
*reason_to_stop = ValueObject::eExpressionPathScanEndReasonNoSuchChild;
2428+
*final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
2429+
return nullptr;
24892430
}
24902431
case '[': {
24912432
if (!root_compiler_type_info.Test(eTypeIsArray) &&
@@ -2601,7 +2542,8 @@ ValueObjectSP ValueObject::GetValueForExpressionPath_Impl(
26012542
// a bitfield
26022543
pointee_compiler_type_info.Test(eTypeIsScalar)) {
26032544
Status error;
2604-
root = root->Dereference(error);
2545+
root = DereferenceValueOrAlternate(
2546+
*root, options.m_synthetic_children_traversal, error);
26052547
if (error.Fail() || !root) {
26062548
*reason_to_stop =
26072549
ValueObject::eExpressionPathScanEndReasonDereferencingFailed;
@@ -2746,7 +2688,8 @@ ValueObjectSP ValueObject::GetValueForExpressionPath_Impl(
27462688
ValueObject::eExpressionPathAftermathDereference &&
27472689
pointee_compiler_type_info.Test(eTypeIsScalar)) {
27482690
Status error;
2749-
root = root->Dereference(error);
2691+
root = DereferenceValueOrAlternate(
2692+
*root, options.m_synthetic_children_traversal, error);
27502693
if (error.Fail() || !root) {
27512694
*reason_to_stop =
27522695
ValueObject::eExpressionPathScanEndReasonDereferencingFailed;
@@ -2916,9 +2859,6 @@ ValueObjectSP ValueObject::Dereference(Status &error) {
29162859
}
29172860
}
29182861

2919-
} else if (HasSyntheticValue()) {
2920-
m_deref_valobj =
2921-
GetSyntheticValue()->GetChildMemberWithName("$$dereference$$").get();
29222862
} else if (IsSynthetic()) {
29232863
m_deref_valobj = GetChildMemberWithName("$$dereference$$").get();
29242864
}

0 commit comments

Comments
 (0)