From d47bb95f87ed655c2441a7befb17dd44fc8ecea2 Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Fri, 16 Aug 2024 21:11:42 -0500 Subject: [PATCH 01/14] allow string specification to enable cilk alerts --- runtime/debug.c | 133 ++++++++++++++++++++++++++++++++++++++++++----- runtime/debug.h | 4 +- runtime/global.c | 2 +- 3 files changed, 124 insertions(+), 15 deletions(-) diff --git a/runtime/debug.c b/runtime/debug.c index f08e5414..5959a6eb 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -3,11 +3,14 @@ #include "global.h" #include +#include #include #include #include #include +#define ALERT_STR_BUF_LEN 16 + #if ALERT_LVL & (ALERT_CFRAME|ALERT_RETURN) unsigned int alert_level = 0; #else @@ -20,24 +23,130 @@ CHEETAH_INTERNAL unsigned int debug_level = 0; static size_t alert_log_size = 0, alert_log_offset = 0; static char *alert_log = NULL; -void set_alert_level(unsigned int level) { - alert_level = level; - if (level == 0) { - flush_alert_log(); - return; +static const char *const ALERT_NONE_STR = "none"; +static const char *const ALERT_FIBER_STR = "fiber"; +static const char *const ALERT_MEMORY_STR = "memory"; +static const char *const ALERT_SYNC_STR = "sync"; +static const char *const ALERT_SCHED_STR = "sched"; +static const char *const ALERT_STEAL_STR = "steal"; +static const char *const ALERT_RETURN_STR = "return"; +static const char *const ALERT_EXCEPT_STR = "except"; +static const char *const ALERT_CFRAME_STR = "cframe"; +static const char *const ALERT_REDUCE_STR = "reduce"; +static const char *const ALERT_REDUCE_ID_STR = "reduce_id"; +static const char *const ALERT_BOOT_STR = "boot"; +static const char *const ALERT_START_STR = "start"; +static const char *const ALERT_CLOSURE_STR = "closure"; + +static void parse_alert_level_str(const char *const alert_str) { + if (strcmp(ALERT_NONE_STR, alert_str) == 0) { + // no-op + } else if (strcmp(ALERT_FIBER_STR, alert_str) == 0) { + alert_level |= ALERT_FIBER; + } else if (strcmp(ALERT_MEMORY_STR, alert_str) == 0) { + alert_level |= ALERT_MEMORY; + } else if (strcmp(ALERT_SYNC_STR, alert_str) == 0) { + alert_level |= ALERT_SYNC; + } else if (strcmp(ALERT_SCHED_STR, alert_str) == 0) { + alert_level |= ALERT_SCHED; + } else if (strcmp(ALERT_STEAL_STR, alert_str) == 0) { + alert_level |= ALERT_STEAL; + } else if (strcmp(ALERT_RETURN_STR, alert_str) == 0) { + alert_level |= ALERT_RETURN; + } else if (strcmp(ALERT_EXCEPT_STR, alert_str) == 0) { + alert_level |= ALERT_EXCEPT; + } else if (strcmp(ALERT_CFRAME_STR, alert_str) == 0) { + alert_level |= ALERT_CFRAME; + } else if (strcmp(ALERT_REDUCE_STR, alert_str) == 0) { + alert_level |= ALERT_REDUCE; + } else if (strcmp(ALERT_REDUCE_ID_STR, alert_str) == 0) { + alert_level |= ALERT_REDUCE_ID; + } else if (strcmp(ALERT_BOOT_STR, alert_str) == 0) { + alert_level |= ALERT_BOOT; + } else if (strcmp(ALERT_START_STR, alert_str) == 0) { + alert_level |= ALERT_START; + } else if (strcmp(ALERT_CLOSURE_STR, alert_str) == 0) { + alert_level |= ALERT_CLOSURE; + } else { + fprintf(stderr, "Invalid CILK_ALERT value: %s\n", alert_str); } - if (level & ALERT_NOBUF) { - return; +} + +static void parse_alert_level_env(const char *const alert_env) { + char curr_alert[ALERT_STR_BUF_LEN + 1]; + + size_t buf_str_len = 0; + + // We only allow numbers for backwards compatibility, so + // only allow numbers if they are the only thing passed to + // CILK_ALERT + bool numbers_allowed = true; + bool invalid = false; + + for (size_t i = 0; i < strlen(alert_env); ++i) { + if (alert_env[i] == ',') { + curr_alert[buf_str_len] = '\0'; + numbers_allowed = false; + if (invalid) { + fputc('\n', stderr); + invalid = false; + } else { + parse_alert_level_str(curr_alert); + } + buf_str_len = 0; + } else if (buf_str_len < ALERT_STR_BUF_LEN) { + curr_alert[buf_str_len++] = tolower(alert_env[i]); + } else { + curr_alert[buf_str_len] = '\0'; + fprintf(stderr, "Invalid CILK_ALERT option: %s%c", curr_alert, alert_env[i]); + curr_alert[0] = '\0'; + } } - if (alert_log == NULL) { - alert_log_size = 5000; - alert_log = malloc(alert_log_size); - if (alert_log) { - memset(alert_log, ' ', alert_log_size); + + if (invalid) { + fputc('\n', stderr); + buf_str_len = 0; + } + + if (buf_str_len > 0) { + curr_alert[buf_str_len] = '\0'; + + if (numbers_allowed) { + char **tol_end = &alert_env; + alert_level = strtol(alert_env, tol_end, 0); + if (alert_level == 0 && (**tol_end != '\0' || *tol_end == alert_env)) { + parse_alert_level_str(curr_alert); + } + } else { + parse_alert_level_str(curr_alert); } } } +void set_alert_level(const char *const alert_env) { + if (alert_env) { + parse_alert_level_env(alert_env); + } +} + +//void set_alert_level(unsigned int level) { +// alert_level = level; +// if (level == 0) { +// flush_alert_log(); +// return; +// } +// if (level & ALERT_NOBUF) { +// return; +// } +// if (alert_log == NULL) { +// alert_log_size = 5000; +// alert_log = malloc(alert_log_size); +// if (alert_log) { +// memset(alert_log, ' ', alert_log_size); +// } +// } +//} + void set_debug_level(unsigned int level) { debug_level = level; } const char *const __cilkrts_assertion_failed = diff --git a/runtime/debug.h b/runtime/debug.h index 2666de75..f18d9e9a 100644 --- a/runtime/debug.h +++ b/runtime/debug.h @@ -30,7 +30,6 @@ struct __cilkrts_worker; #define ALERT_BOOT 0x1000 #define ALERT_START 0x2000 #define ALERT_CLOSURE 0x4000 -#define ALERT_NOBUF 0x80000000 #if ALERT_LVL & (ALERT_CFRAME|ALERT_RETURN) extern unsigned int alert_level; @@ -54,7 +53,8 @@ extern CHEETAH_INTERNAL unsigned int debug_level; // Unused: compiler inlines the stack frame creation // #define CILK_STACKFRAME_MAGIC 0xCAFEBABE -CHEETAH_INTERNAL void set_alert_level(unsigned int); +//CHEETAH_INTERNAL void set_alert_level(unsigned int); +CHEETAH_INTERNAL void set_alert_level(const char *const); CHEETAH_INTERNAL void set_debug_level(unsigned int); CHEETAH_INTERNAL void flush_alert_log(void); diff --git a/runtime/global.c b/runtime/global.c index d3341242..005f3564 100644 --- a/runtime/global.c +++ b/runtime/global.c @@ -41,7 +41,7 @@ unsigned __cilkrts_nproc = 0; static void set_alert_debug_level() { /* Only the bits also set in ALERT_LVL are used. */ - set_alert_level(env_get_int("CILK_ALERT")); + set_alert_level(getenv("CILK_ALERT")); /* Only the bits also set in DEBUG_LVL are used. */ set_debug_level(env_get_int("CILK_DEBUG")); } From 936dab86d4837d9b4034cca5233064e23389237e Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Sat, 17 Aug 2024 12:53:26 -0500 Subject: [PATCH 02/14] remove commented out code --- runtime/debug.h | 1 - 1 file changed, 1 deletion(-) diff --git a/runtime/debug.h b/runtime/debug.h index f18d9e9a..c548e4d8 100644 --- a/runtime/debug.h +++ b/runtime/debug.h @@ -53,7 +53,6 @@ extern CHEETAH_INTERNAL unsigned int debug_level; // Unused: compiler inlines the stack frame creation // #define CILK_STACKFRAME_MAGIC 0xCAFEBABE -//CHEETAH_INTERNAL void set_alert_level(unsigned int); CHEETAH_INTERNAL void set_alert_level(const char *const); CHEETAH_INTERNAL void set_debug_level(unsigned int); CHEETAH_INTERNAL void flush_alert_log(void); From 920e62233aa87165dabd9fac42cbabb409565eb6 Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Sat, 17 Aug 2024 13:41:10 -0500 Subject: [PATCH 03/14] add back missing logic related to buffering cilkrts_alert --- runtime/debug.c | 101 ++++++++++++++++++++++++++--------------------- runtime/debug.h | 4 +- runtime/global.c | 2 +- 3 files changed, 61 insertions(+), 46 deletions(-) diff --git a/runtime/debug.c b/runtime/debug.c index 5959a6eb..43c9e5fd 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -9,7 +9,6 @@ #include #include -#define ALERT_STR_BUF_LEN 16 #if ALERT_LVL & (ALERT_CFRAME|ALERT_RETURN) unsigned int alert_level = 0; @@ -23,6 +22,7 @@ CHEETAH_INTERNAL unsigned int debug_level = 0; static size_t alert_log_size = 0, alert_log_offset = 0; static char *alert_log = NULL; +#define ALERT_STR_BUF_LEN 16 static const char *const ALERT_NONE_STR = "none"; static const char *const ALERT_FIBER_STR = "fiber"; static const char *const ALERT_MEMORY_STR = "memory"; @@ -37,42 +37,48 @@ static const char *const ALERT_REDUCE_ID_STR = "reduce_id"; static const char *const ALERT_BOOT_STR = "boot"; static const char *const ALERT_START_STR = "start"; static const char *const ALERT_CLOSURE_STR = "closure"; +static const char *const ALERT_NOBUF_STR = "nobuf"; -static void parse_alert_level_str(const char *const alert_str) { +static int parse_alert_level_str(const char *const alert_str) { if (strcmp(ALERT_NONE_STR, alert_str) == 0) { - // no-op + return ALERT_NONE; } else if (strcmp(ALERT_FIBER_STR, alert_str) == 0) { - alert_level |= ALERT_FIBER; + return ALERT_FIBER; } else if (strcmp(ALERT_MEMORY_STR, alert_str) == 0) { - alert_level |= ALERT_MEMORY; + return ALERT_MEMORY; } else if (strcmp(ALERT_SYNC_STR, alert_str) == 0) { - alert_level |= ALERT_SYNC; + return ALERT_SYNC; } else if (strcmp(ALERT_SCHED_STR, alert_str) == 0) { - alert_level |= ALERT_SCHED; + return ALERT_SCHED; } else if (strcmp(ALERT_STEAL_STR, alert_str) == 0) { - alert_level |= ALERT_STEAL; + return ALERT_STEAL; } else if (strcmp(ALERT_RETURN_STR, alert_str) == 0) { - alert_level |= ALERT_RETURN; + return ALERT_RETURN; } else if (strcmp(ALERT_EXCEPT_STR, alert_str) == 0) { - alert_level |= ALERT_EXCEPT; + return ALERT_EXCEPT; } else if (strcmp(ALERT_CFRAME_STR, alert_str) == 0) { - alert_level |= ALERT_CFRAME; + return ALERT_CFRAME; } else if (strcmp(ALERT_REDUCE_STR, alert_str) == 0) { - alert_level |= ALERT_REDUCE; + return ALERT_REDUCE; } else if (strcmp(ALERT_REDUCE_ID_STR, alert_str) == 0) { - alert_level |= ALERT_REDUCE_ID; + return ALERT_REDUCE_ID; } else if (strcmp(ALERT_BOOT_STR, alert_str) == 0) { - alert_level |= ALERT_BOOT; + return ALERT_BOOT; } else if (strcmp(ALERT_START_STR, alert_str) == 0) { - alert_level |= ALERT_START; + return ALERT_START; } else if (strcmp(ALERT_CLOSURE_STR, alert_str) == 0) { - alert_level |= ALERT_CLOSURE; - } else { - fprintf(stderr, "Invalid CILK_ALERT value: %s\n", alert_str); + return ALERT_CLOSURE; + } else if (strcmp(ALERT_NOBUF_STR, alert_str) == 0) { + return ALERT_NOBUF; } + + fprintf(stderr, "Invalid CILK_ALERT value: %s\n", alert_str); + + return ALERT_NONE; } -static void parse_alert_level_env(const char *const alert_env) { +static int parse_alert_level_env(const char *const alert_env) { + int new_alert_lvl = 0; char curr_alert[ALERT_STR_BUF_LEN + 1]; size_t buf_str_len = 0; @@ -91,15 +97,20 @@ static void parse_alert_level_env(const char *const alert_env) { fputc('\n', stderr); invalid = false; } else { - parse_alert_level_str(curr_alert); + new_alert_lvl |= parse_alert_level_str(curr_alert); } buf_str_len = 0; } else if (buf_str_len < ALERT_STR_BUF_LEN) { curr_alert[buf_str_len++] = tolower(alert_env[i]); } else { curr_alert[buf_str_len] = '\0'; - fprintf(stderr, "Invalid CILK_ALERT option: %s%c", curr_alert, alert_env[i]); + fprintf(stderr, "%s%s%c", + // Only print the prefix the first time we get here. + invalid ? "" : "Invalid CILK_ALERT value: ", + curr_alert, + alert_env[i]); curr_alert[0] = '\0'; + invalid = true; } } @@ -113,39 +124,41 @@ static void parse_alert_level_env(const char *const alert_env) { if (numbers_allowed) { char **tol_end = &alert_env; - alert_level = strtol(alert_env, tol_end, 0); - if (alert_level == 0 && (**tol_end != '\0' || *tol_end == alert_env)) { - parse_alert_level_str(curr_alert); + new_alert_lvl = strtol(alert_env, tol_end, 0); + if (new_alert_lvl == 0 && (**tol_end != '\0' || *tol_end == alert_env)) { + new_alert_lvl |= parse_alert_level_str(curr_alert); } } else { - parse_alert_level_str(curr_alert); + new_alert_lvl |= parse_alert_level_str(curr_alert); } } + + set_alert_level(new_alert_lvl); } -void set_alert_level(const char *const alert_env) { +void set_alert_level_from_str(const char *const alert_env) { if (alert_env) { - parse_alert_level_env(alert_env); + int new_alert_lvl = parse_alert_level_env(alert_env); } } -//void set_alert_level(unsigned int level) { -// alert_level = level; -// if (level == 0) { -// flush_alert_log(); -// return; -// } -// if (level & ALERT_NOBUF) { -// return; -// } -// if (alert_log == NULL) { -// alert_log_size = 5000; -// alert_log = malloc(alert_log_size); -// if (alert_log) { -// memset(alert_log, ' ', alert_log_size); -// } -// } -//} +void set_alert_level(unsigned int level) { + alert_level = level; + if (level == 0) { + flush_alert_log(); + return; + } + if (level & ALERT_NOBUF) { + return; + } + if (alert_log == NULL) { + alert_log_size = 5000; + alert_log = malloc(alert_log_size); + if (alert_log) { + memset(alert_log, ' ', alert_log_size); + } + } +} void set_debug_level(unsigned int level) { debug_level = level; } diff --git a/runtime/debug.h b/runtime/debug.h index c548e4d8..a4d026b7 100644 --- a/runtime/debug.h +++ b/runtime/debug.h @@ -30,6 +30,7 @@ struct __cilkrts_worker; #define ALERT_BOOT 0x1000 #define ALERT_START 0x2000 #define ALERT_CLOSURE 0x4000 +#define ALERT_NOBUF 0x80000000 #if ALERT_LVL & (ALERT_CFRAME|ALERT_RETURN) extern unsigned int alert_level; @@ -53,7 +54,8 @@ extern CHEETAH_INTERNAL unsigned int debug_level; // Unused: compiler inlines the stack frame creation // #define CILK_STACKFRAME_MAGIC 0xCAFEBABE -CHEETAH_INTERNAL void set_alert_level(const char *const); +CHEETAH_INTERNAL void set_alert_level_from_str(const char *const); +CHEETAH_INTERNAL void set_alert_level(unsigned int); CHEETAH_INTERNAL void set_debug_level(unsigned int); CHEETAH_INTERNAL void flush_alert_log(void); diff --git a/runtime/global.c b/runtime/global.c index 005f3564..12415c28 100644 --- a/runtime/global.c +++ b/runtime/global.c @@ -41,7 +41,7 @@ unsigned __cilkrts_nproc = 0; static void set_alert_debug_level() { /* Only the bits also set in ALERT_LVL are used. */ - set_alert_level(getenv("CILK_ALERT")); + set_alert_level_from_str(getenv("CILK_ALERT")); /* Only the bits also set in DEBUG_LVL are used. */ set_debug_level(env_get_int("CILK_DEBUG")); } From 897e3bca3ce87b6c56da9a158d0ea77acc10f181 Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Sat, 17 Aug 2024 13:55:36 -0500 Subject: [PATCH 04/14] remove extra whitespace --- runtime/debug.c | 1 - 1 file changed, 1 deletion(-) diff --git a/runtime/debug.c b/runtime/debug.c index 43c9e5fd..fe4aabec 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -9,7 +9,6 @@ #include #include - #if ALERT_LVL & (ALERT_CFRAME|ALERT_RETURN) unsigned int alert_level = 0; #else From 56d95271dbd7e8e0aa80c91432c8c4ea2c67b567 Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Tue, 20 Aug 2024 13:59:39 -0500 Subject: [PATCH 05/14] fix some stupid midnight coding issues --- runtime/debug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/runtime/debug.c b/runtime/debug.c index fe4aabec..1a3ae5ae 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -132,12 +132,13 @@ static int parse_alert_level_env(const char *const alert_env) { } } - set_alert_level(new_alert_lvl); + return new_alert_lvl; } void set_alert_level_from_str(const char *const alert_env) { if (alert_env) { int new_alert_lvl = parse_alert_level_env(alert_env); + set_alert_level(new_alert_lvl); } } From e091118ec399ecf5b2797c5f00d66df9567b984c Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Fri, 23 Aug 2024 08:44:22 -0500 Subject: [PATCH 06/14] strtok and lfind --- runtime/debug.c | 176 +++++++++++++++++++++++------------------------- runtime/debug.h | 2 +- 2 files changed, 86 insertions(+), 92 deletions(-) diff --git a/runtime/debug.c b/runtime/debug.c index 1a3ae5ae..9099a983 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -22,120 +23,113 @@ static size_t alert_log_size = 0, alert_log_offset = 0; static char *alert_log = NULL; #define ALERT_STR_BUF_LEN 16 -static const char *const ALERT_NONE_STR = "none"; -static const char *const ALERT_FIBER_STR = "fiber"; -static const char *const ALERT_MEMORY_STR = "memory"; -static const char *const ALERT_SYNC_STR = "sync"; -static const char *const ALERT_SCHED_STR = "sched"; -static const char *const ALERT_STEAL_STR = "steal"; -static const char *const ALERT_RETURN_STR = "return"; -static const char *const ALERT_EXCEPT_STR = "except"; -static const char *const ALERT_CFRAME_STR = "cframe"; -static const char *const ALERT_REDUCE_STR = "reduce"; -static const char *const ALERT_REDUCE_ID_STR = "reduce_id"; -static const char *const ALERT_BOOT_STR = "boot"; -static const char *const ALERT_START_STR = "start"; -static const char *const ALERT_CLOSURE_STR = "closure"; -static const char *const ALERT_NOBUF_STR = "nobuf"; + +typedef struct __alert_level_t { + const char *const name; + const int mask_value; +} alert_level_t; + +static const alert_level_t alert_table[] = { + {"none", ALERT_NONE}, + {"fiber", ALERT_FIBER}, + {"memory", ALERT_MEMORY}, + {"sync", ALERT_SYNC}, + {"sched", ALERT_SCHED}, + {"steal", ALERT_STEAL}, + {"return", ALERT_RETURN}, + {"except", ALERT_EXCEPT}, + {"cframe", ALERT_CFRAME}, + {"reduce", ALERT_REDUCE}, + {"reduce_id", ALERT_REDUCE_ID}, + {"boot", ALERT_BOOT}, + {"start", ALERT_START}, + {"closure", ALERT_CLOSURE}, + // Must be last in the table + {"nobuf", ALERT_NOBUF}, +}; + +static int alert_name_comparison(const void *a, const void *b) { + const alert_level_t *ala = (const alert_level_t*)a; + const alert_level_t *alb = (const alert_level_t*)b; + + return strcmp(ala->name, alb->name); +} + +static size_t get_alert_table_size() { + size_t s = 0; + + for (s = 0; alert_table[s].mask_value != ALERT_NOBUF; ++s) { + // no-op + } + + return s; +} static int parse_alert_level_str(const char *const alert_str) { - if (strcmp(ALERT_NONE_STR, alert_str) == 0) { - return ALERT_NONE; - } else if (strcmp(ALERT_FIBER_STR, alert_str) == 0) { - return ALERT_FIBER; - } else if (strcmp(ALERT_MEMORY_STR, alert_str) == 0) { - return ALERT_MEMORY; - } else if (strcmp(ALERT_SYNC_STR, alert_str) == 0) { - return ALERT_SYNC; - } else if (strcmp(ALERT_SCHED_STR, alert_str) == 0) { - return ALERT_SCHED; - } else if (strcmp(ALERT_STEAL_STR, alert_str) == 0) { - return ALERT_STEAL; - } else if (strcmp(ALERT_RETURN_STR, alert_str) == 0) { - return ALERT_RETURN; - } else if (strcmp(ALERT_EXCEPT_STR, alert_str) == 0) { - return ALERT_EXCEPT; - } else if (strcmp(ALERT_CFRAME_STR, alert_str) == 0) { - return ALERT_CFRAME; - } else if (strcmp(ALERT_REDUCE_STR, alert_str) == 0) { - return ALERT_REDUCE; - } else if (strcmp(ALERT_REDUCE_ID_STR, alert_str) == 0) { - return ALERT_REDUCE_ID; - } else if (strcmp(ALERT_BOOT_STR, alert_str) == 0) { - return ALERT_BOOT; - } else if (strcmp(ALERT_START_STR, alert_str) == 0) { - return ALERT_START; - } else if (strcmp(ALERT_CLOSURE_STR, alert_str) == 0) { - return ALERT_CLOSURE; - } else if (strcmp(ALERT_NOBUF_STR, alert_str) == 0) { - return ALERT_NOBUF; + char alert_str_lowered[512]; + // save 1 for the null terminator + size_t max_str_len = sizeof(alert_str_lowered) - 1; + + size_t which_alert; + size_t table_size = get_alert_table_size(); + size_t alert_len = strlen(alert_str); + + size_t i; + for (i = 0; i < alert_len && i < max_str_len; ++i) { + alert_str_lowered[i] = tolower(alert_str[i]); } + alert_str_lowered[i] = '\0'; + + alert_level_t search_key = { .name = alert_str, .mask_value = ALERT_NONE }; + + // The table is small, and performance isn't critical for loading + // debug options, so use linear search (lfind) + alert_level_t *table_element = + (alert_level_t*)lfind(&search_key, alert_table, &table_size, + sizeof(search_key), alert_name_comparison + ); + if (table_element != NULL) { + return table_element->mask_value; + } + fprintf(stderr, "Invalid CILK_ALERT value: %s\n", alert_str); return ALERT_NONE; } -static int parse_alert_level_env(const char *const alert_env) { - int new_alert_lvl = 0; - char curr_alert[ALERT_STR_BUF_LEN + 1]; - - size_t buf_str_len = 0; - - // We only allow numbers for backwards compatibility, so - // only allow numbers if they are the only thing passed to - // CILK_ALERT - bool numbers_allowed = true; - bool invalid = false; - - for (size_t i = 0; i < strlen(alert_env); ++i) { - if (alert_env[i] == ',') { - curr_alert[buf_str_len] = '\0'; - numbers_allowed = false; - if (invalid) { - fputc('\n', stderr); - invalid = false; - } else { - new_alert_lvl |= parse_alert_level_str(curr_alert); - } - buf_str_len = 0; - } else if (buf_str_len < ALERT_STR_BUF_LEN) { - curr_alert[buf_str_len++] = tolower(alert_env[i]); - } else { - curr_alert[buf_str_len] = '\0'; - fprintf(stderr, "%s%s%c", - // Only print the prefix the first time we get here. - invalid ? "" : "Invalid CILK_ALERT value: ", - curr_alert, - alert_env[i]); - curr_alert[0] = '\0'; - invalid = true; - } - } +static int parse_alert_level_env(char *alert_env) { + int new_alert_lvl = ALERT_NONE; - if (invalid) { - fputc('\n', stderr); - buf_str_len = 0; - } + size_t env_len = strlen(alert_env); - if (buf_str_len > 0) { - curr_alert[buf_str_len] = '\0'; + char *alert_str = strtok(alert_env, ","); - if (numbers_allowed) { + if (alert_str) { + if (strlen(alert_str) == env_len) { + // Can be a number char **tol_end = &alert_env; new_alert_lvl = strtol(alert_env, tol_end, 0); if (new_alert_lvl == 0 && (**tol_end != '\0' || *tol_end == alert_env)) { - new_alert_lvl |= parse_alert_level_str(curr_alert); + new_alert_lvl |= parse_alert_level_str(alert_str); } } else { - new_alert_lvl |= parse_alert_level_str(curr_alert); + while (alert_str != NULL) { + new_alert_lvl |= parse_alert_level_str(alert_str); + char *new_alert_str = strtok(NULL, ","); + // add back the delimiter that was removed by strtok; + // must be after the above strtok to avoid a segfault + // on some implementations of strtok + alert_str[strlen(alert_str)] = ','; + alert_str = new_alert_str; + } } } return new_alert_lvl; } -void set_alert_level_from_str(const char *const alert_env) { +void set_alert_level_from_str(char *alert_env) { if (alert_env) { int new_alert_lvl = parse_alert_level_env(alert_env); set_alert_level(new_alert_lvl); diff --git a/runtime/debug.h b/runtime/debug.h index a4d026b7..8b1d0f65 100644 --- a/runtime/debug.h +++ b/runtime/debug.h @@ -54,7 +54,7 @@ extern CHEETAH_INTERNAL unsigned int debug_level; // Unused: compiler inlines the stack frame creation // #define CILK_STACKFRAME_MAGIC 0xCAFEBABE -CHEETAH_INTERNAL void set_alert_level_from_str(const char *const); +CHEETAH_INTERNAL void set_alert_level_from_str(char *); CHEETAH_INTERNAL void set_alert_level(unsigned int); CHEETAH_INTERNAL void set_debug_level(unsigned int); CHEETAH_INTERNAL void flush_alert_log(void); From 1efd7c396c5b9ec876c84924f4060c823545a273 Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Fri, 23 Aug 2024 10:53:00 -0500 Subject: [PATCH 07/14] fix overwrite of null terminator --- runtime/debug.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/runtime/debug.c b/runtime/debug.c index 9099a983..479415ff 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -117,15 +117,19 @@ static int parse_alert_level_env(char *alert_env) { while (alert_str != NULL) { new_alert_lvl |= parse_alert_level_str(alert_str); char *new_alert_str = strtok(NULL, ","); - // add back the delimiter that was removed by strtok; - // must be after the above strtok to avoid a segfault - // on some implementations of strtok + // Do this after reading the next pointer to avoid + // (1) branching (if last string) and (2) segfaulting + // in strtok due to overwriting the null terminator. alert_str[strlen(alert_str)] = ','; alert_str = new_alert_str; } } } + // We may have overwritten NULL with , above; + // fix it here + alert_env[env_len] = '\0'; + return new_alert_lvl; } From 0bb595a98f01700e4e15354422fd0ab1195aafb8 Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Sat, 24 Aug 2024 08:06:07 -0500 Subject: [PATCH 08/14] strdup and strcasecmp and constness --- runtime/debug.c | 51 ++++++++++++------------------------------------- runtime/debug.h | 2 +- 2 files changed, 13 insertions(+), 40 deletions(-) diff --git a/runtime/debug.c b/runtime/debug.c index 479415ff..ff0024b1 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -9,6 +9,7 @@ #include #include #include +#include #if ALERT_LVL & (ALERT_CFRAME|ALERT_RETURN) unsigned int alert_level = 0; @@ -22,11 +23,9 @@ CHEETAH_INTERNAL unsigned int debug_level = 0; static size_t alert_log_size = 0, alert_log_offset = 0; static char *alert_log = NULL; -#define ALERT_STR_BUF_LEN 16 - typedef struct __alert_level_t { - const char *const name; - const int mask_value; + const char *name; + int mask_value; } alert_level_t; static const alert_level_t alert_table[] = { @@ -44,7 +43,6 @@ static const alert_level_t alert_table[] = { {"boot", ALERT_BOOT}, {"start", ALERT_START}, {"closure", ALERT_CLOSURE}, - // Must be last in the table {"nobuf", ALERT_NOBUF}, }; @@ -52,35 +50,14 @@ static int alert_name_comparison(const void *a, const void *b) { const alert_level_t *ala = (const alert_level_t*)a; const alert_level_t *alb = (const alert_level_t*)b; - return strcmp(ala->name, alb->name); -} - -static size_t get_alert_table_size() { - size_t s = 0; - - for (s = 0; alert_table[s].mask_value != ALERT_NOBUF; ++s) { - // no-op - } - - return s; + // TODO: If Windows, use _stricmp + return strcasecmp(ala->name, alb->name); } static int parse_alert_level_str(const char *const alert_str) { - char alert_str_lowered[512]; - // save 1 for the null terminator - size_t max_str_len = sizeof(alert_str_lowered) - 1; - - size_t which_alert; - size_t table_size = get_alert_table_size(); - size_t alert_len = strlen(alert_str); - - size_t i; - for (i = 0; i < alert_len && i < max_str_len; ++i) { - alert_str_lowered[i] = tolower(alert_str[i]); - } - alert_str_lowered[i] = '\0'; + size_t table_size = sizeof(alert_table) / sizeof(alert_table[0]); - alert_level_t search_key = { .name = alert_str, .mask_value = ALERT_NONE }; + const alert_level_t search_key = { .name = alert_str, .mask_value = ALERT_NONE }; // The table is small, and performance isn't critical for loading // debug options, so use linear search (lfind) @@ -114,14 +91,8 @@ static int parse_alert_level_env(char *alert_env) { new_alert_lvl |= parse_alert_level_str(alert_str); } } else { - while (alert_str != NULL) { + for (; alert_str != NULL; alert_str = strtok(NULL, ",")) { new_alert_lvl |= parse_alert_level_str(alert_str); - char *new_alert_str = strtok(NULL, ","); - // Do this after reading the next pointer to avoid - // (1) branching (if last string) and (2) segfaulting - // in strtok due to overwriting the null terminator. - alert_str[strlen(alert_str)] = ','; - alert_str = new_alert_str; } } } @@ -133,10 +104,12 @@ static int parse_alert_level_env(char *alert_env) { return new_alert_lvl; } -void set_alert_level_from_str(char *alert_env) { +void set_alert_level_from_str(const char *const alert_env) { if (alert_env) { - int new_alert_lvl = parse_alert_level_env(alert_env); + char *alert_env_cpy = strdup(alert_env); + int new_alert_lvl = parse_alert_level_env(alert_env_cpy); set_alert_level(new_alert_lvl); + free(alert_env_cpy); } } diff --git a/runtime/debug.h b/runtime/debug.h index 8b1d0f65..a4d026b7 100644 --- a/runtime/debug.h +++ b/runtime/debug.h @@ -54,7 +54,7 @@ extern CHEETAH_INTERNAL unsigned int debug_level; // Unused: compiler inlines the stack frame creation // #define CILK_STACKFRAME_MAGIC 0xCAFEBABE -CHEETAH_INTERNAL void set_alert_level_from_str(char *); +CHEETAH_INTERNAL void set_alert_level_from_str(const char *const); CHEETAH_INTERNAL void set_alert_level(unsigned int); CHEETAH_INTERNAL void set_debug_level(unsigned int); CHEETAH_INTERNAL void flush_alert_log(void); From a5a3a607ac69046f3b49a070a551c740356c5dd6 Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Sat, 24 Aug 2024 08:08:17 -0500 Subject: [PATCH 09/14] remove unecessary include --- runtime/debug.c | 1 - 1 file changed, 1 deletion(-) diff --git a/runtime/debug.c b/runtime/debug.c index ff0024b1..d932c49b 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -3,7 +3,6 @@ #include "global.h" #include -#include #include #include #include From c4088c6e392180dc4afe4d58f5469612dfac7e27 Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Sat, 24 Aug 2024 08:11:41 -0500 Subject: [PATCH 10/14] consistent null check --- runtime/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/debug.c b/runtime/debug.c index d932c49b..39fd5af4 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -90,7 +90,7 @@ static int parse_alert_level_env(char *alert_env) { new_alert_lvl |= parse_alert_level_str(alert_str); } } else { - for (; alert_str != NULL; alert_str = strtok(NULL, ",")) { + for (; alert_str; alert_str = strtok(NULL, ",")) { new_alert_lvl |= parse_alert_level_str(alert_str); } } From 2248ff591263edbe444764810f3d2efab0a9759f Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Sat, 24 Aug 2024 08:13:35 -0500 Subject: [PATCH 11/14] remove old env string fixing logic --- runtime/debug.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/runtime/debug.c b/runtime/debug.c index 39fd5af4..314f97a8 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -96,10 +96,6 @@ static int parse_alert_level_env(char *alert_env) { } } - // We may have overwritten NULL with , above; - // fix it here - alert_env[env_len] = '\0'; - return new_alert_lvl; } From ab7344e6a935893f43ce7ae965440d911a25d803 Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Tue, 27 Aug 2024 10:25:43 -0500 Subject: [PATCH 12/14] Documentation; use more portable malloc+strcpy compared to strdup --- runtime/debug.c | 91 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 73 insertions(+), 18 deletions(-) diff --git a/runtime/debug.c b/runtime/debug.c index 314f97a8..55e58771 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -8,7 +8,6 @@ #include #include #include -#include #if ALERT_LVL & (ALERT_CFRAME|ALERT_RETURN) unsigned int alert_level = 0; @@ -45,14 +44,35 @@ static const alert_level_t alert_table[] = { {"nobuf", ALERT_NOBUF}, }; -static int alert_name_comparison(const void *a, const void *b) { - const alert_level_t *ala = (const alert_level_t*)a; - const alert_level_t *alb = (const alert_level_t*)b; +/** + * Compare the name<\code> of the alert_level_t<\code> pointed to by + * left to the name<\code> of the alert_level_t<\code> + * pointed to by right<\code>, ignoring case. + * + * @param left alert_level_t<\code> to compare + * @param right alert_level_t<\code> to compare + * + * @return Ignoring case, returns negative if left->name < right->name, + * 0 if they are equal, or positive if left->name > right->name + **/ +static int alert_name_comparison(const void *left, const void *right) { + const alert_level_t *al_left = (const alert_level_t*)left; + const alert_level_t *al_right = (const alert_level_t*)right; // TODO: If Windows, use _stricmp - return strcasecmp(ala->name, alb->name); + return strcasecmp(al_left->name, al_right->name); } +/** + * Parse an alert level represented by a string into the proper bitmask value. + * If the string is not represented in alert_table<\code>, then prints + * an error and returns ALERT_NONE. + * + * @param alert_str A C string to attempt to parse + * + * @return The bitmask corresponding to alert_str<\code>, if in + * alert_table<\code>, else ALERT_NONE. + **/ static int parse_alert_level_str(const char *const alert_str) { size_t table_size = sizeof(alert_table) / sizeof(alert_table[0]); @@ -74,19 +94,46 @@ static int parse_alert_level_str(const char *const alert_str) { return ALERT_NONE; } -static int parse_alert_level_env(char *alert_env) { +/** + * Parse a CSV line representing which alert levels should be enabled, and + * return the bitmask representing all of the passed in options. If the CSV line + * is a single number, then treat that number as the bitmask. + * alert_csv<\code> is copied, as strtok<\code> is used, and it + * modifies its arguments. + * + * @param alert_csv A C string that is either a comma-separated list of alert + * level names -or- a single number. + * + * @return The bitmask representing the passed in + * alert_csv<\code>. If alert_csv<\code> cannot + * be copied, then returns the current + * alert_level<\code> value. + **/ +static int parse_alert_level_csv(const char *const alert_csv) { int new_alert_lvl = ALERT_NONE; - size_t env_len = strlen(alert_env); + size_t csv_len = strlen(alert_csv); - char *alert_str = strtok(alert_env, ","); + // strtok modifies the passed in string, so copy alert_csv and use + // the copy instead + char *alert_csv_cpy = malloc(csv_len + 1); + if (!alert_csv_cpy) { + // Non-critical error, so just print a warning + fprintf(stderr, "Cilk: unable to copy CILK_ALERT settings (%s)\n", + strerror(errno) + ); + return alert_level; + } + strcpy(alert_csv_cpy, alert_csv); + + char *alert_str = strtok(alert_csv_cpy, ","); if (alert_str) { - if (strlen(alert_str) == env_len) { - // Can be a number - char **tol_end = &alert_env; - new_alert_lvl = strtol(alert_env, tol_end, 0); - if (new_alert_lvl == 0 && (**tol_end != '\0' || *tol_end == alert_env)) { + if (strlen(alert_str) == csv_len) { + // Can be a number, as there is no other option in the string + char *tol_end = alert_csv_cpy; + new_alert_lvl = strtol(alert_csv_cpy, &tol_end, 0); + if (new_alert_lvl == 0 && (*tol_end != '\0' || tol_end == alert_csv_cpy)) { new_alert_lvl |= parse_alert_level_str(alert_str); } } else { @@ -96,15 +143,23 @@ static int parse_alert_level_env(char *alert_env) { } } + free(alert_csv_cpy); + return new_alert_lvl; } -void set_alert_level_from_str(const char *const alert_env) { - if (alert_env) { - char *alert_env_cpy = strdup(alert_env); - int new_alert_lvl = parse_alert_level_env(alert_env_cpy); +/** + * Parse a CSV line representing which alert levels should be enabled, and + * and set the current alert_level<\code> bitamsk based on the result. + * If the passed in C string is NULL, then no change is made. + * + * @param alert_csv A C string that is either a comma-separated list of alert + * level names -or- a single number. + **/ +void set_alert_level_from_str(const char *const alert_csv) { + if (alert_csv) { + int new_alert_lvl = parse_alert_level_csv(alert_csv); set_alert_level(new_alert_lvl); - free(alert_env_cpy); } } From 5308b8de0650085598a7c160d1839c81204f63b8 Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Tue, 27 Aug 2024 10:49:55 -0500 Subject: [PATCH 13/14] slighlty more comments --- runtime/debug.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/runtime/debug.c b/runtime/debug.c index 55e58771..ebd5707a 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -21,11 +21,20 @@ CHEETAH_INTERNAL unsigned int debug_level = 0; static size_t alert_log_size = 0, alert_log_offset = 0; static char *alert_log = NULL; +/** + * Represents a usable alert level with a human-readable + * name<\code> and the corresponding + * mask_value<\code> used by the runtime. + **/ typedef struct __alert_level_t { const char *name; int mask_value; } alert_level_t; +/** + * A table relating a human-readable alert level name to + * the corresponding bitfield value used by the runtime. + **/ static const alert_level_t alert_table[] = { {"none", ALERT_NONE}, {"fiber", ALERT_FIBER}, From 8b1069c8cdf2ddfb4e286a7fa70e60943a31f06a Mon Sep 17 00:00:00 2001 From: Kyle Singer Date: Tue, 17 Sep 2024 15:50:31 -0500 Subject: [PATCH 14/14] back to strdup --- runtime/debug.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/runtime/debug.c b/runtime/debug.c index ebd5707a..316ce6ce 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -125,7 +125,7 @@ static int parse_alert_level_csv(const char *const alert_csv) { // strtok modifies the passed in string, so copy alert_csv and use // the copy instead - char *alert_csv_cpy = malloc(csv_len + 1); + char *alert_csv_cpy = strdup(alert_csv); if (!alert_csv_cpy) { // Non-critical error, so just print a warning fprintf(stderr, "Cilk: unable to copy CILK_ALERT settings (%s)\n", @@ -133,7 +133,6 @@ static int parse_alert_level_csv(const char *const alert_csv) { ); return alert_level; } - strcpy(alert_csv_cpy, alert_csv); char *alert_str = strtok(alert_csv_cpy, ",");