Skip to content

Commit dfe5b5a

Browse files
committed
chore: aggregate pending diagnostics and add cross-lang dataflow fix
1 parent 0fa499d commit dfe5b5a

10 files changed

Lines changed: 411 additions & 72 deletions

File tree

CODE_REVIEW_2026-06-07.md

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
# Code Review — 未提交改动(2026-06-07)
2+
3+
5 个文件改动,方向都对,但**有 4 处必须先修**
4+
5+
## 改动概览
6+
7+
| 文件 | 主题 |
8+
|---|---|
9+
| `build.zig` + `src/ir/llvm_cpp_bridge.cpp` (新) + `src/ir/llvm_safe.zig` | C++ 桥替代 `llvm-as` |
10+
| `src/diag/aggregator.zig` | 同 message 跨函数聚合 |
11+
| `src/pass/analysis/ffi/cross_lang_dataflow.zig` | Orphan 抑制收紧 |
12+
| `src/semantics/language_detector.zig` | metadata 优先的语言检测 |
13+
14+
## 🔴 必改 4 处
15+
16+
### 1. `aggregator.zig:307-352` — 聚合根本不聚合
17+
18+
原诊断仍走到 `return true` 入列,summary 只是**额外加一条**。结果:930 条噪音 → 931 条,比之前还多 1 条。
19+
20+
**修法**:改成延迟分桶——所有诊断先暂存,`flush()` 时按 `(kind+message)` 分桶,超阈值的折叠成 1 条 summary。
21+
22+
### 2. `aggregator.zig:336` — fold 文本是乱码
23+
24+
`"折叠前请款未变更,但是格式很有规律..."` 不是中文,模型幻觉。改成英文:
25+
26+
```
27+
"[aggregated] {s} ×{d} (first: {s})"
28+
```
29+
30+
### 3. `language_detector.zig:151, 157` — 两处栈缓冲区溢出
31+
32+
```zig
33+
var cu_operands: [METADATA_MAX_CU]c.LLVMValueRef = undefined; // 容量 4
34+
c.LLVMGetNamedMetadataOperands(module, "llvm.dbg.cu", &cu_operands);
35+
```
36+
37+
`LLVMGetNamedMetadataOperands` 不接受 buffer 大小,按 `LLVMGetNamedMetadataNumOperands` 实际个数写。`cu_count > 4`(LTO 模块常见)就**栈溢出**。同样的 bug 在 `LLVMGetMDNodeOperands` 那行——DICompileUnit 有 15-20 个字段,容易超 10。
38+
39+
**修法**:改堆分配,按 LLVM 给的 num 取 buffer 大小。
40+
41+
### 4. `cross_lang_dataflow.zig:530` — 同 bug 改了一半
42+
43+
文件里有两处 `funcs_with_frees.contains(alloc.alloc_func)` 用同一过宽启发式。这次只改了 line 953-973,**line 530 还是原样**。把新逻辑套用过去,或者抽成共用函数。
44+
45+
## 🟠 高优先级 4 处
46+
47+
| 位置 | 问题 |
48+
|---|---|
49+
| `language_detector.zig:171, 202` | `"rust"`/`"go"` 子串匹配过宽 —— `"go"` 两字符会误匹配 `"google"``"Lego"`。改成 `startsWith("rustc")` / `startsWith("Go cmd/compile")` |
50+
| `llvm_safe.zig:183-189` | C++ 桥走 `.ll` 路径时仍然 `LLVMCreateMemoryBufferWithContentsOfFile` 多读一次文件,然后 dispose 掉。后缀判断提前到 buffer 创建之前 |
51+
| `llvm_cpp_bridge.cpp:23-30` | `omni_create_llvm_context`/`destroy_llvm_context` 是死代码(Zig 端没调用)。删掉,或在桥里用 LLVM `unwrap()` 宏表达 C API ↔ C++ 的转换意图 |
52+
| `llvm_cpp_bridge.cpp:38-50` | `omni_parse_ir_file` 完全没 null 检查(`path`/`context`/`module_out`),`strdup` OOM 失败时 caller 拿到 nullptr 会二次崩 |
53+
54+
## 🟡 中/低(合入后再清理)
55+
56+
- `llvm_safe.zig:188` 死赋值 `parse_result = 0`
57+
- `aggregator.zig:325` `kind_tag``@tagName(...)` 的静态字符串,不需要 dup
58+
- `build.zig:82` `linkSystemLibrary("c++")` 是 macOS 特定,Linux 上要 `stdc++`
59+
- C++ 桥建议加 `-fno-exceptions -fno-rtti`,函数加 `noexcept`
60+
- `DetectionMethod` 新增 `metadata`,grep 一下确保所有 switch 都覆盖
61+
62+
## 合入顺序
63+
64+
1. 修必改 4 处
65+
2. 修高优先级 4 处
66+
3. 重跑 `/tmp/bun_ll/*.ll`,看 `issue_count` 从 1327 真降到 ~400
67+
4. 跑 noise 回归套件(`rust_ffi`/`gopyjava`/`cscpp`)防 cross_lang 改动回归
68+
5. 拆 4 个 commit:llvm 解耦 / cross_lang 收紧 / metadata 语言检测 / 聚合重写
69+
70+
## C++ 单独回应
71+
72+
C++ 代码审过了,覆盖在高 #7#8 和中低条目里:
73+
- 死代码 + 隐性 C API/C++ ABI 耦合(依赖 `LLVMContextRef` typedef 巧合)
74+
- 全函数缺 null 检查
75+
- `strdup` OOM 无兜底
76+
- `-std=c++17` 没和 LLVM 22 实际要求对齐
77+
- `extern "C"` 不阻断 C++ 异常传播

