Use static_pointer_cast to do SyntheticChildrenSP -> ScriptedSyntheticChildrenSP (#216181) - #216203
Use static_pointer_cast to do SyntheticChildrenSP -> ScriptedSyntheticChildrenSP (#216181)#216203jimingham wants to merge 1 commit into
Conversation
…cChildrenSP (llvm#216181) FormatManager::GetSyntheticForType was taking a pointer out one shared pointer and making a new shared pointer referring to it which messes up the lifecycle of the object. This is just a little thinko from the original implementation. We do the same thing in several other places in the TypeCategory, etc. and it's done correctly in all the other places. I'm not adding a test here because trying to guess what you have to do to cause one or the other shared_pointer to get their reference count to 0 isn't particularly stable. The testing for SBTypeSynthetic is pretty minimal - it would be better to write a complete test for this class, which would have tripped this. But that's a bigger task, and I want to get this obvious crasher fix in now. This fixes: github.com/llvm/issues/213628 (cherry picked from commit ae3ea1b)
|
@llvm/pr-subscribers-lldb Author: jimingham ChangesFormatManager::GetSyntheticForType was taking a pointer out one shared pointer and making a new shared pointer referring to it which messes up the lifecycle of the object. This is just a little thinko from the original implementation. We do the same thing in several other places in the TypeCategory, etc. and it's done correctly in all the other places. I'm not adding a test here because trying to guess what you have to do to cause one or the other shared_pointer to get their reference count to 0 isn't particularly stable. The testing for SBTypeSynthetic is pretty minimal - it would be better to write a complete test for this class, which would have tripped this. But that's a bigger task, and I want to get this obvious crasher fix in now. This fixes: github.com//issues/213628 (cherry picked from commit ae3ea1b) Full diff: https://github.com/llvm/llvm-project/pull/216203.diff 1 Files Affected:
diff --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp
index 6342fd89cc9be..e9f0e51e34149 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -388,14 +388,14 @@ FormatManager::GetSyntheticForType(lldb::TypeNameSpecifierImplSP type_sp) {
category_sp = GetCategoryAtIndex(category_id);
if (!category_sp->IsEnabled())
continue;
- lldb::ScriptedSyntheticChildrenSP synth_current_sp(
- (ScriptedSyntheticChildren *)category_sp->GetSyntheticForType(type_sp)
- .get());
- if (synth_current_sp &&
+ auto synth_current_sp = category_sp->GetSyntheticForType(type_sp);
+
+ if (synth_current_sp && synth_current_sp->IsScripted() &&
(synth_chosen_sp.get() == nullptr ||
(prio_category > category_sp->GetEnabledPosition()))) {
prio_category = category_sp->GetEnabledPosition();
- synth_chosen_sp = synth_current_sp;
+ synth_chosen_sp =
+ std::static_pointer_cast<ScriptedSyntheticChildren>(synth_current_sp);
}
}
return synth_chosen_sp;
|
|
Thanks. |
|
@JDevlieghere and @adrian-prantl, could one of you review whether this change looks like something we should include on the 23.x release branch? |
3 participants
FormatManager::GetSyntheticForType was taking a pointer out one shared pointer and making a new shared pointer referring to it which messes up the lifecycle of the object.
This is just a little thinko from the original implementation. We do the same thing in several other places in the TypeCategory, etc. and it's done correctly in all the other places.
I'm not adding a test here because trying to guess what you have to do to cause one or the other shared_pointer to get their reference count to 0 isn't particularly stable.
The testing for SBTypeSynthetic is pretty minimal - it would be better to write a complete test for this class, which would have tripped this. But that's a bigger task, and I want to get this obvious crasher fix in now.
This fixes:
github.com//issues/213628
(cherry picked from commit ae3ea1b)