Skip to content

Commit d96e26a

Browse files
CodeZHXSwwbmmm
authored andcommitted
Bugfix: SignalTrace mode has memory access problem (#3032)
* Bugfix: SignalTrace mode has memory access problem
1 parent 1280ac7 commit d96e26a

2 files changed

Lines changed: 12 additions & 18 deletions

File tree

src/bthread/task_tracer.cpp

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232

3333
namespace bthread {
3434

35-
DEFINE_bool(enable_fast_unwind, true, "Whether to enable fast unwind");
3635
DEFINE_uint32(signal_trace_timeout_ms, 50, "Timeout for signal trace in ms");
3736
BUTIL_VALIDATE_GFLAG(signal_trace_timeout_ms, butil::PositiveInteger<uint32_t>);
3837

@@ -356,15 +355,14 @@ void TaskTracer::SignalHandler(int, siginfo_t* info, void* context) {
356355
// Ref has been taken before the signal is sent, so no need to add ref here.
357356
butil::intrusive_ptr<SignalSync> signal_sync(
358357
static_cast<SignalSync*>(info->si_value.sival_ptr), false);
359-
if (NULL == signal_sync || NULL == signal_sync->result) {
358+
if (NULL == signal_sync) {
360359
// The signal is not from Tracer, such as TaskControl, do nothing.
361360
return;
362361
}
363-
Result* result = signal_sync->result;
364362
// Skip the first frame, which is the signal handler itself.
365-
result->frame_count = absl::DefaultStackUnwinder(&result->ips[0], NULL,
366-
arraysize(result->ips),
367-
1, context, NULL);
363+
signal_sync->result.frame_count = absl::DefaultStackUnwinder(signal_sync->result.ips, NULL,
364+
arraysize(signal_sync->result.ips), 1,
365+
context, NULL);
368366
// write() is async-signal-safe.
369367
// Don't care about the return value.
370368
butil::ignore_result(write(signal_sync->pipe_fds[1], "1", 1));
@@ -415,11 +413,9 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
415413

416414
// Each signal trace has an independent SignalSync to
417415
// prevent the previous SignalHandler from affecting the new SignalTrace.
418-
Result result;
419-
butil::intrusive_ptr<SignalSync> signal_sync(new SignalSync(&result));
416+
butil::intrusive_ptr<SignalSync> signal_sync(new SignalSync());
420417
if (!signal_sync->Init()) {
421-
result.SetError("Fail to init SignalSync");
422-
return result;
418+
return Result::MakeErrorResult("Fail to init SignalSync");
423419
}
424420

425421
// Add reference for SignalHandler.
@@ -432,8 +428,7 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
432428
if (errno != EAGAIN || sigqueue_try++ >= 3) {
433429
// Remove reference for SignalHandler.
434430
signal_sync->RemoveRefManually();
435-
result.SetError("Fail to sigqueue: %s", berror());
436-
return result;
431+
return Result::MakeErrorResult("Fail to sigqueue: %s, tid: %d", berror(), tid);
437432
}
438433
}
439434
_inuse_signal_syncs.push_back(signal_sync);
@@ -450,8 +445,7 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
450445
if (FLAGS_signal_trace_timeout_ms > 0) {
451446
timer.stop();
452447
if (timer.m_elapsed() >= FLAGS_signal_trace_timeout_ms) {
453-
result.SetError("Timeout exceed %dms", FLAGS_signal_trace_timeout_ms);
454-
break;
448+
return Result::MakeErrorResult("Timeout exceed %dms", FLAGS_signal_trace_timeout_ms);
455449
}
456450
timeout = (int64_t)FLAGS_signal_trace_timeout_ms - timer.m_elapsed();
457451
}
@@ -462,11 +456,12 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
462456
if (EINTR == errno) {
463457
continue;
464458
}
465-
result.SetError("Fail to poll: %s", berror());
459+
return Result::MakeErrorResult("Fail to poll: %s", berror());
466460
}
467461
break;
468462
}
469-
return result;
463+
464+
return signal_sync->result;
470465
}
471466

472467
} // namespace bthread

src/bthread/task_tracer.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,11 @@ class TaskTracer {
8787

8888
// For signal trace.
8989
struct SignalSync : public butil::SharedObject {
90-
explicit SignalSync(Result* result) : result(result) {}
9190
~SignalSync() override;
9291
bool Init();
9392

9493
int pipe_fds[2]{-1, -1};
95-
Result* result{NULL};
94+
Result result;
9695
};
9796

9897
static TaskStatus WaitForJumping(TaskMeta* m);

0 commit comments

Comments
 (0)