RetroSearch Browse

Home - News ( United States | United Kingdom | Italy | Germany ) - Football scores

Showing content from https://github.com/ocaml/ocaml/issues/11508 below:

Bytes.escaped unsafe under concurrent modification · Issue #11508 · ocaml/ocaml · GitHub

There's a safety issue with the current definition of Bytes.escaped:

let escaped s = let n = ref 0 in for i = 0 to length s - 1 do n := !n + (match unsafe_get s i with | '\"' | '\\' | '\n' | '\t' | '\r' | '\b' -> 2 | ' ' .. '~' -> 1 | _ -> 4) done; if !n = length s then copy s else begin let s' = create !n in n := 0; for i = 0 to length s - 1 do begin match unsafe_get s i with | ('\"' | '\\') as c -> unsafe_set s' !n '\\'; incr n; unsafe_set s' !n c | '\n' -> unsafe_set s' !n '\\'; incr n; unsafe_set s' !n 'n' | '\t' -> unsafe_set s' !n '\\'; incr n; unsafe_set s' !n 't' | '\r' -> unsafe_set s' !n '\\'; incr n; unsafe_set s' !n 'r' | '\b' -> unsafe_set s' !n '\\'; incr n; unsafe_set s' !n 'b' | (' ' .. '~') as c -> unsafe_set s' !n c | c -> let a = char_code c in unsafe_set s' !n '\\'; incr n; unsafe_set s' !n (char_chr (48 + a / 100)); incr n; unsafe_set s' !n (char_chr (48 + (a / 10) mod 10)); incr n; unsafe_set s' !n (char_chr (48 + a mod 10)); end; incr n done; s' end

Bytes operations should not crash under concurrent usage as documented in #11237.
However under concurrent modification of the underlying data structure, the above may result in out-of-bounds unsafe_set if more escapable chars are present since the target length n was calculated.

Here's a test case to trigger it:

(*TEST*)

let buf = Bytes.make 128 '0'

let fn_mutate s =
  let c = if Random.bool () then '0' else '\n' in
  Bytes.fill s 0 127 c

let fn_escape s =
  let s' = Bytes.escaped s in
  assert ((Bytes.length s') <= 4*(Bytes.length s))

let _ =
  let iter = Atomic.make 0 in
  let d_mutate = Domain.spawn (fun () ->
        while Atomic.get iter < 100_000; do fn_mutate buf done; ) in
  let d_escape = Domain.spawn (fun () ->
        while Atomic.get iter < 100_000; do
          fn_escape buf; ignore (Atomic.fetch_and_add iter 1) done; ) in

  ignore (Domain.join d_mutate);
  ignore (Domain.join d_escape)

$ ocamlopt -o /tmp/bytescrash.exe /tmp/bytescrash.ml 
$ /tmp/bytescrash.exe 
Fatal error: allocation failure during minor GC
Aborted (core dumped)

I'm creating a issue rather than a PR, to discuss the preferred fix.

one option is to make an early copy of s which will prevent concurrent changes to the copy
another option is to switch from unsafe_set to safe set
a third option is to bound writing to be within the bounds of the output buffer
...

This was a team effort: @ctk21 identified the cause and wrote the independent test case illustrating it. @haesbaert suggested an elegant fix. I just made it crash with a random test... 😅

RetroSearch is an open source project built by @garambo | Open a GitHub Issue

Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo

HTML: 3.2 | Encoding: UTF-8 | Version: 0.7.4