From 9bdcf141aa56e7ed2e363d679f562c0656c127d4 Mon Sep 17 00:00:00 2001 From: Jonathan Peyton Date: Mon, 9 Jun 2025 15:08:41 -0500 Subject: [PATCH 1/2] [OpenMP] Fixup bugs found during fuzz testing A lot of these only trip when using sanitizers with the library. * Insert forgotten free()s * Change (-1) << amount to 0xffffffffu as left shifting a negative is UB * Fixup integer parser to return INT_MAX when parsing huge string of digits. e.g., 452523423423423423 returns INT_MAX. * Fixup range parsing for affinity mask so integer overflow does not occur * Don't assert when branch bits are 0, instead warn user that is invalid and use the default value. * Fixup kmp_set_defaults() so the C version only uses null terminated strings and the Fortran version uses the string + size version. * Make sure the KMP_ALIGN_ALLOC is power of two, otherwise use CACHE_LINE. * Disallow ability to set KMP_TASKING=1 (task barrier) this doesn't work and hasn't worked for a long time. * Limit KMP_HOT_TEAMS_MAX_LEVEL to 1024, an array is allocated based on this value. * Remove integer values for OMP_PROC_BIND. The specification only allows strings and CSV of strings. * Fix setting KMP_AFFINITY=disabled + OMP_DISPLAY_AFFINITY=TRUE --- openmp/runtime/src/include/omp_lib.F90.var | 5 +- openmp/runtime/src/include/omp_lib.h.var | 4 +- openmp/runtime/src/kmp_affinity.cpp | 24 ++++-- openmp/runtime/src/kmp_barrier.cpp | 31 +++++-- openmp/runtime/src/kmp_barrier.h | 3 +- openmp/runtime/src/kmp_ftn_entry.h | 18 ++-- openmp/runtime/src/kmp_i18n.cpp | 13 ++- openmp/runtime/src/kmp_lock.cpp | 1 + openmp/runtime/src/kmp_runtime.cpp | 25 ++++-- openmp/runtime/src/kmp_settings.cpp | 84 +++++++++---------- openmp/runtime/src/kmp_str.cpp | 16 ++++ .../runtime/test/env/check_certain_values.c | 35 ++++++++ openmp/runtime/test/tasking/no_task_barrier.c | 28 +++++++ 13 files changed, 210 insertions(+), 77 deletions(-) create mode 100644 openmp/runtime/test/env/check_certain_values.c create mode 100644 openmp/runtime/test/tasking/no_task_barrier.c diff --git a/openmp/runtime/src/include/omp_lib.F90.var b/openmp/runtime/src/include/omp_lib.F90.var index 3463b698291e1..a79470975a112 100644 --- a/openmp/runtime/src/include/omp_lib.F90.var +++ b/openmp/runtime/src/include/omp_lib.F90.var @@ -937,9 +937,8 @@ integer (kind=omp_integer_kind), value :: libnum end subroutine kmp_set_library - subroutine kmp_set_defaults(string) bind(c) - use, intrinsic :: iso_c_binding - character (kind=c_char) :: string(*) + subroutine kmp_set_defaults(string) + character (len=*) :: string end subroutine kmp_set_defaults function kmp_get_stacksize() bind(c) diff --git a/openmp/runtime/src/include/omp_lib.h.var b/openmp/runtime/src/include/omp_lib.h.var index 5793a3ac2e685..a50bb018c7cc3 100644 --- a/openmp/runtime/src/include/omp_lib.h.var +++ b/openmp/runtime/src/include/omp_lib.h.var @@ -1010,8 +1010,8 @@ integer (kind=omp_integer_kind), value :: libnum end subroutine kmp_set_library - subroutine kmp_set_defaults(string) bind(c) - character string(*) + subroutine kmp_set_defaults(string) + character (len=*) :: string end subroutine kmp_set_defaults function kmp_get_stacksize() bind(c) diff --git a/openmp/runtime/src/kmp_affinity.cpp b/openmp/runtime/src/kmp_affinity.cpp index a6065fe792d55..6bfdfbf2d3cdc 100644 --- a/openmp/runtime/src/kmp_affinity.cpp +++ b/openmp/runtime/src/kmp_affinity.cpp @@ -1396,11 +1396,13 @@ bool kmp_topology_t::filter_hw_subset() { // One last check that we shouldn't allow filtering entire machine if (num_filtered == num_hw_threads) { KMP_AFF_WARNING(__kmp_affinity, AffHWSubsetAllFiltered); + KMP_CPU_FREE(filtered_mask); return false; } // Apply the filter restrict_to_mask(filtered_mask); + KMP_CPU_FREE(filtered_mask); return true; } @@ -2225,7 +2227,7 @@ class cpuid_cache_info_t { cache_mask_width = __kmp_cpuid_mask_width(max_threads_sharing); cache_level = __kmp_extract_bits<5, 7>(buf2.eax); table[depth].level = cache_level; - table[depth].mask = ((-1) << cache_mask_width); + table[depth].mask = ((0xffffffffu) << cache_mask_width); depth++; level++; } @@ -2755,13 +2757,13 @@ static bool __kmp_x2apicid_get_levels(int leaf, cpuid_proc_info_t *info, // Set the masks to & with apicid for (unsigned i = 0; i < levels_index; ++i) { if (levels[i].level_type != INTEL_LEVEL_TYPE_INVALID) { - levels[i].mask = ~((-1) << levels[i].mask_width); - levels[i].cache_mask = (-1) << levels[i].mask_width; + levels[i].mask = ~((0xffffffffu) << levels[i].mask_width); + levels[i].cache_mask = (0xffffffffu) << levels[i].mask_width; for (unsigned j = 0; j < i; ++j) levels[i].mask ^= levels[j].mask; } else { KMP_DEBUG_ASSERT(i > 0); - levels[i].mask = (-1) << levels[i - 1].mask_width; + levels[i].mask = (0xffffffffu) << levels[i - 1].mask_width; levels[i].cache_mask = 0; } info->description.add(info->levels[i].level_type); @@ -4217,6 +4219,9 @@ static void __kmp_affinity_process_proclist(kmp_affinity_t &affinity) { if (stride > 0) { do { ADD_MASK_OSID(start, osId2Mask, maxOsId); + // Prevent possible overflow calculation + if (end - start < stride) + break; start += stride; } while (start <= end); } else { @@ -4238,6 +4243,7 @@ static void __kmp_affinity_process_proclist(kmp_affinity_t &affinity) { if (nextNewMask == 0) { *out_masks = NULL; KMP_CPU_INTERNAL_FREE_ARRAY(newMasks, numNewMasks); + KMP_CPU_FREE(sumMask); return; } KMP_CPU_ALLOC_ARRAY((*out_masks), nextNewMask); @@ -4406,6 +4412,7 @@ static void __kmp_process_place(const char **scan, kmp_affinity_t &affinity, (*scan)++; // skip '!' __kmp_process_place(scan, affinity, maxOsId, tempMask, setSize); KMP_CPU_COMPLEMENT(maxOsId, tempMask); + KMP_CPU_AND(tempMask, __kmp_affin_fullMask); } else if ((**scan >= '0') && (**scan <= '9')) { next = *scan; SKIP_DIGITS(next); @@ -4559,6 +4566,8 @@ void __kmp_affinity_process_placelist(kmp_affinity_t &affinity) { *out_numMasks = nextNewMask; if (nextNewMask == 0) { *out_masks = NULL; + KMP_CPU_FREE(tempMask); + KMP_CPU_FREE(previousMask); KMP_CPU_INTERNAL_FREE_ARRAY(newMasks, numNewMasks); return; } @@ -5280,13 +5289,18 @@ void __kmp_affinity_uninitialize(void) { if (affinity->os_id_masks != NULL) KMP_CPU_FREE_ARRAY(affinity->os_id_masks, affinity->num_os_id_masks); if (affinity->proclist != NULL) - __kmp_free(affinity->proclist); + KMP_INTERNAL_FREE(affinity->proclist); if (affinity->ids != NULL) __kmp_free(affinity->ids); if (affinity->attrs != NULL) __kmp_free(affinity->attrs); *affinity = KMP_AFFINITY_INIT(affinity->env_var); } + if (__kmp_affin_fullMask != NULL) { + KMP_CPU_FREE(__kmp_affin_fullMask); + __kmp_affin_fullMask = NULL; + } + __kmp_avail_proc = 0; if (__kmp_affin_origMask != NULL) { if (KMP_AFFINITY_CAPABLE()) { #if KMP_OS_AIX diff --git a/openmp/runtime/src/kmp_barrier.cpp b/openmp/runtime/src/kmp_barrier.cpp index d7ef57c608149..88a5cbb69ba87 100644 --- a/openmp/runtime/src/kmp_barrier.cpp +++ b/openmp/runtime/src/kmp_barrier.cpp @@ -205,6 +205,31 @@ void distributedBarrier::init(size_t nthr) { team_icvs = __kmp_allocate(sizeof(kmp_internal_control_t)); } +void distributedBarrier::deallocate(distributedBarrier *db) { + for (int i = 0; i < MAX_ITERS; ++i) { + if (db->flags[i]) + KMP_INTERNAL_FREE(db->flags[i]); + db->flags[i] = NULL; + } + if (db->go) { + KMP_INTERNAL_FREE(db->go); + db->go = NULL; + } + if (db->iter) { + KMP_INTERNAL_FREE(db->iter); + db->iter = NULL; + } + if (db->sleep) { + KMP_INTERNAL_FREE(db->sleep); + db->sleep = NULL; + } + if (db->team_icvs) { + __kmp_free(db->team_icvs); + db->team_icvs = NULL; + } + KMP_ALIGNED_FREE(db); +} + // This function is used only when KMP_BLOCKTIME is not infinite. // static void __kmp_dist_barrier_wakeup(enum barrier_type bt, kmp_team_t *team, @@ -1890,8 +1915,6 @@ static int __kmp_barrier_template(enum barrier_type bt, int gtid, int is_split, break; } case bp_hyper_bar: { - // don't set branch bits to 0; use linear - KMP_ASSERT(__kmp_barrier_gather_branch_bits[bt]); __kmp_hyper_barrier_gather(bt, this_thr, gtid, tid, reduce USE_ITT_BUILD_ARG(itt_sync_obj)); break; @@ -1902,8 +1925,6 @@ static int __kmp_barrier_template(enum barrier_type bt, int gtid, int is_split, break; } case bp_tree_bar: { - // don't set branch bits to 0; use linear - KMP_ASSERT(__kmp_barrier_gather_branch_bits[bt]); __kmp_tree_barrier_gather(bt, this_thr, gtid, tid, reduce USE_ITT_BUILD_ARG(itt_sync_obj)); break; @@ -2297,7 +2318,6 @@ void __kmp_join_barrier(int gtid) { break; } case bp_hyper_bar: { - KMP_ASSERT(__kmp_barrier_gather_branch_bits[bs_forkjoin_barrier]); __kmp_hyper_barrier_gather(bs_forkjoin_barrier, this_thr, gtid, tid, NULL USE_ITT_BUILD_ARG(itt_sync_obj)); break; @@ -2308,7 +2328,6 @@ void __kmp_join_barrier(int gtid) { break; } case bp_tree_bar: { - KMP_ASSERT(__kmp_barrier_gather_branch_bits[bs_forkjoin_barrier]); __kmp_tree_barrier_gather(bs_forkjoin_barrier, this_thr, gtid, tid, NULL USE_ITT_BUILD_ARG(itt_sync_obj)); break; diff --git a/openmp/runtime/src/kmp_barrier.h b/openmp/runtime/src/kmp_barrier.h index ae9b8d62f4c3d..ce6100acc008e 100644 --- a/openmp/runtime/src/kmp_barrier.h +++ b/openmp/runtime/src/kmp_barrier.h @@ -130,8 +130,7 @@ class distributedBarrier { d->init(nThreads); return d; } - - static void deallocate(distributedBarrier *db) { KMP_ALIGNED_FREE(db); } + static void deallocate(distributedBarrier *db); void update_num_threads(size_t nthr) { init(nthr); } diff --git a/openmp/runtime/src/kmp_ftn_entry.h b/openmp/runtime/src/kmp_ftn_entry.h index 59a9571d59534..2b0063eb23a0a 100644 --- a/openmp/runtime/src/kmp_ftn_entry.h +++ b/openmp/runtime/src/kmp_ftn_entry.h @@ -572,16 +572,14 @@ static void __kmp_fortran_strncpy_truncate(char *buffer, size_t buf_size, // Convert a Fortran string to a C string by adding null byte class ConvertedString { char *buf; - kmp_info_t *th; public: ConvertedString(char const *fortran_str, size_t size) { - th = __kmp_get_thread(); - buf = (char *)__kmp_thread_malloc(th, size + 1); + buf = (char *)KMP_INTERNAL_MALLOC(size + 1); KMP_STRNCPY_S(buf, size + 1, fortran_str, size); buf[size] = '\0'; } - ~ConvertedString() { __kmp_thread_free(th, buf); } + ~ConvertedString() { KMP_INTERNAL_FREE(buf); } const char *get() const { return buf; } }; #endif // KMP_STUB @@ -1495,10 +1493,18 @@ void FTN_STDCALL FTN_SET_DEFAULTS(char const *str #endif ) { #ifndef KMP_STUB + size_t sz; + char const *defaults = str; + #ifdef PASS_ARGS_BY_VALUE - int len = (int)KMP_STRLEN(str); + sz = KMP_STRLEN(str); +#else + sz = (size_t)len; + ConvertedString cstr(str, sz); + defaults = cstr.get(); #endif - __kmp_aux_set_defaults(str, len); + + __kmp_aux_set_defaults(defaults, sz); #endif } diff --git a/openmp/runtime/src/kmp_i18n.cpp b/openmp/runtime/src/kmp_i18n.cpp index a164aa180dd48..f93e2b9f9592f 100644 --- a/openmp/runtime/src/kmp_i18n.cpp +++ b/openmp/runtime/src/kmp_i18n.cpp @@ -791,8 +791,19 @@ void __kmp_msg(kmp_msg_severity_t severity, kmp_msg_t message, va_list args) { kmp_msg_t fmsg; // formatted message kmp_str_buf_t buffer; - if (severity != kmp_ms_fatal && __kmp_generate_warnings == kmp_warnings_off) + if (severity != kmp_ms_fatal && __kmp_generate_warnings == kmp_warnings_off) { + // Have to free all possible pre-allocated messages + // sent in through message and args + __kmp_str_free(&message.str); + for (;;) { + message = va_arg(args, kmp_msg_t); + if (message.type == kmp_mt_dummy && message.str == NULL) { + break; + } + __kmp_str_free(&message.str); + } return; // no reason to form a string in order to not print it + } __kmp_str_buf_init(&buffer); diff --git a/openmp/runtime/src/kmp_lock.cpp b/openmp/runtime/src/kmp_lock.cpp index 0ad14f862bcb9..3492d5e9d39ea 100644 --- a/openmp/runtime/src/kmp_lock.cpp +++ b/openmp/runtime/src/kmp_lock.cpp @@ -3458,6 +3458,7 @@ void __kmp_cleanup_indirect_user_locks() { } __kmp_free(ptr->table[row]); } + __kmp_free(ptr->table); kmp_indirect_lock_table_t *next_table = ptr->next_table; if (ptr != &__kmp_i_lock_table) __kmp_free(ptr); diff --git a/openmp/runtime/src/kmp_runtime.cpp b/openmp/runtime/src/kmp_runtime.cpp index 417eceb8ebecc..09b536367cf55 100644 --- a/openmp/runtime/src/kmp_runtime.cpp +++ b/openmp/runtime/src/kmp_runtime.cpp @@ -8325,6 +8325,7 @@ void __kmp_cleanup(void) { __kmp_free(ptr); ptr = next; } + __kmp_old_threads_list = NULL; #if KMP_USE_DYNAMIC_LOCK __kmp_cleanup_indirect_user_locks(); @@ -8332,7 +8333,7 @@ void __kmp_cleanup(void) { __kmp_cleanup_user_locks(); #endif #if OMPD_SUPPORT - if (ompd_state) { + if (ompd_env_block) { __kmp_free(ompd_env_block); ompd_env_block = NULL; ompd_env_block_size = 0; @@ -8358,6 +8359,8 @@ void __kmp_cleanup(void) { __kmp_nested_proc_bind.bind_types = NULL; __kmp_nested_proc_bind.size = 0; __kmp_nested_proc_bind.used = 0; + __kmp_dflt_team_nth = 0; + __kmp_dflt_team_nth_ub = 0; if (__kmp_affinity_format) { KMP_INTERNAL_FREE(__kmp_affinity_format); __kmp_affinity_format = NULL; @@ -8365,6 +8368,9 @@ void __kmp_cleanup(void) { __kmp_i18n_catclose(); + if (__kmp_nesting_nth_level) + KMP_INTERNAL_FREE(__kmp_nesting_nth_level); + #if KMP_USE_HIER_SCHED __kmp_hier_scheds.deallocate(); #endif @@ -8373,6 +8379,9 @@ void __kmp_cleanup(void) { __kmp_stats_fini(); #endif + __kmpc_destroy_allocator(KMP_GTID_SHUTDOWN, __kmp_def_allocator); + __kmp_def_allocator = omp_default_mem_alloc; + KA_TRACE(10, ("__kmp_cleanup: exit\n")); } @@ -8768,11 +8777,15 @@ static int __kmp_aux_capture_affinity_field(int gtid, const kmp_info_t *th, break; #if KMP_AFFINITY_SUPPORTED case 'A': { - kmp_str_buf_t buf; - __kmp_str_buf_init(&buf); - __kmp_affinity_str_buf_mask(&buf, th->th.th_affin_mask); - rc = __kmp_str_buf_print(field_buffer, format, buf.str); - __kmp_str_buf_free(&buf); + if (th->th.th_affin_mask) { + kmp_str_buf_t buf; + __kmp_str_buf_init(&buf); + __kmp_affinity_str_buf_mask(&buf, th->th.th_affin_mask); + rc = __kmp_str_buf_print(field_buffer, format, buf.str); + __kmp_str_buf_free(&buf); + } else { + rc = __kmp_str_buf_print(field_buffer, "%s", "disabled"); + } } break; #endif default: diff --git a/openmp/runtime/src/kmp_settings.cpp b/openmp/runtime/src/kmp_settings.cpp index 392a02ebbd9aa..54f2468768bb8 100644 --- a/openmp/runtime/src/kmp_settings.cpp +++ b/openmp/runtime/src/kmp_settings.cpp @@ -1158,7 +1158,6 @@ static void __kmp_parse_nested_num_threads(const char *var, const char *env, } if (!__kmp_dflt_max_active_levels_set && total > 1) __kmp_dflt_max_active_levels = KMP_MAX_ACTIVE_LEVELS_LIMIT; - KMP_DEBUG_ASSERT(total > 0); if (total <= 0) { KMP_WARNING(NthSyntaxError, var, env); return; @@ -1248,8 +1247,11 @@ static void __kmp_stg_parse_num_threads(char const *name, char const *value, // TODO: Remove this option. OMP_NUM_THREADS is a list of positive integers! if (!__kmp_strcasecmp_with_sentinel("all", value, 0)) { // The array of 1 element - __kmp_nested_nth.nth = (int *)KMP_INTERNAL_MALLOC(sizeof(int)); - __kmp_nested_nth.size = __kmp_nested_nth.used = 1; + if (!__kmp_nested_nth.nth) { + __kmp_nested_nth.nth = (int *)KMP_INTERNAL_MALLOC(sizeof(int)); + __kmp_nested_nth.size = 1; + } + __kmp_nested_nth.used = 1; __kmp_nested_nth.nth[0] = __kmp_dflt_team_nth = __kmp_dflt_team_nth_ub = __kmp_xproc; } else { @@ -1361,6 +1363,11 @@ static void __kmp_stg_parse_tasking(char const *name, char const *value, void *data) { __kmp_stg_parse_int(name, value, 0, (int)tskm_max, (int *)&__kmp_tasking_mode); + // KMP_TASKING=1 (task barrier) doesn't work anymore, change to task_teams (2) + if (__kmp_tasking_mode == tskm_extra_barrier) { + KMP_WARNING(StgInvalidValue, name, value); + __kmp_tasking_mode = tskm_task_teams; + } } // __kmp_stg_parse_tasking static void __kmp_stg_print_tasking(kmp_str_buf_t *buffer, char const *name, @@ -1511,8 +1518,8 @@ static void __kmp_stg_parse_hot_teams_level(char const *name, char const *value, KMP_WARNING(EnvParallelWarn, name); return; } // read value before first parallel only - __kmp_stg_parse_int(name, value, 0, KMP_MAX_ACTIVE_LEVELS_LIMIT, - &__kmp_hot_teams_max_level); + __kmp_stg_parse_int(name, value, 0, 1024, &__kmp_hot_teams_max_level); + } // __kmp_stg_parse_hot_teams_level static void __kmp_stg_print_hot_teams_level(kmp_str_buf_t *buffer, @@ -1678,6 +1685,11 @@ static void __kmp_stg_parse_align_alloc(char const *name, char const *value, void *data) { __kmp_stg_parse_size(name, value, CACHE_LINE, INT_MAX, NULL, &__kmp_align_alloc, 1); + // Must be power of 2 + if (__kmp_align_alloc == 0 || ((__kmp_align_alloc - 1) & __kmp_align_alloc)) { + KMP_WARNING(StgInvalidValue, name, value); + __kmp_align_alloc = CACHE_LINE; + } } // __kmp_stg_parse_align_alloc static void __kmp_stg_print_align_alloc(kmp_str_buf_t *buffer, char const *name, @@ -1710,15 +1722,16 @@ static void __kmp_stg_parse_barrier_branch_bit(char const *name, } else { __kmp_barrier_release_branch_bits[i] = (kmp_uint32)__kmp_str_to_int(comma + 1, 0); - - if (__kmp_barrier_release_branch_bits[i] > KMP_MAX_BRANCH_BITS) { + if (__kmp_barrier_release_branch_bits[i] == 0 || + __kmp_barrier_release_branch_bits[i] > KMP_MAX_BRANCH_BITS) { __kmp_msg(kmp_ms_warning, KMP_MSG(BarrReleaseValueInvalid, name, comma + 1), __kmp_msg_null); __kmp_barrier_release_branch_bits[i] = __kmp_barrier_release_bb_dflt; } } - if (__kmp_barrier_gather_branch_bits[i] > KMP_MAX_BRANCH_BITS) { + if (__kmp_barrier_gather_branch_bits[i] == 0 || + __kmp_barrier_gather_branch_bits[i] > KMP_MAX_BRANCH_BITS) { KMP_WARNING(BarrGatherValueInvalid, name, value); KMP_INFORM(Using_uint_Value, name, __kmp_barrier_gather_bb_dflt); __kmp_barrier_gather_branch_bits[i] = __kmp_barrier_gather_bb_dflt; @@ -2198,7 +2211,7 @@ static int __kmp_parse_affinity_proc_id_list(const char *var, const char *env, { ptrdiff_t len = next - env; - char *retlist = (char *)__kmp_allocate((len + 1) * sizeof(char)); + char *retlist = (char *)KMP_INTERNAL_MALLOC((len + 1) * sizeof(char)); KMP_MEMCPY_S(retlist, (len + 1) * sizeof(char), env, len * sizeof(char)); retlist[len] = '\0'; *proclist = retlist; @@ -3016,7 +3029,7 @@ static int __kmp_parse_place_list(const char *var, const char *env, { ptrdiff_t len = scan - env; - char *retlist = (char *)__kmp_allocate((len + 1) * sizeof(char)); + char *retlist = (char *)KMP_INTERNAL_MALLOC((len + 1) * sizeof(char)); KMP_MEMCPY_S(retlist, (len + 1) * sizeof(char), env, len * sizeof(char)); retlist[len] = '\0'; *place_list = retlist; @@ -3486,18 +3499,8 @@ static void __kmp_stg_parse_proc_bind(char const *name, char const *value, const char *buf = value; const char *next; - int num; + SKIP_WS(buf); - if ((*buf >= '0') && (*buf <= '9')) { - next = buf; - SKIP_DIGITS(next); - num = __kmp_str_to_int(buf, *next); - KMP_ASSERT(num >= 0); - buf = next; - SKIP_WS(buf); - } else { - num = -1; - } next = buf; if (__kmp_match_str("disabled", buf, &next)) { @@ -3508,8 +3511,7 @@ static void __kmp_stg_parse_proc_bind(char const *name, char const *value, #endif /* KMP_AFFINITY_SUPPORTED */ __kmp_nested_proc_bind.used = 1; __kmp_nested_proc_bind.bind_types[0] = proc_bind_false; - } else if ((num == (int)proc_bind_false) || - __kmp_match_str("false", buf, &next)) { + } else if (__kmp_match_str("false", buf, &next)) { buf = next; SKIP_WS(buf); #if KMP_AFFINITY_SUPPORTED @@ -3517,8 +3519,7 @@ static void __kmp_stg_parse_proc_bind(char const *name, char const *value, #endif /* KMP_AFFINITY_SUPPORTED */ __kmp_nested_proc_bind.used = 1; __kmp_nested_proc_bind.bind_types[0] = proc_bind_false; - } else if ((num == (int)proc_bind_true) || - __kmp_match_str("true", buf, &next)) { + } else if (__kmp_match_str("true", buf, &next)) { buf = next; SKIP_WS(buf); __kmp_nested_proc_bind.used = 1; @@ -3554,19 +3555,16 @@ static void __kmp_stg_parse_proc_bind(char const *name, char const *value, for (;;) { enum kmp_proc_bind_t bind; - if ((num == (int)proc_bind_primary) || - __kmp_match_str("master", buf, &next) || + if (__kmp_match_str("master", buf, &next) || __kmp_match_str("primary", buf, &next)) { buf = next; SKIP_WS(buf); bind = proc_bind_primary; - } else if ((num == (int)proc_bind_close) || - __kmp_match_str("close", buf, &next)) { + } else if (__kmp_match_str("close", buf, &next)) { buf = next; SKIP_WS(buf); bind = proc_bind_close; - } else if ((num == (int)proc_bind_spread) || - __kmp_match_str("spread", buf, &next)) { + } else if (__kmp_match_str("spread", buf, &next)) { buf = next; SKIP_WS(buf); bind = proc_bind_spread; @@ -3581,21 +3579,13 @@ static void __kmp_stg_parse_proc_bind(char const *name, char const *value, if (i >= nelem) { break; } - KMP_DEBUG_ASSERT(*buf == ','); + if (*buf != ',') { + KMP_WARNING(ParseExtraCharsWarn, name, buf); + while (*buf != ',') + buf++; + } buf++; SKIP_WS(buf); - - // Read next value if it was specified as an integer - if ((*buf >= '0') && (*buf <= '9')) { - next = buf; - SKIP_DIGITS(next); - num = __kmp_str_to_int(buf, *next); - KMP_ASSERT(num >= 0); - buf = next; - SKIP_WS(buf); - } else { - num = -1; - } } SKIP_WS(buf); } @@ -4536,6 +4526,10 @@ static void __kmp_stg_print_atomic_mode(kmp_str_buf_t *buffer, char const *name, static void __kmp_stg_parse_consistency_check(char const *name, char const *value, void *data) { + if (TCR_4(__kmp_init_serial)) { + KMP_WARNING(EnvSerialWarn, name); + return; + } // read value before serial initialization only if (!__kmp_strcasecmp_with_sentinel("all", value, 0)) { // Note, this will not work from kmp_set_defaults because th_cons stack was // not allocated @@ -4902,7 +4896,6 @@ static void __kmp_stg_parse_spin_backoff_params(const char *name, } } } - KMP_DEBUG_ASSERT(total > 0); if (total <= 0) { KMP_WARNING(EnvSyntaxError, name, value); return; @@ -4998,7 +4991,6 @@ static void __kmp_stg_parse_adaptive_lock_props(const char *name, } } } - KMP_DEBUG_ASSERT(total > 0); if (total <= 0) { KMP_WARNING(EnvSyntaxError, name, value); return; diff --git a/openmp/runtime/src/kmp_str.cpp b/openmp/runtime/src/kmp_str.cpp index 6ee2df724487c..12cce53074821 100644 --- a/openmp/runtime/src/kmp_str.cpp +++ b/openmp/runtime/src/kmp_str.cpp @@ -628,6 +628,11 @@ int __kmp_basic_str_to_int(char const *str) { for (t = str; *t != '\0'; ++t) { if (*t < '0' || *t > '9') break; + // Cap parsing to create largest integer + if (result >= (INT_MAX - (*t - '0')) / 10) { + result = INT_MAX; + break; + } result = (result * 10) + (*t - '0'); } @@ -643,9 +648,20 @@ int __kmp_str_to_int(char const *str, char sentinel) { for (t = str; *t != '\0'; ++t) { if (*t < '0' || *t > '9') break; + // Cap parsing to create largest integer + if (result >= (INT_MAX - (*t - '0')) / 10) { + result = INT_MAX; + break; + } result = (result * 10) + (*t - '0'); } + // Parse rest of large number by skipping the digits so t points to sentinel + if (result == INT_MAX) + for (t = str; *t != '\0'; ++t) + if (*t < '0' || *t > '9') + break; + switch (*t) { case '\0': /* the current default for no suffix is bytes */ factor = 1; diff --git a/openmp/runtime/test/env/check_certain_values.c b/openmp/runtime/test/env/check_certain_values.c new file mode 100644 index 0000000000000..99a7a6073e29e --- /dev/null +++ b/openmp/runtime/test/env/check_certain_values.c @@ -0,0 +1,35 @@ +// RUN: %libomp-compile +// RUN: env KMP_FORKJOIN_BARRIER=0,0 %libomp-run +// RUN: env KMP_PLAIN_BARRIER=0,0 %libomp-run +// RUN: env KMP_REDUCTION_BARRIER=0,0 %libomp-run +// RUN: env KMP_ALIGN_ALLOC=7 %libomp-run +// RUN: env KMP_ALIGN_ALLOC=8 %libomp-run +// RUN: env KMP_AFFINITY='explicit,proclist=[0-1222333333333444444]' %libomp-run +// RUN: env KMP_AFFINITY=disabled OMP_DISPLAY_AFFINITY=TRUE %libomp-run +// +// Test that certain environment variable values do not crash the runtime. +#include +#include + +int a = 0; + +int test() { + #pragma omp parallel reduction(+:a) + { + a += omp_get_thread_num(); + } + if (a == 0) { + // If the test passes, 'a' should not be zero + // because we are using reduction on thread numbers. + return 0; + } + return 1; +} + +int main(int argc, char **argv) { + int status = EXIT_SUCCESS; + if (!test()) { + status = EXIT_FAILURE; + } + return status; +} diff --git a/openmp/runtime/test/tasking/no_task_barrier.c b/openmp/runtime/test/tasking/no_task_barrier.c new file mode 100644 index 0000000000000..da2e99ee408f7 --- /dev/null +++ b/openmp/runtime/test/tasking/no_task_barrier.c @@ -0,0 +1,28 @@ +// RUN: %libomp-compile +// RUN: env KMP_TASKING=0 %libomp-run +// RUN: env KMP_TASKING=1 %libomp-run +// RUN: env KMP_TASKING=2 %libomp-run +// +// Test to make sure the KMP_TASKING=1 option doesn't crash +// Can use KMP_TASKING=0 (immediate exec) or 2 (defer to task queue +// and steal during regular barrier) but cannot use +// KMP_TASKING=1 (explicit tasking barrier before regular barrier) +#include +#include +#include +int main() { + int i; +#pragma omp parallel + { +#pragma omp single + { + for (i = 0; i < 10; i++) { +#pragma omp task + { + printf("Task %d executed by thread %d\n", i, omp_get_thread_num()); + } + } + } + } + return EXIT_SUCCESS; +} From 927735e7e3d62e1e0e20eeccb52f7199734fa290 Mon Sep 17 00:00:00 2001 From: Jonathan Peyton Date: Tue, 10 Jun 2025 09:41:05 -0500 Subject: [PATCH 2/2] Fix clang-format issue in test file --- openmp/runtime/test/env/check_certain_values.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmp/runtime/test/env/check_certain_values.c b/openmp/runtime/test/env/check_certain_values.c index 99a7a6073e29e..6d2623749ab0a 100644 --- a/openmp/runtime/test/env/check_certain_values.c +++ b/openmp/runtime/test/env/check_certain_values.c @@ -14,7 +14,7 @@ int a = 0; int test() { - #pragma omp parallel reduction(+:a) +#pragma omp parallel reduction(+ : a) { a += omp_get_thread_num(); }