Skip to content

[lldb] Use std::make_shared for StopInfoSP #149612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 18, 2025

Conversation

JDevlieghere
Copy link
Member

Use std::make_shared to create a StopInfoSP, which inherits from shared_from_this. It's both the most efficient and safest way to create these objects:

  • With make_shared, the object and the control block are allocated together, which is more efficient.
  • With make_shared, the enable_shared_from_this base class is properly linked to the control block before the constructor finishes, so shared_from_this() will be safe to use (though still not recommended during construction).

@JDevlieghere JDevlieghere requested a review from jimingham July 18, 2025 22:57
@llvmbot llvmbot added the lldb label Jul 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Use std::make_shared to create a StopInfoSP, which inherits from shared_from_this. It's both the most efficient and safest way to create these objects:

  • With make_shared, the object and the control block are allocated together, which is more efficient.
  • With make_shared, the enable_shared_from_this base class is properly linked to the control block before the constructor finishes, so shared_from_this() will be safe to use (though still not recommended during construction).

Full diff: https://github.com/llvm/llvm-project/pull/149612.diff

1 Files Affected:

  • (modified) lldb/source/Target/StopInfo.cpp (+18-17)
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 19f89b8246926..240e75a414c63 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -851,8 +851,9 @@ class StopInfoWatchpoint : public StopInfo {
       // We have to step over the watchpoint before we know what to do:   
       StopInfoWatchpointSP me_as_siwp_sp 
           = std::static_pointer_cast<StopInfoWatchpoint>(shared_from_this());
-      ThreadPlanSP step_over_wp_sp(new ThreadPlanStepOverWatchpoint(
-          *(thread_sp.get()), me_as_siwp_sp, wp_sp));
+      ThreadPlanSP step_over_wp_sp =
+          std::make_shared<ThreadPlanStepOverWatchpoint>(*(thread_sp.get()),
+                                                         me_as_siwp_sp, wp_sp);
       // When this plan is done we want to stop, so set this as a Controlling
       // plan.    
       step_over_wp_sp->SetIsControllingPlan(true);
@@ -1475,13 +1476,13 @@ StopInfoSP StopInfo::CreateStopReasonWithBreakpointSiteID(Thread &thread,
                                                           break_id_t break_id) {
   thread.SetThreadHitBreakpointSite();
 
-  return StopInfoSP(new StopInfoBreakpoint(thread, break_id));
+  return std::make_shared<StopInfoBreakpoint>(thread, break_id);
 }
 
 StopInfoSP StopInfo::CreateStopReasonWithBreakpointSiteID(Thread &thread,
                                                           break_id_t break_id,
                                                           bool should_stop) {
-  return StopInfoSP(new StopInfoBreakpoint(thread, break_id, should_stop));
+  return std::make_shared<StopInfoBreakpoint>(thread, break_id, should_stop);
 }
 
 // LWP_TODO: We'll need a CreateStopReasonWithWatchpointResourceID akin
@@ -1489,20 +1490,20 @@ StopInfoSP StopInfo::CreateStopReasonWithBreakpointSiteID(Thread &thread,
 StopInfoSP StopInfo::CreateStopReasonWithWatchpointID(Thread &thread,
                                                       break_id_t watch_id,
                                                       bool silently_continue) {
-  return StopInfoSP(
-      new StopInfoWatchpoint(thread, watch_id, silently_continue));
+  return std::make_shared<StopInfoWatchpoint>(thread, watch_id,
+                                              silently_continue);
 }
 
 StopInfoSP StopInfo::CreateStopReasonWithSignal(Thread &thread, int signo,
                                                 const char *description,
                                                 std::optional<int> code) {
   thread.GetProcess()->GetUnixSignals()->IncrementSignalHitCount(signo);
-  return StopInfoSP(new StopInfoUnixSignal(thread, signo, description, code));
+  return std::make_shared<StopInfoUnixSignal>(thread, signo, description, code);
 }
 
 StopInfoSP StopInfo::CreateStopReasonWithInterrupt(Thread &thread, int signo,
                                                    const char *description) {
-  return StopInfoSP(new StopInfoInterrupt(thread, signo, description));
+  return std::make_shared<StopInfoInterrupt>(thread, signo, description);
 }
 
 StopInfoSP StopInfo::CreateStopReasonToTrace(Thread &thread) {
@@ -1512,44 +1513,44 @@ StopInfoSP StopInfo::CreateStopReasonToTrace(Thread &thread) {
 StopInfoSP StopInfo::CreateStopReasonWithPlan(
     ThreadPlanSP &plan_sp, ValueObjectSP return_valobj_sp,
     ExpressionVariableSP expression_variable_sp) {
-  return StopInfoSP(new StopInfoThreadPlan(plan_sp, return_valobj_sp,
-                                           expression_variable_sp));
+  return std::make_shared<StopInfoThreadPlan>(plan_sp, return_valobj_sp,
+                                              expression_variable_sp);
 }
 
 StopInfoSP StopInfo::CreateStopReasonWithException(Thread &thread,
                                                    const char *description) {
-  return StopInfoSP(new StopInfoException(thread, description));
+  return std::make_shared<StopInfoException>(thread, description);
 }
 
 StopInfoSP StopInfo::CreateStopReasonProcessorTrace(Thread &thread,
                                                     const char *description) {
-  return StopInfoSP(new StopInfoProcessorTrace(thread, description));
+  return std::make_shared<StopInfoProcessorTrace>(thread, description);
 }
 
 StopInfoSP StopInfo::CreateStopReasonHistoryBoundary(Thread &thread,
                                                      const char *description) {
-  return StopInfoSP(new StopInfoHistoryBoundary(thread, description));
+  return std::make_shared<StopInfoHistoryBoundary>(thread, description);
 }
 
 StopInfoSP StopInfo::CreateStopReasonWithExec(Thread &thread) {
-  return StopInfoSP(new StopInfoExec(thread));
+  return std::make_shared<StopInfoExec>(thread);
 }
 
 StopInfoSP StopInfo::CreateStopReasonFork(Thread &thread,
                                           lldb::pid_t child_pid,
                                           lldb::tid_t child_tid) {
-  return StopInfoSP(new StopInfoFork(thread, child_pid, child_tid));
+  return std::make_shared<StopInfoFork>(thread, child_pid, child_tid);
 }
 
 
 StopInfoSP StopInfo::CreateStopReasonVFork(Thread &thread,
                                            lldb::pid_t child_pid,
                                            lldb::tid_t child_tid) {
-  return StopInfoSP(new StopInfoVFork(thread, child_pid, child_tid));
+  return std::make_shared<StopInfoVFork>(thread, child_pid, child_tid);
 }
 
 StopInfoSP StopInfo::CreateStopReasonVForkDone(Thread &thread) {
-  return StopInfoSP(new StopInfoVForkDone(thread));
+  return std::make_shared<StopInfoVForkDone>(thread);
 }
 
 ValueObjectSP StopInfo::GetReturnValueObject(StopInfoSP &stop_info_sp) {

Use std::make_shared to create a StopInfoSP, which inherits from
shared_from_this. It's both the most efficient and safest way to
create these objects:

  - With make_shared, the object and the control block are
    allocated together, which is more efficient.
  - With make_shared, the enable_shared_from_this base class is properly
    linked to the control block before the constructor finishes, so
    shared_from_this() will be safe to use (though still not recommended
    during construction).
Copy link
Collaborator

@jimingham jimingham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. If there were somewhere that this could be documented so somebody won't make the same mistake, that would be a bonus. But this is fine on its own.

@JDevlieghere
Copy link
Member Author

LGTM. If there were somewhere that this could be documented so somebody won't make the same mistake, that would be a bonus. But this is fine on its own.

I think the preference for make_shared and make_unique is fairly universally accepted. The code here looks like legacy code that likely just predated that.

@JDevlieghere JDevlieghere merged commit 68fd102 into llvm:main Jul 18, 2025
9 checks passed
@JDevlieghere JDevlieghere deleted the use-make-shared branch July 18, 2025 23:15
@jimingham
Copy link
Collaborator

Sounds good. A lot of this code was written before C++11 was available, so that's to be expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants