Commit 464c358
Keep the parse-expr fuzz harness from growing without bound (Covered by dco/Brian_Egge.md)
OSS-Fuzz testcase 4850385077207040 reports fuzz-parse-expr running out of
memory, with no minimized reproducer and a 51 byte unminimized one:
'66666\xaa6f.r6766#66c'anotpx.t!=1 \x82.x 0 f+\x99 -A\0"ot?pS
Replayed once it is nothing -- 11ms, a syntax error. Replayed repeatedly it
is the whole story: every read of it leaves the process about 168KB larger
than before, for good. ClusterFuzz could not minimize it because no single
input causes the failure; it is the harness, accumulating.
What accumulates comes from regex literals. The harness's own comment says
"this harness only parses -- nothing is compiled or evaluated", and the
second half is true; the first is not quite. A regex literal is turned into a
matching function where it is read: makeRegexFn determinizes it and defines
the result in the compiler under a fresh name (".regex." + freshName()), and
the types that takes -- the capture buffer, the function's own -- are
interned in the process-wide type memo (tctorMaps in lang/type.C), which
holds a reference of its own to each. Nothing removes the definition, and
only compactMTypeMemory() lets go of the types. Measured per readExpr,
retained, unsanitized:
1+2 0KB
1 + (syntax error) 0KB
'abc' 31KB
'a.b.c.d.e' 108KB
the testcase above 168KB
Nothing but regex literals retains anything, and what a regex retains scales
with its DFA. Where it goes, measured on the testcase in cycles of 100 reads:
100 reads +17.1MB
compactMTypeMemory() -8.9MB (the memo's half)
destroy the cc -15.2MB (its half, and its own)
build a new cc +10.1MB
net per cycle, doing both +0.5MB
So roughly half is the memo and half is the compiler, and neither release
alone is enough: with the compiler alive the memo's half goes but the
compiler's stays, and with the compiler replaced but the memo left alone the
memo's half stays -- which is what a first version of this change did, and
under ASan on Linux it grew almost as fast as no change at all (2512MB against
3169MB at 8000 reads). Doing both holds it.
So the harness now does both, on a count of inputs: compact the memo every
64, as the decoder harnesses do, and replace the compiler every 1024 -- a
fraction of a second each time, amortised over enough inputs not to show.
Counts are a coarse measure of what has accumulated but a portable and
predictable one; resident size is neither, since freed memory is not handed
back to the operating system, and a version that watched RSS rebuilt the
compiler on every input once it first went over.
One detail of the replacement matters. The first compiler is built in the
slot's initializer, not on first use: constructing a cc also constructs the
LLVM statics it depends on, and everything static is destroyed at exit in
reverse order of construction, so a slot registered empty and filled later
would be destroyed after those statics and the cc in it would tear down
against an LLVM context that was already gone. That shape crashed at exit in
LLVMContext::removeModule; this one exits cleanly, as the original did.
Under ASan on Linux, 8000 copies of the testcase, RSS at 1024/2048/4096/8000:
before: 1081 / 1388 / 2001 / 3169 MB -- climbing
after: 915 / 1071 / 1108 / 1146 MB -- levelling off
This bounds the harness. Both halves of the growth are the library's, and a
host that reads regex literals from many distinct sources has the same
exposure: the memo's half is what the decoder harness change (morganstanley#549) also
works around, and the compiler's half -- a compiled matcher per regex literal
read, never reclaimed -- is left as it is, since making them reclaimable is a
design change rather than a fix. Both are worth knowing about.
The checked-in corpus replays cleanly. A local five minute fuzz run of the
new harness found a pre-existing stack overflow in dfaState's recursion on a
regex within the DFA-state cap, which is unrelated to this change and is
being handled separately. README and the harness's header comment updated to
say what the harness actually does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016rGT4C394qeh2DBQhqy3Tb1 parent a340567 commit 464c358
2 files changed
Lines changed: 70 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
57 | 60 | | |
58 | 61 | | |
59 | 62 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
6 | | - | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
7 | 34 | | |
8 | 35 | | |
9 | 36 | | |
10 | 37 | | |
11 | 38 | | |
12 | 39 | | |
| 40 | + | |
13 | 41 | | |
14 | 42 | | |
15 | 43 | | |
16 | 44 | | |
17 | | - | |
18 | | - | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
19 | 56 | | |
20 | 57 | | |
21 | 58 | | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
22 | 80 | | |
23 | 81 | | |
24 | 82 | | |
| |||
28 | 86 | | |
29 | 87 | | |
30 | 88 | | |
| 89 | + | |
31 | 90 | | |
32 | 91 | | |
0 commit comments