build.zig

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,27 @@ fn configureLLVM(b: *std.Build, compile: *std.Build.Step.Compile, llvm_path: []c
3030
compile.root_module.addRPath(.{ .cwd_relative = lib });
3131
}
3232

33+
/// Configure a Step.Compile with the C++ bridge source for IR parsing
34+
fn configureCppBridge(b: *std.Build, compile: *std.Build.Step.Compile, llvm_path: []const u8) void {
35+
compile.root_module.addCSourceFile(.{ .file = b.path("src/ir/llvm_cpp_bridge.cpp"), .flags = &.{ "-std=c++17", "-fno-exceptions", "-fno-rtti" } });
36+
compile.root_module.addIncludePath(.{ .cwd_relative = b.pathJoin(&.{ llvm_path, "include" }) });
37+
38+
const target: std.Target = if (compile.root_module.resolved_target) |rt|
39+
rt.result
40+
else
41+
@import("builtin").target;
42+
43+
const cxxlib: []const u8 = switch (target.os.tag) {
44+
.macos, .ios, .tvos, .watchos => "c++",
45+
.linux, .freebsd, .openbsd, .netbsd => "stdc++",
46+
.windows => "",
47+
else => "c++",
48+
};
49+
if (cxxlib.len > 0) {
50+
compile.root_module.linkSystemLibrary(cxxlib, .{});
51+
}
52+
}
53+
3354
/// Build configuration for OmniScope
3455
pub fn build(b: *std.Build) void {
3556
// Parse build options
@@ -62,6 +83,22 @@ pub fn build(b: *std.Build) void {
6283

6384
lib_mod.addIncludePath(.{ .cwd_relative = b.pathJoin(&.{ llvm_path, "include" }) });
6485

86+
// Link C++ bridge for direct LLVM IR parsing (llvm::parseIRFile)
87+
// Replaces the external llvm-as dependency that fails in macOS sandbox.
88+
// Added to lib_mod so all test targets that import OmniScope get it.
89+
lib_mod.addCSourceFile(.{ .file = b.path("src/ir/llvm_cpp_bridge.cpp"), .flags = &.{ "-std=c++17", "-fno-exceptions", "-fno-rtti" } });
90+
91+
// Cross-platform C++ standard library linking
92+
const cxxlib: []const u8 = switch (target.result.os.tag) {
93+
.macos, .ios, .tvos, .watchos => "c++",
94+
.linux, .freebsd, .openbsd, .netbsd => "stdc++",
95+
.windows => "", // MSVC links automatically
96+
else => "c++",
97+
};
98+
if (cxxlib.len > 0) {
99+
lib_mod.linkSystemLibrary(cxxlib, .{});
100+
}
101+
65102
// Build main executable
66103
const exe = b.addExecutable(.{
67104
.name = "OmniScope",

src/diag/aggregator.zig

Lines changed: 142 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,15 @@ pub const DiagnosticAggregator = struct {
9292
/// Pattern aggregation: tracks (issue_kind, pattern_base) → count
9393
pattern_counts: std.AutoHashMap(u64, PatternInfo),
9494

95+
/// Pending diagnostics for deferred bucketing in flush()
96+
pending: std.ArrayList(PendingDiag),
97+
9598
/// Threshold for pattern folding (fold when count exceeds this)
9699
const PATTERN_FOLD_THRESHOLD: usize = 3;
97100

101+
/// Threshold for message-based folding (same kind+message across functions)
102+
const MESSAGE_FOLD_THRESHOLD: usize = 5;
103+
98104
/// Pattern information for aggregation
99105
const PatternInfo = struct {
100106
kind_tag: []const u8,
@@ -104,13 +110,24 @@ pub const DiagnosticAggregator = struct {
104110
last_func_name: []const u8,
105111
};
106112

113+
/// Pending diagnostic for deferred bucketing in flush()
114+
const PendingDiag = struct {
115+
kind_tag: []const u8,
116+
message: []const u8,
117+
func_name: []const u8,
118+
severity: OutputSeverity,
119+
loc: u32,
120+
confidence: f32,
121+
};
122+
107123
/// Create a new diagnostic aggregator
108124
pub fn init(allocator: std.mem.Allocator) !DiagnosticAggregator {
109125
return .{
110126
.allocator = allocator,
111127
.diagnostics = try std.ArrayList(Diagnostic).initCapacity(allocator, 0),
112128
.seen_keys = std.AutoHashMap(u64, void).init(allocator),
113129
.pattern_counts = std.AutoHashMap(u64, PatternInfo).init(allocator),
130+
.pending = try std.ArrayList(PendingDiag).initCapacity(allocator, 0),
114131
};
115132
}
116133

@@ -129,6 +146,8 @@ pub const DiagnosticAggregator = struct {
129146
self.allocator.free(entry.value_ptr.last_func_name);
130147
}
131148
self.pattern_counts.deinit();
149+
150+
self.pending.deinit(self.allocator);
132151
}
133152

134153
/// Add a diagnostic with cross-pass deduplication
@@ -207,9 +226,6 @@ pub const DiagnosticAggregator = struct {
207226
else
208227
0.5;
209228

210-
// Map issue kind to diagnostic kind (preserve semantic info)
211-
const diag_kind = mapKindToDiagnostic(kind_tag);
212-
213229
// Preserve location info when available.
214230
// Handle both u32 and ?u32 for line field (test compatibility).
215231
const loc_id: u32 = blk: {
@@ -226,15 +242,19 @@ pub const DiagnosticAggregator = struct {
226242
}
227243
};
228244

229-
try self.add(.{
230-
.kind = diag_kind,
231-
.severity = if (conf >= 0.8) .err else if (conf >= 0.5) .warning else .info,
245+
const severity: OutputSeverity = if (conf >= 0.8) .err else if (conf >= 0.5) .warning else .info;
246+
247+
// Push to pending list for deferred bucketing in flush()
248+
try self.pending.append(self.allocator, .{
249+
.kind_tag = kind_tag,
250+
.message = try self.allocator.dupe(u8, msg),
251+
.func_name = try self.allocator.dupe(u8, func_name),
252+
.severity = severity,
232253
.loc = loc_id,
233-
.message = msg,
234254
.confidence = conf,
235255
});
236256

237-
// Pattern-based aggregation: detect and fold repetitive patterns
257+
// Pattern-based aggregation: detect and track repetitive patterns
238258
// (e.g., ffi_alloc_1, ffi_alloc_2, ... ffi_alloc_20)
239259
if (extractPatternBase(func_name)) |pattern_base| {
240260
const pkey = patternHashKey(kind_tag, pattern_base);
@@ -256,34 +276,126 @@ pub const DiagnosticAggregator = struct {
256276
// Free old last_func_name before updating
257277
self.allocator.free(pattern_gop.value_ptr.last_func_name);
258278
pattern_gop.value_ptr.last_func_name = try self.allocator.dupe(u8, func_name);
279+
}
280+
}
259281

260-
// Check if we should generate a folded summary
261-
if (pattern_gop.value_ptr.count == PATTERN_FOLD_THRESHOLD + 1) {
282+
return true;
283+
}
284+
285+
/// Flush pending diagnostics with deferred bucketing and aggregation.
286+
///
287+
/// Processes the pending list accumulated by addIssue():
288+
/// 1. Emits folded diagnostics for patterns exceeding PATTERN_FOLD_THRESHOLD
289+
/// 2. Groups pending diagnostics by (kind_tag + message) hash key
290+
/// 3. For groups > MESSAGE_FOLD_THRESHOLD, emits ONE aggregated diagnostic
291+
/// 4. For groups <= MESSAGE_FOLD_THRESHOLD, emits each diagnostic individually
292+
/// 5. Clears the pending list
293+
pub fn flush(self: *DiagnosticAggregator) !void {
294+
// 1. Emit pattern-folded diagnostics for patterns exceeding threshold
295+
{
296+
var it = self.pattern_counts.iterator();
297+
while (it.next()) |entry| {
298+
if (entry.value_ptr.count > PATTERN_FOLD_THRESHOLD) {
262299
const fold_msg = try std.fmt.allocPrint(
263300
self.allocator,
264301
"[{s}×{d}] {s}{{1..{d}}} — {d} identical patterns",
265302
.{
266-
kind_tag,
267-
pattern_gop.value_ptr.count,
268-
pattern_gop.value_ptr.pattern_base,
269-
pattern_gop.value_ptr.count,
270-
pattern_gop.value_ptr.count,
303+
entry.value_ptr.kind_tag,
304+
entry.value_ptr.count,
305+
entry.value_ptr.pattern_base,
306+
entry.value_ptr.count,
307+
entry.value_ptr.count,
271308
},
272309
);
273310
defer self.allocator.free(fold_msg);
274311

275312
try self.add(.{
276-
.kind = diag_kind,
277-
.severity = if (conf >= 0.8) .err else if (conf >= 0.5) .warning else .info,
278-
.loc = loc_id,
313+
.kind = mapKindToDiagnostic(entry.value_ptr.kind_tag),
314+
.severity = .warning,
315+
.loc = 0,
279316
.message = fold_msg,
280-
.confidence = conf,
317+
.confidence = 0.8,
281318
});
282319
}
283320
}
284321
}
285322

286-
return true;
323+
// If nothing pending, nothing more to do
324+
if (self.pending.items.len == 0) return;
325+
326+
// 2. Build group counts by (kind_tag + message) hash key
327+
var group_counts = std.AutoHashMap(u64, usize).init(self.allocator);
328+
defer group_counts.deinit();
329+
330+
for (self.pending.items) |diag| {
331+
var hasher = std.hash.Fnv1a_64.init();
332+
hasher.update(diag.kind_tag);
333+
hasher.update(diag.message);
334+
const key = hasher.final();
335+
336+
const gop = try group_counts.getOrPut(key);
337+
if (!gop.found_existing) {
338+
gop.value_ptr.* = 1;
339+
} else {
340+
gop.value_ptr.* += 1;
341+
}
342+
}
343+
344+
// 3. Track which aggregated groups have already been emitted
345+
var emitted_groups = std.AutoHashMap(u64, void).init(self.allocator);
346+
defer emitted_groups.deinit();
347+
348+
// 4. Iterate pending and emit individual or aggregated diagnostics
349+
for (self.pending.items) |diag| {
350+
var hasher = std.hash.Fnv1a_64.init();
351+
hasher.update(diag.kind_tag);
352+
hasher.update(diag.message);
353+
const key = hasher.final();
354+
355+
const count = group_counts.get(key).?;
356+
357+
if (count > MESSAGE_FOLD_THRESHOLD) {
358+
// Emit aggregated diagnostic once per group
359+
const gop = try emitted_groups.getOrPut(key);
360+
if (!gop.found_existing) {
361+
const fold_msg = try std.fmt.allocPrint(
362+
self.allocator,
363+
"[aggregated] {s} ×{d} ({s} and {d} other functions)",
364+
.{
365+
diag.kind_tag,
366+
count,
367+
diag.func_name,
368+
count - 1,
369+
},
370+
);
371+
defer self.allocator.free(fold_msg);
372+
373+
try self.add(.{
374+
.kind = mapKindToDiagnostic(diag.kind_tag),
375+
.severity = diag.severity,
376+
.loc = diag.loc,
377+
.message = fold_msg,
378+
.confidence = diag.confidence,
379+
});
380+
}
381+
} else {
382+
// Emit individually
383+
try self.add(.{
384+
.kind = mapKindToDiagnostic(diag.kind_tag),
385+
.severity = diag.severity,
386+
.loc = diag.loc,
387+
.message = diag.message,
388+
.confidence = diag.confidence,
389+
});
390+
}
391+
}
392+
393+
// 5. Clear pending list
394+
for (self.pending.items) |diag| {
395+
self.allocator.free(diag.message);
396+
self.allocator.free(diag.func_name);
397+
}
398+
self.pending.clearRetainingCapacity();
287399
}
288400

289401
fn mapKindToDiagnostic(kind_str: []const u8) DiagnosticKind {
@@ -453,6 +565,13 @@ pub const DiagnosticAggregator = struct {
453565
self.allocator.free(entry.value_ptr.last_func_name);
454566
}
455567
self.pattern_counts.clearRetainingCapacity();
568+
569+
// Free pending items and clear
570+
for (self.pending.items) |diag| {
571+
self.allocator.free(diag.message);
572+
self.allocator.free(diag.func_name);
573+
}
574+
self.pending.clearRetainingCapacity();
456575
}
457576

458577
/// Extract pattern base from function name by detecting numeric suffixes.
@@ -899,6 +1018,9 @@ test "DiagnosticAggregator - pattern aggregation" {
8991018
_ = try aggregator.addIssue(issue);
9001019
}
9011020

1021+
// Flush pending to process pattern folding
1022+
try aggregator.flush();
1023+
9021024
// Should have individual issues + 1 folded summary
9031025
const all_diags = aggregator.getAll();
9041026

0 commit comments

Comments
 (0)