[OpenMP] Fixup bugs found during fuzz testing (#143455)

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

Author: Jonathan Peyton

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)

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)

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

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;

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); }
 

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
 }
 

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);
 

openmp/runtime/src/kmp_lock.cpp

@@ -3453,6 +3453,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);

openmp/runtime/src/kmp_runtime.cpp

@@ -8281,14 +8281,15 @@ void __kmp_cleanup(void) {
  __kmp_free(ptr);
  ptr = next;
  }
+ __kmp_old_threads_list = NULL;
 
 #if KMP_USE_DYNAMIC_LOCK
  __kmp_cleanup_indirect_user_locks();
 #else
  __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;
@@ -8314,13 +8315,18 @@ 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;
  }
 
  __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
@@ -8329,6 +8335,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"));
 }
 
@@ -8724,11 +8733,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: