Skip to content

Commit 4981bc2

Browse files
author
Jonathan Peyton
authored
[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
1 parent de011e3 commit 4981bc2

File tree

13 files changed

+210
-77
lines changed

13 files changed

+210
-77
lines changed

openmp/runtime/src/include/omp_lib.F90.var

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -937,9 +937,8 @@
937937
integer (kind=omp_integer_kind), value :: libnum
938938
end subroutine kmp_set_library
939939

940-
subroutine kmp_set_defaults(string) bind(c)
941-
use, intrinsic :: iso_c_binding
942-
character (kind=c_char) :: string(*)
940+
subroutine kmp_set_defaults(string)
941+
character (len=*) :: string
943942
end subroutine kmp_set_defaults
944943

945944
function kmp_get_stacksize() bind(c)

openmp/runtime/src/include/omp_lib.h.var

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,8 +1010,8 @@
10101010
integer (kind=omp_integer_kind), value :: libnum
10111011
end subroutine kmp_set_library
10121012

1013-
subroutine kmp_set_defaults(string) bind(c)
1014-
character string(*)
1013+
subroutine kmp_set_defaults(string)
1014+
character (len=*) :: string
10151015
end subroutine kmp_set_defaults
10161016

10171017
function kmp_get_stacksize() bind(c)

openmp/runtime/src/kmp_affinity.cpp

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1396,11 +1396,13 @@ bool kmp_topology_t::filter_hw_subset() {
13961396
// One last check that we shouldn't allow filtering entire machine
13971397
if (num_filtered == num_hw_threads) {
13981398
KMP_AFF_WARNING(__kmp_affinity, AffHWSubsetAllFiltered);
1399+
KMP_CPU_FREE(filtered_mask);
13991400
return false;
14001401
}
14011402

14021403
// Apply the filter
14031404
restrict_to_mask(filtered_mask);
1405+
KMP_CPU_FREE(filtered_mask);
14041406
return true;
14051407
}
14061408

@@ -2225,7 +2227,7 @@ class cpuid_cache_info_t {
22252227
cache_mask_width = __kmp_cpuid_mask_width(max_threads_sharing);
22262228
cache_level = __kmp_extract_bits<5, 7>(buf2.eax);
22272229
table[depth].level = cache_level;
2228-
table[depth].mask = ((-1) << cache_mask_width);
2230+
table[depth].mask = ((0xffffffffu) << cache_mask_width);
22292231
depth++;
22302232
level++;
22312233
}
@@ -2755,13 +2757,13 @@ static bool __kmp_x2apicid_get_levels(int leaf, cpuid_proc_info_t *info,
27552757
// Set the masks to & with apicid
27562758
for (unsigned i = 0; i < levels_index; ++i) {
27572759
if (levels[i].level_type != INTEL_LEVEL_TYPE_INVALID) {
2758-
levels[i].mask = ~((-1) << levels[i].mask_width);
2759-
levels[i].cache_mask = (-1) << levels[i].mask_width;
2760+
levels[i].mask = ~((0xffffffffu) << levels[i].mask_width);
2761+
levels[i].cache_mask = (0xffffffffu) << levels[i].mask_width;
27602762
for (unsigned j = 0; j < i; ++j)
27612763
levels[i].mask ^= levels[j].mask;
27622764
} else {
27632765
KMP_DEBUG_ASSERT(i > 0);
2764-
levels[i].mask = (-1) << levels[i - 1].mask_width;
2766+
levels[i].mask = (0xffffffffu) << levels[i - 1].mask_width;
27652767
levels[i].cache_mask = 0;
27662768
}
27672769
info->description.add(info->levels[i].level_type);
@@ -4217,6 +4219,9 @@ static void __kmp_affinity_process_proclist(kmp_affinity_t &affinity) {
42174219
if (stride > 0) {
42184220
do {
42194221
ADD_MASK_OSID(start, osId2Mask, maxOsId);
4222+
// Prevent possible overflow calculation
4223+
if (end - start < stride)
4224+
break;
42204225
start += stride;
42214226
} while (start <= end);
42224227
} else {
@@ -4238,6 +4243,7 @@ static void __kmp_affinity_process_proclist(kmp_affinity_t &affinity) {
42384243
if (nextNewMask == 0) {
42394244
*out_masks = NULL;
42404245
KMP_CPU_INTERNAL_FREE_ARRAY(newMasks, numNewMasks);
4246+
KMP_CPU_FREE(sumMask);
42414247
return;
42424248
}
42434249
KMP_CPU_ALLOC_ARRAY((*out_masks), nextNewMask);
@@ -4406,6 +4412,7 @@ static void __kmp_process_place(const char **scan, kmp_affinity_t &affinity,
44064412
(*scan)++; // skip '!'
44074413
__kmp_process_place(scan, affinity, maxOsId, tempMask, setSize);
44084414
KMP_CPU_COMPLEMENT(maxOsId, tempMask);
4415+
KMP_CPU_AND(tempMask, __kmp_affin_fullMask);
44094416
} else if ((**scan >= '0') && (**scan <= '9')) {
44104417
next = *scan;
44114418
SKIP_DIGITS(next);
@@ -4559,6 +4566,8 @@ void __kmp_affinity_process_placelist(kmp_affinity_t &affinity) {
45594566
*out_numMasks = nextNewMask;
45604567
if (nextNewMask == 0) {
45614568
*out_masks = NULL;
4569+
KMP_CPU_FREE(tempMask);
4570+
KMP_CPU_FREE(previousMask);
45624571
KMP_CPU_INTERNAL_FREE_ARRAY(newMasks, numNewMasks);
45634572
return;
45644573
}
@@ -5280,13 +5289,18 @@ void __kmp_affinity_uninitialize(void) {
52805289
if (affinity->os_id_masks != NULL)
52815290
KMP_CPU_FREE_ARRAY(affinity->os_id_masks, affinity->num_os_id_masks);
52825291
if (affinity->proclist != NULL)
5283-
__kmp_free(affinity->proclist);
5292+
KMP_INTERNAL_FREE(affinity->proclist);
52845293
if (affinity->ids != NULL)
52855294
__kmp_free(affinity->ids);
52865295
if (affinity->attrs != NULL)
52875296
__kmp_free(affinity->attrs);
52885297
*affinity = KMP_AFFINITY_INIT(affinity->env_var);
52895298
}
5299+
if (__kmp_affin_fullMask != NULL) {
5300+
KMP_CPU_FREE(__kmp_affin_fullMask);
5301+
__kmp_affin_fullMask = NULL;
5302+
}
5303+
__kmp_avail_proc = 0;
52905304
if (__kmp_affin_origMask != NULL) {
52915305
if (KMP_AFFINITY_CAPABLE()) {
52925306
#if KMP_OS_AIX

openmp/runtime/src/kmp_barrier.cpp

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,31 @@ void distributedBarrier::init(size_t nthr) {
205205
team_icvs = __kmp_allocate(sizeof(kmp_internal_control_t));
206206
}
207207

208+
void distributedBarrier::deallocate(distributedBarrier *db) {
209+
for (int i = 0; i < MAX_ITERS; ++i) {
210+
if (db->flags[i])
211+
KMP_INTERNAL_FREE(db->flags[i]);
212+
db->flags[i] = NULL;
213+
}
214+
if (db->go) {
215+
KMP_INTERNAL_FREE(db->go);
216+
db->go = NULL;
217+
}
218+
if (db->iter) {
219+
KMP_INTERNAL_FREE(db->iter);
220+
db->iter = NULL;
221+
}
222+
if (db->sleep) {
223+
KMP_INTERNAL_FREE(db->sleep);
224+
db->sleep = NULL;
225+
}
226+
if (db->team_icvs) {
227+
__kmp_free(db->team_icvs);
228+
db->team_icvs = NULL;
229+
}
230+
KMP_ALIGNED_FREE(db);
231+
}
232+
208233
// This function is used only when KMP_BLOCKTIME is not infinite.
209234
// static
210235
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,
18901915
break;
18911916
}
18921917
case bp_hyper_bar: {
1893-
// don't set branch bits to 0; use linear
1894-
KMP_ASSERT(__kmp_barrier_gather_branch_bits[bt]);
18951918
__kmp_hyper_barrier_gather(bt, this_thr, gtid, tid,
18961919
reduce USE_ITT_BUILD_ARG(itt_sync_obj));
18971920
break;
@@ -1902,8 +1925,6 @@ static int __kmp_barrier_template(enum barrier_type bt, int gtid, int is_split,
19021925
break;
19031926
}
19041927
case bp_tree_bar: {
1905-
// don't set branch bits to 0; use linear
1906-
KMP_ASSERT(__kmp_barrier_gather_branch_bits[bt]);
19071928
__kmp_tree_barrier_gather(bt, this_thr, gtid, tid,
19081929
reduce USE_ITT_BUILD_ARG(itt_sync_obj));
19091930
break;
@@ -2297,7 +2318,6 @@ void __kmp_join_barrier(int gtid) {
22972318
break;
22982319
}
22992320
case bp_hyper_bar: {
2300-
KMP_ASSERT(__kmp_barrier_gather_branch_bits[bs_forkjoin_barrier]);
23012321
__kmp_hyper_barrier_gather(bs_forkjoin_barrier, this_thr, gtid, tid,
23022322
NULL USE_ITT_BUILD_ARG(itt_sync_obj));
23032323
break;
@@ -2308,7 +2328,6 @@ void __kmp_join_barrier(int gtid) {
23082328
break;
23092329
}
23102330
case bp_tree_bar: {
2311-
KMP_ASSERT(__kmp_barrier_gather_branch_bits[bs_forkjoin_barrier]);
23122331
__kmp_tree_barrier_gather(bs_forkjoin_barrier, this_thr, gtid, tid,
23132332
NULL USE_ITT_BUILD_ARG(itt_sync_obj));
23142333
break;

openmp/runtime/src/kmp_barrier.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,7 @@ class distributedBarrier {
130130
d->init(nThreads);
131131
return d;
132132
}
133-
134-
static void deallocate(distributedBarrier *db) { KMP_ALIGNED_FREE(db); }
133+
static void deallocate(distributedBarrier *db);
135134

136135
void update_num_threads(size_t nthr) { init(nthr); }
137136

openmp/runtime/src/kmp_ftn_entry.h

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -572,16 +572,14 @@ static void __kmp_fortran_strncpy_truncate(char *buffer, size_t buf_size,
572572
// Convert a Fortran string to a C string by adding null byte
573573
class ConvertedString {
574574
char *buf;
575-
kmp_info_t *th;
576575

577576
public:
578577
ConvertedString(char const *fortran_str, size_t size) {
579-
th = __kmp_get_thread();
580-
buf = (char *)__kmp_thread_malloc(th, size + 1);
578+
buf = (char *)KMP_INTERNAL_MALLOC(size + 1);
581579
KMP_STRNCPY_S(buf, size + 1, fortran_str, size);
582580
buf[size] = '\0';
583581
}
584-
~ConvertedString() { __kmp_thread_free(th, buf); }
582+
~ConvertedString() { KMP_INTERNAL_FREE(buf); }
585583
const char *get() const { return buf; }
586584
};
587585
#endif // KMP_STUB
@@ -1495,10 +1493,18 @@ void FTN_STDCALL FTN_SET_DEFAULTS(char const *str
14951493
#endif
14961494
) {
14971495
#ifndef KMP_STUB
1496+
size_t sz;
1497+
char const *defaults = str;
1498+
14981499
#ifdef PASS_ARGS_BY_VALUE
1499-
int len = (int)KMP_STRLEN(str);
1500+
sz = KMP_STRLEN(str);
1501+
#else
1502+
sz = (size_t)len;
1503+
ConvertedString cstr(str, sz);
1504+
defaults = cstr.get();
15001505
#endif
1501-
__kmp_aux_set_defaults(str, len);
1506+
1507+
__kmp_aux_set_defaults(defaults, sz);
15021508
#endif
15031509
}
15041510

openmp/runtime/src/kmp_i18n.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -791,8 +791,19 @@ void __kmp_msg(kmp_msg_severity_t severity, kmp_msg_t message, va_list args) {
791791
kmp_msg_t fmsg; // formatted message
792792
kmp_str_buf_t buffer;
793793

794-
if (severity != kmp_ms_fatal && __kmp_generate_warnings == kmp_warnings_off)
794+
if (severity != kmp_ms_fatal && __kmp_generate_warnings == kmp_warnings_off) {
795+
// Have to free all possible pre-allocated messages
796+
// sent in through message and args
797+
__kmp_str_free(&message.str);
798+
for (;;) {
799+
message = va_arg(args, kmp_msg_t);
800+
if (message.type == kmp_mt_dummy && message.str == NULL) {
801+
break;
802+
}
803+
__kmp_str_free(&message.str);
804+
}
795805
return; // no reason to form a string in order to not print it
806+
}
796807

797808
__kmp_str_buf_init(&buffer);
798809

openmp/runtime/src/kmp_lock.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3453,6 +3453,7 @@ void __kmp_cleanup_indirect_user_locks() {
34533453
}
34543454
__kmp_free(ptr->table[row]);
34553455
}
3456+
__kmp_free(ptr->table);
34563457
kmp_indirect_lock_table_t *next_table = ptr->next_table;
34573458
if (ptr != &__kmp_i_lock_table)
34583459
__kmp_free(ptr);

openmp/runtime/src/kmp_runtime.cpp

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8281,14 +8281,15 @@ void __kmp_cleanup(void) {
82818281
__kmp_free(ptr);
82828282
ptr = next;
82838283
}
8284+
__kmp_old_threads_list = NULL;
82848285

82858286
#if KMP_USE_DYNAMIC_LOCK
82868287
__kmp_cleanup_indirect_user_locks();
82878288
#else
82888289
__kmp_cleanup_user_locks();
82898290
#endif
82908291
#if OMPD_SUPPORT
8291-
if (ompd_state) {
8292+
if (ompd_env_block) {
82928293
__kmp_free(ompd_env_block);
82938294
ompd_env_block = NULL;
82948295
ompd_env_block_size = 0;
@@ -8314,13 +8315,18 @@ void __kmp_cleanup(void) {
83148315
__kmp_nested_proc_bind.bind_types = NULL;
83158316
__kmp_nested_proc_bind.size = 0;
83168317
__kmp_nested_proc_bind.used = 0;
8318+
__kmp_dflt_team_nth = 0;
8319+
__kmp_dflt_team_nth_ub = 0;
83178320
if (__kmp_affinity_format) {
83188321
KMP_INTERNAL_FREE(__kmp_affinity_format);
83198322
__kmp_affinity_format = NULL;
83208323
}
83218324

83228325
__kmp_i18n_catclose();
83238326

8327+
if (__kmp_nesting_nth_level)
8328+
KMP_INTERNAL_FREE(__kmp_nesting_nth_level);
8329+
83248330
#if KMP_USE_HIER_SCHED
83258331
__kmp_hier_scheds.deallocate();
83268332
#endif
@@ -8329,6 +8335,9 @@ void __kmp_cleanup(void) {
83298335
__kmp_stats_fini();
83308336
#endif
83318337

8338+
__kmpc_destroy_allocator(KMP_GTID_SHUTDOWN, __kmp_def_allocator);
8339+
__kmp_def_allocator = omp_default_mem_alloc;
8340+
83328341
KA_TRACE(10, ("__kmp_cleanup: exit\n"));
83338342
}
83348343

@@ -8724,11 +8733,15 @@ static int __kmp_aux_capture_affinity_field(int gtid, const kmp_info_t *th,
87248733
break;
87258734
#if KMP_AFFINITY_SUPPORTED
87268735
case 'A': {
8727-
kmp_str_buf_t buf;
8728-
__kmp_str_buf_init(&buf);
8729-
__kmp_affinity_str_buf_mask(&buf, th->th.th_affin_mask);
8730-
rc = __kmp_str_buf_print(field_buffer, format, buf.str);
8731-
__kmp_str_buf_free(&buf);
8736+
if (th->th.th_affin_mask) {
8737+
kmp_str_buf_t buf;
8738+
__kmp_str_buf_init(&buf);
8739+
__kmp_affinity_str_buf_mask(&buf, th->th.th_affin_mask);
8740+
rc = __kmp_str_buf_print(field_buffer, format, buf.str);
8741+
__kmp_str_buf_free(&buf);
8742+
} else {
8743+
rc = __kmp_str_buf_print(field_buffer, "%s", "disabled");
8744+
}
87328745
} break;
87338746
#endif
87348747
default:

0 commit comments

Comments
 (0)