mirror of
https://github.com/sbrow/thor.git
synced 2026-10-25 10:11:55 -04:00
fix: fixed use-after-free in highlight.odin.
This commit is contained in:
@@ -12,6 +12,26 @@
|
|||||||
parsers via a pool or per-thread storage, plugged in behind `open_parser`
|
parsers via a pool or per-thread storage, plugged in behind `open_parser`
|
||||||
without touching call sites. A `TSParser`/`TSQueryCursor` is one-per-thread.
|
without touching call sites. A `TSParser`/`TSQueryCursor` is one-per-thread.
|
||||||
- Full write-up: `GRAMMAR_CACHE_THREADING.md`.
|
- Full write-up: `GRAMMAR_CACHE_THREADING.md`.
|
||||||
|
- [ ] Restore parallel grammar preload (BEHAVIOUR CHANGE in `edb2e64`).
|
||||||
|
`preload_grammars` used to `dlopen`+compile each grammar on its own thread;
|
||||||
|
it is now serial (`grammar()` per lang on the calling thread). The startup
|
||||||
|
dlopen cost is no longer overlapped.
|
||||||
|
- Why serial for now: `grammar()` holds `registry.mu` across the whole
|
||||||
|
dlopen+compile, so naively fanning out `grammar()` calls just queues every
|
||||||
|
worker on that one lock — zero real parallelism.
|
||||||
|
- To bring it back: move `build_grammar` (dlopen + `compile_query`, no shared
|
||||||
|
mutable state) OUTSIDE the lock; take `mu` only to (1) init the registry +
|
||||||
|
snapshot which langs are missing, and (2) briefly publish each finished
|
||||||
|
`^Grammar`. Restore the `thread`-based worker fan-out + `SPALL`
|
||||||
|
init/cleanup hooks that `edb2e64` deleted. Safe because glibc `dlopen`, the
|
||||||
|
heap allocator, and each thread's own temp allocator are all thread-safe.
|
||||||
|
- An `RW_Mutex` does NOT help the parallel-load side (writers stay exclusive);
|
||||||
|
it only helps a future multi-threaded *read* path (concurrent cache-hit
|
||||||
|
`grammar()` lookups during a parallel render). Consider it only if/when
|
||||||
|
render itself goes multi-threaded — then pair rlock hits with off-lock build
|
||||||
|
+ a double-check-and-discard on the miss race (needs a `free_grammar`).
|
||||||
|
- Assumption to preserve: preload completes before any concurrent `grammar()`
|
||||||
|
use (render is single-threaded and runs after preload).
|
||||||
- [ ] instead of `warnings: [dynamic]Error` we should use `warnings: [dynamic; 8]Error`
|
- [ ] instead of `warnings: [dynamic]Error` we should use `warnings: [dynamic; 8]Error`
|
||||||
- when limit reached, the template fails and stops rendering.
|
- when limit reached, the template fails and stops rendering.
|
||||||
- [ ] Brainstorm plugin/tool/(postccs/less/tailwind/sass) support.
|
- [ ] Brainstorm plugin/tool/(postccs/less/tailwind/sass) support.
|
||||||
|
|||||||
@@ -90,9 +90,12 @@ write_escaped :: proc(b: ^strings.Builder, s: string) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// unescape_html returns a caller-owned string (free it with delete). When the
|
||||||
|
// input has no entities it returns a clone; otherwise it returns the builder's
|
||||||
|
// buffer. It must NOT destroy the builder — to_string aliases that buffer, so the
|
||||||
|
// returned string owns it.
|
||||||
unescape_html :: proc(s: string) -> string {
|
unescape_html :: proc(s: string) -> string {
|
||||||
sb := strings.builder_make()
|
sb := strings.builder_make()
|
||||||
defer strings.builder_destroy(&sb)
|
|
||||||
|
|
||||||
start := 0
|
start := 0
|
||||||
for i in 0 ..< len(s) {
|
for i in 0 ..< len(s) {
|
||||||
@@ -119,7 +122,7 @@ unescape_html :: proc(s: string) -> string {
|
|||||||
strings.write_string(&sb, replacement)
|
strings.write_string(&sb, replacement)
|
||||||
start = i + semi + 1
|
start = i + semi + 1
|
||||||
}
|
}
|
||||||
if start == 0 do return s
|
if start == 0 do return strings.clone(s)
|
||||||
if start < len(s) do strings.write_string(&sb, s[start:])
|
if start < len(s) do strings.write_string(&sb, s[start:])
|
||||||
return strings.to_string(sb)
|
return strings.to_string(sb)
|
||||||
}
|
}
|
||||||
@@ -136,6 +139,7 @@ highlight_block :: proc(code: string, lang: string, file_path: string) -> string
|
|||||||
defer ts.parser_delete(parser)
|
defer ts.parser_delete(parser)
|
||||||
|
|
||||||
raw_code := unescape_html(code)
|
raw_code := unescape_html(code)
|
||||||
|
defer delete(raw_code)
|
||||||
raw_c := strings.clone_to_cstring(raw_code)
|
raw_c := strings.clone_to_cstring(raw_code)
|
||||||
defer delete(raw_c)
|
defer delete(raw_c)
|
||||||
|
|
||||||
|
|||||||
@@ -81,3 +81,23 @@ test_highlight_code_wraps_css_block :: proc(t: ^testing.T) {
|
|||||||
"block body should be highlighted",
|
"block body should be highlighted",
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// unescape_html must return a caller-owned allocation the caller frees exactly
|
||||||
|
// once. The buggy version frees its builder internally (defer builder_destroy)
|
||||||
|
// and returns the freed alias, so the caller's delete becomes a double-free that
|
||||||
|
// the test runner's tracking allocator flags.
|
||||||
|
@(test)
|
||||||
|
test_unescape_html_entities_owned :: proc(t: ^testing.T) {
|
||||||
|
got := unescape_html("<a>")
|
||||||
|
defer delete(got)
|
||||||
|
testing.expect_value(t, got, "<a>")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same contract on the no-entity path: the buggy version returns the borrowed
|
||||||
|
// input, so freeing it is a bad free (pointer not from the allocator).
|
||||||
|
@(test)
|
||||||
|
test_unescape_html_passthrough_owned :: proc(t: ^testing.T) {
|
||||||
|
got := unescape_html("plain text")
|
||||||
|
defer delete(got)
|
||||||
|
testing.expect_value(t, got, "plain text")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user