Bugfix: SignalTrace mode has memory access problem (#3031)

* Bugfix: SignalTrace mode has memory access problem

Author: CodeZHXS

src/bthread/task_tracer.cpp

@@ -356,15 +356,18 @@ void TaskTracer::SignalHandler(int, siginfo_t* info, void* context) {
     // Ref has been taken before the signal is sent, so no need to add ref here.
     butil::intrusive_ptr<SignalSync> signal_sync(
         static_cast<SignalSync*>(info->si_value.sival_ptr), false);
-    if (NULL == signal_sync || NULL == signal_sync->result) {
+    if (NULL == signal_sync) {
         // The signal is not from Tracer, such as TaskControl, do nothing.
         return;
     }
-    Result* result = signal_sync->result;
+    void *pcs[Result::MAX_TRACE_NUM];
     // Skip the first frame, which is the signal handler itself.
-    result->frame_count = absl::DefaultStackUnwinder(&result->ips[0], NULL,
-                                                     arraysize(result->ips),
-                                                     1, context, NULL);
+    int depth = absl::DefaultStackUnwinder(pcs, NULL, arraysize(pcs), 1,
+                                           context, NULL);
+    signal_sync->pcs.reserve(depth);
+    for (int i = 0; i < depth; ++i) {
+        signal_sync->pcs.push_back(pcs[i]);
+    }
     // write() is async-signal-safe.
     // Don't care about the return value.
     butil::ignore_result(write(signal_sync->pipe_fds[1], "1", 1));
@@ -415,11 +418,9 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
 
     // Each signal trace has an independent SignalSync to
     // prevent the previous SignalHandler from affecting the new SignalTrace.
-    Result result;
-    butil::intrusive_ptr<SignalSync> signal_sync(new SignalSync(&result));
+    butil::intrusive_ptr<SignalSync> signal_sync(new SignalSync());
     if (!signal_sync->Init()) {
-        result.SetError("Fail to init SignalSync");
-        return result;
+        return Result::MakeErrorResult("Fail to init SignalSync");
     }
 
     // Add reference for SignalHandler.
@@ -432,8 +433,7 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
         if (errno != EAGAIN || sigqueue_try++ >= 3) {
             // Remove reference for SignalHandler.
             signal_sync->RemoveRefManually();
-            result.SetError("Fail to sigqueue: %s", berror());
-            return result;
+            return Result::MakeErrorResult("Fail to sigqueue: %s, tid: %d", berror(), tid);
         }
     }
     _inuse_signal_syncs.push_back(signal_sync);
@@ -450,8 +450,7 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
         if (FLAGS_signal_trace_timeout_ms > 0) {
             timer.stop();
             if (timer.m_elapsed() >= FLAGS_signal_trace_timeout_ms) {
-                result.SetError("Timeout exceed %dms", FLAGS_signal_trace_timeout_ms);
-                break;
+                return Result::MakeErrorResult("Timeout exceed %dms", FLAGS_signal_trace_timeout_ms);
             }
             timeout = (int64_t)FLAGS_signal_trace_timeout_ms - timer.m_elapsed();
         }
@@ -462,10 +461,17 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
             if (EINTR == errno) {
                 continue;
             }
-            result.SetError("Fail to poll: %s", berror());
+            return Result::MakeErrorResult("Fail to poll: %s", berror());
         }
         break;
     }
+    
+    Result result;
+    result.frame_count = signal_sync->pcs.size();
+    for(size_t i = 0; i < result.frame_count; ++i) {
+        result.ips[i] = signal_sync->pcs[i];
+    }
+    
     return result;
 }
 

src/bthread/task_tracer.h

@@ -87,12 +87,12 @@ class TaskTracer {
 
     // For signal trace.
     struct SignalSync : public butil::SharedObject {
-        explicit SignalSync(Result* result) : result(result) {}
+        explicit SignalSync() {}
         ~SignalSync() override;
         bool Init();
 
         int pipe_fds[2]{-1, -1};
-        Result* result{NULL};
+        std::vector<void *> pcs;
     };
 
     static TaskStatus WaitForJumping(TaskMeta* m);