Skip to content

Commit

Permalink
Fix pool header block problem
Browse files Browse the repository at this point in the history
Summary:
There seems to be a bug in the pool allocation regarding the header block in the scenario where no words are left (besides the header word).

Added a panic in the scenario I believe is broken and where the null terminating word is actually overwriting the header word.

In OCaml, any heap block has to be at least 2 words (1 header + one block word). The ocamlpool code needs to change so that when we cannot allocate the next block, there is one word left in the pool (in addition to the reserved header word).

That way, the zero terminator can be properly written in the String_tagged header block.

Reviewed By: rjbailey

Differential Revision: D52574962

fbshipit-source-id: 41795b3900551627e444cf627ad8642745a18434
  • Loading branch information
Manuel Fahndrich authored and facebook-github-bot committed Jan 9, 2024
1 parent 6c3a04c commit b5dfcd1
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions ocamlrep_ocamlpool/ocamlpool.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ static void assert_out_of_section(void) {
*/

static void init_cursor(void) {
ocamlpool_limit = (value*)ocamlpool_root;
ocamlpool_limit =
(value*)ocamlpool_root + 1; // Need one word left for header block
ocamlpool_bound = (value*)ocamlpool_root + Wosize_val(ocamlpool_root);
ocamlpool_cursor = ocamlpool_bound;
}
Expand All @@ -135,6 +136,7 @@ __attribute__((no_sanitize("undefined"))) static void ocamlpool_chunk_truncate(

OCAMLPOOL_SET_HEADER(ocamlpool_root, word_size, String_tag, ocamlpool_color);
value* first_word = (value*)ocamlpool_root;
abort_unless(word_size > 0, "ocamlpool_truncate 0 words left for terminator");
first_word[word_size - 1] = 0;
}

Expand Down Expand Up @@ -253,8 +255,8 @@ value ocamlpool_reserve_block(int tag, size_t words) {
if (pointer < ocamlpool_limit || pointer >= ocamlpool_bound) {
size_t old_ocamlpool_next_chunk_size = ocamlpool_next_chunk_size;
if (size >= ocamlpool_next_chunk_size) {
// Add 1 word for ocaml's header
ocamlpool_next_chunk_size = size + 1;
// Add 2 words for ocaml's header + block
ocamlpool_next_chunk_size = size + 2;
}
ocamlpool_chunk_truncate();
ocamlpool_chunk_alloc();
Expand All @@ -276,7 +278,7 @@ value ocamlpool_reserve_string(size_t bytes) {
size_t words =
((bytes + 1 /*null-ending*/ + (WORD_SIZE - 1) /*rounding*/) / WORD_SIZE);
size_t length = (words * WORD_SIZE);

abort_unless(words > 0, "ocamlpool_reserve_string 0 length");
value result = ocamlpool_reserve_block(String_tag, words);

((value*)result)[words - 1] = 0;
Expand Down

0 comments on commit b5dfcd1

Please sign in to comment.