Skip to content

Commit 2e2ef7b

Browse files
Timwood0x10Timwood0x10
committed
refactor(analysis): fix memory leak in issue reporting and cleanup
1 parent f738d42 commit 2e2ef7b

3 files changed

Lines changed: 62 additions & 26 deletions

File tree

‎src/pass/analysis/callback_escape.zig‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -528,10 +528,19 @@ pub const CallbackEscapePass = struct {
528528
}
529529
if (cgo_ptr_val != 0) {
530530
if (!ctx.isRelevantAlloc(cgo_ptr_val)) continue;
531-
// Generate candidate instead of direct reporting
532531
var candidate = try cb_report.generateCBytesEscapeCandidate(ctx, func_name, call, diag);
533532
defer candidate.deinit();
534-
// Verify candidate through IssueVerifier
533+
534+
const reason_msg = if (candidate.reason) |r|
535+
try ctx.allocator.dupe(u8, r)
536+
else
537+
"CBytes escape detected";
538+
errdefer {
539+
if (candidate.reason != null) {
540+
ctx.allocator.free(reason_msg);
541+
}
542+
}
543+
535544
if (ctx.issue_verifier) |verifier| {
536545
const result = try verifier.verify(&candidate);
537546
if (result.shouldReport()) {
@@ -542,10 +551,9 @@ pub const CallbackEscapePass = struct {
542551
.low => .low,
543552
else => .medium,
544553
};
545-
// Convert candidate to Issue and add to context
546554
const issue = Issue.initWithTrace(
547555
.memory_leak,
548-
candidate.reason orelse "CBytes escape detected",
556+
reason_msg,
549557
Location.init(func_name),
550558
sev,
551559
result.adjusted_score,
@@ -554,13 +562,12 @@ pub const CallbackEscapePass = struct {
554562
try ctx.addIssue(&issue);
555563
}
556564
} else {
557-
// Legacy mode: direct reporting
558565
const issue = Issue.initWithTrace(
559566
.memory_leak,
560-
candidate.reason orelse "CBytes escape detected",
567+
reason_msg,
561568
Location.init(func_name),
562569
.medium,
563-
0.65, // med_cbytes_escape confidence
570+
0.65,
564571
&[_]TraceEntry{},
565572
);
566573
try ctx.addIssue(&issue);
@@ -571,10 +578,19 @@ pub const CallbackEscapePass = struct {
571578
}
572579

573580
if (isUnsafePtrConversion(call.callee_name)) {
574-
// Generate candidate instead of direct reporting
575581
var candidate = try cb_report.generateUnsafePtrRiskCandidate(ctx, func_name, call, diag);
576582
defer candidate.deinit();
577-
// Verify candidate through IssueVerifier
583+
584+
const reason_msg = if (candidate.reason) |r|
585+
try ctx.allocator.dupe(u8, r)
586+
else
587+
"Unsafe pointer risk detected";
588+
errdefer {
589+
if (candidate.reason != null) {
590+
ctx.allocator.free(reason_msg);
591+
}
592+
}
593+
578594
if (ctx.issue_verifier) |verifier| {
579595
const result = try verifier.verify(&candidate);
580596
if (result.shouldReport()) {
@@ -585,10 +601,9 @@ pub const CallbackEscapePass = struct {
585601
.low => .low,
586602
else => .medium,
587603
};
588-
// Convert candidate to Issue and add to context
589604
const issue = Issue.initWithTrace(
590605
.borrow_escape,
591-
candidate.reason orelse "Unsafe pointer risk detected",
606+
reason_msg,
592607
Location.init(func_name),
593608
sev2,
594609
result.adjusted_score,
@@ -597,13 +612,12 @@ pub const CallbackEscapePass = struct {
597612
try ctx.addIssue(&issue);
598613
}
599614
} else {
600-
// Legacy mode: direct reporting
601615
const issue = Issue.initWithTrace(
602616
.borrow_escape,
603-
candidate.reason orelse "Unsafe pointer risk detected",
617+
reason_msg,
604618
Location.init(func_name),
605619
.high,
606-
0.72, // med_unsafe_ptr confidence
620+
0.72,
607621
&[_]TraceEntry{},
608622
);
609623
try ctx.addIssue(&issue);
@@ -700,10 +714,19 @@ pub const CallbackEscapePass = struct {
700714
}
701715
}
702716
if (!has_call_ret_transfer) {
703-
// Generate candidate instead of direct reporting
704717
var candidate = try cb_report.generateMallocLeakCandidate(ctx, func_name, pair_result.malloc_count, pair_result.free_count, diag);
705718
defer candidate.deinit();
706-
// Verify candidate through IssueVerifier
719+
720+
const reason_msg = if (candidate.reason) |r|
721+
try ctx.allocator.dupe(u8, r)
722+
else
723+
"Memory leak detected";
724+
errdefer {
725+
if (candidate.reason != null) {
726+
ctx.allocator.free(reason_msg);
727+
}
728+
}
729+
707730
if (ctx.issue_verifier) |verifier| {
708731
const result = try verifier.verify(&candidate);
709732
if (result.shouldReport()) {
@@ -714,10 +737,9 @@ pub const CallbackEscapePass = struct {
714737
.low => .low,
715738
else => .medium,
716739
};
717-
// Convert candidate to Issue and add to context
718740
const issue = Issue.initWithTrace(
719741
.memory_leak,
720-
candidate.reason orelse "Memory leak detected",
742+
reason_msg,
721743
Location.init(func_name),
722744
sev3,
723745
result.adjusted_score,
@@ -726,10 +748,9 @@ pub const CallbackEscapePass = struct {
726748
try ctx.addIssue(&issue);
727749
}
728750
} else {
729-
// Legacy mode: direct reporting
730751
const issue = Issue.initWithTrace(
731752
.memory_leak,
732-
candidate.reason orelse "Memory leak detected",
753+
reason_msg,
733754
Location.init(func_name),
734755
.medium,
735756
Confidence.med_malloc_leak,

‎src/pass/analysis/callback_escape_report.zig‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,18 @@ pub fn reportMallocLeak(
328328
) !void {
329329
const candidate = try generateMallocLeakCandidate(ctx, func_name, malloc_count, free_count, diag);
330330
const location = Location.init(func_name);
331-
var issue = Issue.init(.memory_leak, candidate.reason orelse "Memory leak detected", location, .low, 0.5);
331+
332+
const reason_msg = if (candidate.reason) |r|
333+
try ctx.allocator.dupe(u8, r)
334+
else
335+
"Memory leak detected";
336+
errdefer {
337+
if (candidate.reason != null) {
338+
ctx.allocator.free(reason_msg);
339+
}
340+
}
341+
342+
var issue = Issue.initWithTrace(.memory_leak, reason_msg, location, .low, 0.5, &[_]TraceEntry{});
332343
errdefer issue.deinit(ctx.allocator);
333344
try ctx.addIssue(&issue);
334345
}

‎src/pass/analysis/resource/issue_candidate_builder.zig‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,13 @@ pub const IssueCandidate = struct {
107107
}
108108

109109
pub fn deinit(self: *IssueCandidate) void {
110+
for (self.evidence.items) |item| {
111+
self.allocator.free(item);
112+
}
110113
self.evidence.deinit(self.allocator);
114+
if (self.reason) |r| {
115+
self.allocator.free(r);
116+
}
111117
}
112118

113119
/// Add an evidence item to this candidate.
@@ -351,12 +357,10 @@ pub const CandidateBuilder = struct {
351357
}
352358

353359
/// Take ownership of all candidates (for transfer to verifier).
360+
/// Transfers ownership of the internal ArrayList - caller must deinit returned list.
354361
pub fn takeCandidates(self: *CandidateBuilder) std.ArrayList(IssueCandidate) {
355-
var result = std.ArrayList(IssueCandidate){};
356-
for (self.candidates.items) |c| {
357-
result.append(c) catch {};
358-
}
359-
self.candidates.clearRetainingCapacity();
362+
const result = self.candidates;
363+
self.candidates = .{ .items = &[_]IssueCandidate{}, .capacity = 0 };
360364
return result;
361365
}
362366
};

0 commit comments

Comments
 (0)