From 103a87281c235dafc783210dddff13dfb65e7988 Mon Sep 17 00:00:00 2001 From: jacob Date: Sat, 6 Apr 2024 20:18:55 -0500 Subject: [PATCH] top-level scratch arena safety check --- src/app.c | 81 +++++++++++++++++++++++++++++---------------------- src/common.h | 8 ++--- src/config.h | 17 ++++++++++- src/font.c | 2 +- src/json.c | 55 ++++++++++++++++++++++++++-------- src/scratch.c | 8 +++++ src/scratch.h | 50 +++++++++++++++---------------- src/sys.h | 2 +- src/tls.c | 33 ++++++++++++++------- src/tls.h | 6 ++-- src/user.c | 6 +++- 11 files changed, 171 insertions(+), 97 deletions(-) diff --git a/src/app.c b/src/app.c index 251da8d6..d952341c 100644 --- a/src/app.c +++ b/src/app.c @@ -21,20 +21,6 @@ #include "draw.h" #include "math.h" -#if RTC -# if DEVELOPER -# define WINDOW_TITLE "Debug (Developer Build)" -# else -# define WINDOW_TITLE "Debug" -# endif -#else -# if DEVELOPER -# define WINDOW_TITLE "Power Play (Developer Build)" -# else -# define WINDOW_TITLE "Power Play" -# endif -#endif - GLOBAL struct { struct arena arena; struct string write_path; @@ -112,34 +98,15 @@ INTERNAL struct sys_window_settings default_window_settings(struct sys_window *w + + void app_entry_point(void) { L.quit_sf = sync_flag_alloc(); - { - struct temp_arena scratch = scratch_begin_no_conflict(); - struct resource resource = resource_open(STR("res/test.json")); - struct json_parse_result res = json_from_string(scratch.arena, STRING_FROM_BUFFER(resource.bytes)); - resource_close(resource); - struct json *root = res.root; - struct json_error_list errors = res.errors; - (UNUSED)root; - (UNUSED)errors; - - if (errors.first) { - struct string err = string_format(scratch.arena, - STR("Error from offset %F to %F:\n%F"), - FMT_UINT(errors.first->start), - FMT_UINT(errors.first->end), - FMT_STR(errors.first->msg)); - sys_panic(err); - } - - scratch_end(scratch); - } @@ -235,6 +202,50 @@ void app_entry_point(void) struct user_startup_receipt user_sr = user_startup(&work_sr, &renderer_sr, &font_sr, &texture_sr, &draw_sr, &game_sr, &asset_cache_sr, &mixer_sr, &window); struct playback_startup_receipt playback_sr = playback_startup(&mixer_sr); + + + + + + + { + struct temp_arena scratch = scratch_begin_no_conflict(); + + struct resource resource = resource_open(STR("res/test.json")); + struct json_parse_result res = json_from_string(scratch.arena, STRING_FROM_BUFFER(resource.bytes)); + resource_close(resource); + + struct json *root = res.root; + struct json_error_list errors = res.errors; + (UNUSED)root; + (UNUSED)errors; + + if (errors.first) { + struct string err = string_format(scratch.arena, + STR("Error from offset %F to %F:\n%F"), + FMT_UINT(errors.first->start), + FMT_UINT(errors.first->end), + FMT_STR(errors.first->msg)); + sys_panic(err); + } + + scratch_end(scratch); + } + + + + + + + + + + + + + + + (UNUSED)user_sr; (UNUSED)playback_sr; diff --git a/src/common.h b/src/common.h index 8669923e..1249c62d 100644 --- a/src/common.h +++ b/src/common.h @@ -138,10 +138,10 @@ extern "C" { /* Address sanitization */ #if ASAN -void __asan_poison_memory_region(void *, size_t); -void __asan_unpoison_memory_region(void *, size_t); -# define ASAN_POISON(addr, size) __asan_poison_memory_region(addr, size); -# define ASAN_UNPOISON(addr, size) __asan_unpoison_memory_region(addr, size); +void __asan_poison_memory_region(void const volatile *, size_t); +void __asan_unpoison_memory_region(void const volatile *add, size_t); +# define ASAN_POISON(addr, size) __asan_poison_memory_region((addr), (size)); +# define ASAN_UNPOISON(addr, size) __asan_unpoison_memory_region((addr), (size)); #else # define ASAN_POISON(addr, size) # define ASAN_UNPOISON(addr, size) diff --git a/src/config.h b/src/config.h index b06315be..51fca89a 100644 --- a/src/config.h +++ b/src/config.h @@ -3,6 +3,21 @@ #define WRITE_DIR "power_play" #define SETTINGS_FILENAME "settings.json" +/* Window title */ +#if RTC +# if DEVELOPER +# define WINDOW_TITLE "Debug (Developer Build)" +# else +# define WINDOW_TITLE "Debug" +# endif +#else +# if DEVELOPER +# define WINDOW_TITLE "Power Play (Developer Build)" +# else +# define WINDOW_TITLE "Power Play" +# endif +#endif + /* If we are not compiling in developer mode, assume resources are embedded as * a tar archive in the executable. Otherwise, look for resources in the file * system. */ @@ -17,7 +32,7 @@ #define GAME_TIMESCALE 1.0 /* How many ticks back in time should the user blend between? - * Delay ms = USER_INTERP_OFFSET_TICK_RATIO * Game tick rate + * = * * E.g: At 1.5, the user thread will render 49.5ms back in time (if game thread runs at 30FPS) */ #define USER_INTERP_OFFSET_TICK_RATIO 1.1 diff --git a/src/font.c b/src/font.c index 3745ab4a..c5e4fb00 100644 --- a/src/font.c +++ b/src/font.c @@ -142,7 +142,7 @@ INTERNAL WORK_TASK_FUNC_DEF(font_load_asset_task, vparams) font->glyphs_count = result.glyphs_count; font->point_size = point_size; - /* FIXME: Load banked font instead of panicking */ + /* FIXME: Load baked font instead of panicking */ if (font->glyphs_count <= 0) { sys_panic(string_format(scratch.arena, STR("Parsed 0 glyphs from font \"%F\"!"), diff --git a/src/json.c b/src/json.c index 8e93c26d..0783cfc7 100644 --- a/src/json.c +++ b/src/json.c @@ -36,9 +36,9 @@ enum token_type { TOKEN_TYPE_NUMBER, TOKEN_TYPE_STRING, - TOKEN_TYPE_TRUE, - TOKEN_TYPE_FALSE, - TOKEN_TYPE_NULL, + TOKEN_TYPE_KEYWORD_TRUE, + TOKEN_TYPE_KEYWORD_FALSE, + TOKEN_TYPE_KEYWORD_NULL, TOKEN_TYPE_COMMA, TOKEN_TYPE_COLON, @@ -53,8 +53,17 @@ enum token_type { struct token { enum token_type type; + u64 start; u64 end; + +#if 0 + union { + f64 number_value; + struct string string_value; + } value; +#endif + struct token *next; }; @@ -76,9 +85,9 @@ GLOBAL READONLY struct string g_keyword_strings[] = { }; GLOBAL READONLY enum token_type g_keyword_types[] = { - ['t'] = TOKEN_TYPE_TRUE, - ['f'] = TOKEN_TYPE_FALSE, - ['n'] = TOKEN_TYPE_NULL + ['t'] = TOKEN_TYPE_KEYWORD_TRUE, + ['f'] = TOKEN_TYPE_KEYWORD_FALSE, + ['n'] = TOKEN_TYPE_KEYWORD_NULL }; INTERNAL struct token *push_token(struct arena *arena, struct token_list *list) @@ -93,6 +102,18 @@ INTERNAL struct token *push_token(struct arena *arena, struct token_list *list) return t; } +#if 0 +INTERNAL u8 peek_char_offset(struct string src, u64 pos, i64 offset) +{ + u8 c = 0; + i64 peek_pos = pos + offset; + if (0 <= peek_pos && peek_pos < (i64)src.len) { + c = src.text[peek_pos]; + } + return c; +} +#endif + INTERNAL struct token_list lex(struct arena *arena, struct string src) { struct token_list res = { 0 }; @@ -161,8 +182,20 @@ INTERNAL struct token_list lex(struct arena *arena, struct string src) } break; #if 0 - case '-': { + + b32 is_whole_sign = false; + switch (peek_char_offset(src, pos, 1)) { + case '0': { + switch (peek_char_offset(src, pos, 2)) { + case '.': { + } break; + } + } break; + } + + if (peek_char_offset(src, pos, 1)) + b32 is_whole_sign = false; if ((pos + 1) < src.len) { switch (src.text[pos + 1]) { @@ -191,7 +224,6 @@ INTERNAL struct token_list lex(struct arena *arena, struct string src) break; } }; - CASE_DIGIT_0_TO_9: { t->type = TOKEN_TYPE_NUMBER; @@ -205,7 +237,6 @@ INTERNAL struct token_list lex(struct arena *arena, struct string src) } break; - #else /* Number */ case '-': { @@ -808,19 +839,19 @@ INTERNAL void parse(struct arena *arena, struct parser *p) } } break; - case TOKEN_TYPE_TRUE: { + case TOKEN_TYPE_KEYWORD_TRUE: { json->type = JSON_TYPE_BOOL; json->value.boolean = true; at = at->next; } break; - case TOKEN_TYPE_FALSE: { + case TOKEN_TYPE_KEYWORD_FALSE: { json->type = JSON_TYPE_BOOL; json->value.boolean = false; at = at->next; } break; - case TOKEN_TYPE_NULL: { + case TOKEN_TYPE_KEYWORD_NULL: { json->type = JSON_TYPE_NULL; at = at->next; } break; diff --git a/src/scratch.c b/src/scratch.c index f12094bb..90554a27 100644 --- a/src/scratch.c +++ b/src/scratch.c @@ -13,6 +13,14 @@ INTERNAL THREAD_LOCAL_VAR_RELEASE_FUNC_DEF(test_scratch_context_release, vctx) { __prof; struct scratch_ctx *ctx = vctx; + +#if RTC + /* If stack count is not 0, then a `scratch_end` is missing on a top-level + * scratch arena (The temp_arena with + * `scratch_id` = ctx->scratch_id_stack[ctx->scratch_id_stack_count - 1]) */ + ASSERT(ctx->scratch_id_stack_count == 0); +#endif + for (u32 i = 0; i < ARRAY_COUNT(ctx->arenas); ++i) { arena_release(&ctx->arenas[i]); } diff --git a/src/scratch.h b/src/scratch.h index 842b3624..ea8e76dd 100644 --- a/src/scratch.h +++ b/src/scratch.h @@ -28,6 +28,20 @@ THREAD_LOCAL_VAR_DECL_EXTERN(tl_scratch_ctx, struct scratch_ctx); * Scratch begin * ========================== */ +INLINE void scratch_dbg_push(struct scratch_ctx *ctx, struct temp_arena *temp) +{ +#if RTC + if (ctx->scratch_id_stack_count >= ARRAY_COUNT(ctx->scratch_id_stack)) { + sys_panic_raw("Max debug scratch depth reached"); + } + temp->scratch_id = ctx->next_scratch_id++; + ctx->scratch_id_stack[ctx->scratch_id_stack_count++] = temp->scratch_id; +#else + (UNUSED)ctx; + (UNUSED)temp; +#endif +} + /* Any arena parameters in the calling function's context should be passed into this * function as a potential `conflict`. This is to prevent conflicts when the * context's arena is itself a scratch arena (since parameterized arenas are @@ -46,21 +60,12 @@ INLINE struct temp_arena _scratch_begin(struct arena *potential_conflict) ASSERT(potential_conflict != NULL); struct scratch_ctx *ctx = thread_local_eval(&tl_scratch_ctx); - struct arena *scratch = &ctx->arenas[0]; - if (potential_conflict && scratch->base == potential_conflict->base) { - scratch = &ctx->arenas[1]; + struct arena *scratch_arena = &ctx->arenas[0]; + if (potential_conflict && scratch_arena->base == potential_conflict->base) { + scratch_arena = &ctx->arenas[1]; } - - struct temp_arena temp = arena_temp_begin(scratch); - -#if RTC - if (ctx->scratch_id_stack_count >= ARRAY_COUNT(ctx->scratch_id_stack)) { - sys_panic(STR("Max debug scratch depth reached")); - } - temp.scratch_id = ctx->next_scratch_id++; - ctx->scratch_id_stack[ctx->scratch_id_stack_count++] = temp.scratch_id; -#endif - + struct temp_arena temp = arena_temp_begin(scratch_arena); + scratch_dbg_push(ctx, &temp); return temp; } @@ -79,17 +84,9 @@ INLINE struct temp_arena _scratch_begin(struct arena *potential_conflict) INLINE struct temp_arena _scratch_begin_no_conflict(void) { struct scratch_ctx *ctx = thread_local_eval(&tl_scratch_ctx); - struct arena *scratch = &ctx->arenas[0]; - struct temp_arena temp = arena_temp_begin(scratch); - -#if RTC - if (ctx->scratch_id_stack_count >= ARRAY_COUNT(ctx->scratch_id_stack)) { - sys_panic(STR("Max debug scratch depth reached")); - } - temp.scratch_id = ctx->next_scratch_id++; - ctx->scratch_id_stack[ctx->scratch_id_stack_count++] = temp.scratch_id; -#endif - + struct arena *scratch_arena = &ctx->arenas[0]; + struct temp_arena temp = arena_temp_begin(scratch_arena); + scratch_dbg_push(ctx, &temp); return temp; } @@ -106,13 +103,12 @@ INLINE void scratch_end(struct temp_arena scratch_temp) u64 expected_id = ctx->scratch_id_stack[--ctx->scratch_id_stack_count]; /* This assertion exists to catch cases where a scratch_end was forgotten. * It will fail if a scratch arena is reset out of order. - * IE there is a missing scratch_end somewhere on a different scratch + * E.g. there is a missing scratch_end somewhere on a different scratch * arena (one that was created between the scratch_begin & the * scratch_end of the arena being reset here). */ ASSERT(scratch_id == expected_id); } #endif - arena_temp_end(scratch_temp); } diff --git a/src/sys.h b/src/sys.h index 092dc355..b04d3a3e 100644 --- a/src/sys.h +++ b/src/sys.h @@ -390,7 +390,7 @@ struct thread_local_store *sys_thread_get_thread_local_store(void); * Threads * ========================== */ -#define SYS_THREAD_STACK_SIZE MEGABYTE(2) +#define SYS_THREAD_STACK_SIZE MEGABYTE(4) #define SYS_THREAD_FUNC_DEF(name, arg_name) void name(void *arg_name) typedef SYS_THREAD_FUNC_DEF(sys_thread_func, data); diff --git a/src/tls.c b/src/tls.c index 7ecea7f2..01c12db7 100644 --- a/src/tls.c +++ b/src/tls.c @@ -6,6 +6,9 @@ #define THREAD_LOCAL_TABLE_RESERVE (MEGABYTE(1)) +/* Arbitrary. Increase if needed. */ +#define MAX_THREAD_LOCAL_VARS 64 + GLOBAL struct { struct atomic_i64 tls_metas_lock_flag; u64 tls_metas_count; @@ -30,6 +33,8 @@ struct thread_local_store thread_local_store_alloc(void) __prof; struct thread_local_store t = { 0 }; t.arena = arena_alloc(THREAD_LOCAL_TABLE_RESERVE); + t.lookup = arena_push_array_zero(&t.arena, void *, MAX_THREAD_LOCAL_VARS); + t.allocation_order = arena_push_array_zero(&t.arena, u64, MAX_THREAD_LOCAL_VARS); return t; } @@ -63,12 +68,17 @@ void *_thread_local_eval(struct thread_local_var_meta *meta) __profscope(_thread_local_eval__REGISTER); tls_metas_lock(); { - id = L.tls_metas_count++; - if (id >= MAX_THREAD_LOCAL_VARS) { - sys_panic_raw("Maximum number of thread local variables reached"); + id_plus_one = atomic_u64_eval(&meta->id_plus_one); /* Re-check now that locked */ + if (id_plus_one == 0) { + id = L.tls_metas_count++; + if (id >= MAX_THREAD_LOCAL_VARS) { + sys_panic_raw("Maximum number of thread local variables reached"); + } + atomic_u64_eval_exchange(&meta->id_plus_one, id + 1); + L.tls_metas[id] = *meta; + } else { + id = id_plus_one - 1; } - atomic_u64_eval_exchange(&meta->id_plus_one, id + 1); - L.tls_metas[id] = *meta; } tls_metas_unlock(); } else { @@ -78,19 +88,20 @@ void *_thread_local_eval(struct thread_local_var_meta *meta) /* Allocate var for thread if unallocated */ struct thread_local_store *t = sys_thread_get_thread_local_store(); - void **data_slot = &t->lookup[id]; - if (!*data_slot) { + void *data = t->lookup[id]; + if (!data) { __profscope(_thread_local_eval__ALLOC); /* Allocate */ arena_align(&t->arena, meta->align); - *data_slot = arena_push_array(&t->arena, u8, meta->size); + data = arena_push_array(&t->arena, u8, meta->size); if (meta->alloc) { - meta->alloc(*data_slot); + meta->alloc(data); } else { - MEMZERO(*data_slot, meta->size); + MEMZERO(data, meta->size); } + t->lookup[id] = data; t->allocation_order[t->allocation_order_count++] = id; } - return *data_slot; + return data; } diff --git a/src/tls.h b/src/tls.h index e4c5e14b..8e0b660e 100644 --- a/src/tls.h +++ b/src/tls.h @@ -1,17 +1,15 @@ #ifndef TLS_H #define TLS_H -#define MAX_THREAD_LOCAL_VARS 1024 - /* ========================== * * Thread local store * ========================== */ struct thread_local_store { - void *lookup[MAX_THREAD_LOCAL_VARS]; + void **lookup; struct arena arena; u64 allocation_order_count; - u64 allocation_order[MAX_THREAD_LOCAL_VARS]; + u64 *allocation_order; }; struct thread_local_store thread_local_store_alloc(void); diff --git a/src/user.c b/src/user.c index f9a131dd..7b95895b 100644 --- a/src/user.c +++ b/src/user.c @@ -1018,7 +1018,6 @@ INTERNAL void user_update(void) /* Draw test input string */ { struct font *font = font_load(STR("res/fonts/fixedsys.ttf"), 12.0f); - //struct font *font = font_load(STR("res/fonts/Inconsolata-Regular.ttf"), 12.0f); #if 0 struct rect atlas_rect = { @@ -1048,6 +1047,11 @@ INTERNAL void user_update(void) draw_str.len += string_copy(scratch.arena, STR("]: '")).len; struct string character = { .len = decoded.advance8, .text = remaining.text }; + if (string_eq(character, STR("\n"))) { + character = STR("\\n"); + } else if (string_eq(character, STR("\r"))) { + character = STR("\\r"); + } draw_str.len += string_copy(scratch.arena, character).len; draw_str.len += string_copy(scratch.arena, STR("' ")).len;