Skip to content

Commit 787eb0c

Browse files
[OpenMP] libomp cleanup: add check of input global tid parameter
Add check of negative gtid before indexing __kmp_threads. This makes static analyzers happier. This is the first part of the patch split in two parts. Differential Revision: https://reviews.llvm.org/D84062
1 parent 3ff220d commit 787eb0c

File tree

7 files changed

+57
-30
lines changed

7 files changed

+57
-30
lines changed

openmp/runtime/src/kmp.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3078,6 +3078,11 @@ static inline kmp_team_t *__kmp_team_from_gtid(int gtid) {
30783078
return __kmp_threads[gtid]->th.th_team;
30793079
}
30803080

3081+
static inline void __kmp_assert_valid_gtid(kmp_int32 gtid) {
3082+
if (UNLIKELY(gtid < 0 || gtid >= __kmp_threads_capacity))
3083+
KMP_FATAL(ThreadIdentInvalid);
3084+
}
3085+
30813086
/* ------------------------------------------------------------------------- */
30823087

30833088
extern kmp_global_t __kmp_global; /* global status */

openmp/runtime/src/kmp_csupport.cpp

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -231,21 +231,20 @@ void __kmpc_push_num_threads(ident_t *loc, kmp_int32 global_tid,
231231
kmp_int32 num_threads) {
232232
KA_TRACE(20, ("__kmpc_push_num_threads: enter T#%d num_threads=%d\n",
233233
global_tid, num_threads));
234-
234+
__kmp_assert_valid_gtid(global_tid);
235235
__kmp_push_num_threads(loc, global_tid, num_threads);
236236
}
237237

238238
void __kmpc_pop_num_threads(ident_t *loc, kmp_int32 global_tid) {
239239
KA_TRACE(20, ("__kmpc_pop_num_threads: enter\n"));
240-
241240
/* the num_threads are automatically popped */
242241
}
243242

244243
void __kmpc_push_proc_bind(ident_t *loc, kmp_int32 global_tid,
245244
kmp_int32 proc_bind) {
246245
KA_TRACE(20, ("__kmpc_push_proc_bind: enter T#%d proc_bind=%d\n", global_tid,
247246
proc_bind));
248-
247+
__kmp_assert_valid_gtid(global_tid);
249248
__kmp_push_proc_bind(loc, global_tid, (kmp_proc_bind_t)proc_bind);
250249
}
251250

@@ -353,7 +352,7 @@ void __kmpc_push_num_teams(ident_t *loc, kmp_int32 global_tid,
353352
KA_TRACE(20,
354353
("__kmpc_push_num_teams: enter T#%d num_teams=%d num_threads=%d\n",
355354
global_tid, num_teams, num_threads));
356-
355+
__kmp_assert_valid_gtid(global_tid);
357356
__kmp_push_num_teams(loc, global_tid, num_teams, num_threads);
358357
}
359358

@@ -474,9 +473,10 @@ conditional parallel region, like this,
474473
when the condition is false.
475474
*/
476475
void __kmpc_serialized_parallel(ident_t *loc, kmp_int32 global_tid) {
477-
// The implementation is now in kmp_runtime.cpp so that it can share static
478-
// functions with kmp_fork_call since the tasks to be done are similar in
479-
// each case.
476+
// The implementation is now in kmp_runtime.cpp so that it can share static
477+
// functions with kmp_fork_call since the tasks to be done are similar in
478+
// each case.
479+
__kmp_assert_valid_gtid(global_tid);
480480
#if OMPT_SUPPORT
481481
OMPT_STORE_RETURN_ADDRESS(global_tid);
482482
#endif
@@ -504,6 +504,7 @@ void __kmpc_end_serialized_parallel(ident_t *loc, kmp_int32 global_tid) {
504504
return;
505505

506506
// Not autopar code
507+
__kmp_assert_valid_gtid(global_tid);
507508
if (!TCR_4(__kmp_init_parallel))
508509
__kmp_parallel_initialize();
509510

@@ -713,6 +714,7 @@ Execute a barrier.
713714
void __kmpc_barrier(ident_t *loc, kmp_int32 global_tid) {
714715
KMP_COUNT_BLOCK(OMP_BARRIER);
715716
KC_TRACE(10, ("__kmpc_barrier: called T#%d\n", global_tid));
717+
__kmp_assert_valid_gtid(global_tid);
716718

717719
if (!TCR_4(__kmp_init_parallel))
718720
__kmp_parallel_initialize();
@@ -762,6 +764,7 @@ kmp_int32 __kmpc_master(ident_t *loc, kmp_int32 global_tid) {
762764
int status = 0;
763765

764766
KC_TRACE(10, ("__kmpc_master: called T#%d\n", global_tid));
767+
__kmp_assert_valid_gtid(global_tid);
765768

766769
if (!TCR_4(__kmp_init_parallel))
767770
__kmp_parallel_initialize();
@@ -816,7 +819,7 @@ thread that executes the <tt>master</tt> region.
816819
*/
817820
void __kmpc_end_master(ident_t *loc, kmp_int32 global_tid) {
818821
KC_TRACE(10, ("__kmpc_end_master: called T#%d\n", global_tid));
819-
822+
__kmp_assert_valid_gtid(global_tid);
820823
KMP_DEBUG_ASSERT(KMP_MASTER_GTID(global_tid));
821824
KMP_POP_PARTITIONED_TIMER();
822825

@@ -833,9 +836,6 @@ void __kmpc_end_master(ident_t *loc, kmp_int32 global_tid) {
833836
#endif
834837

835838
if (__kmp_env_consistency_check) {
836-
if (global_tid < 0)
837-
KMP_WARNING(ThreadIdentInvalid);
838-
839839
if (KMP_MASTER_GTID(global_tid))
840840
__kmp_pop_sync(global_tid, ct_master, loc);
841841
}
@@ -854,6 +854,7 @@ void __kmpc_ordered(ident_t *loc, kmp_int32 gtid) {
854854
KMP_DEBUG_ASSERT(__kmp_init_serial);
855855

856856
KC_TRACE(10, ("__kmpc_ordered: called T#%d\n", gtid));
857+
__kmp_assert_valid_gtid(gtid);
857858

858859
if (!TCR_4(__kmp_init_parallel))
859860
__kmp_parallel_initialize();
@@ -925,6 +926,7 @@ void __kmpc_end_ordered(ident_t *loc, kmp_int32 gtid) {
925926
kmp_info_t *th;
926927

927928
KC_TRACE(10, ("__kmpc_end_ordered: called T#%d\n", gtid));
929+
__kmp_assert_valid_gtid(gtid);
928930

929931
#if USE_ITT_BUILD
930932
__kmp_itt_ordered_end(gtid);
@@ -1147,7 +1149,7 @@ static kmp_user_lock_p __kmp_get_critical_section_ptr(kmp_critical_name *crit,
11471149
/*!
11481150
@ingroup WORK_SHARING
11491151
@param loc source location information.
1150-
@param global_tid global thread number .
1152+
@param global_tid global thread number.
11511153
@param crit identity of the critical section. This could be a pointer to a lock
11521154
associated with the critical section, or some other suitably unique value.
11531155
@@ -1170,6 +1172,7 @@ void __kmpc_critical(ident_t *loc, kmp_int32 global_tid,
11701172
kmp_user_lock_p lck;
11711173

11721174
KC_TRACE(10, ("__kmpc_critical: called T#%d\n", global_tid));
1175+
__kmp_assert_valid_gtid(global_tid);
11731176

11741177
// TODO: add THR_OVHD_STATE
11751178

@@ -1392,6 +1395,7 @@ void __kmpc_critical_with_hint(ident_t *loc, kmp_int32 global_tid,
13921395
#endif
13931396

13941397
KC_TRACE(10, ("__kmpc_critical: called T#%d\n", global_tid));
1398+
__kmp_assert_valid_gtid(global_tid);
13951399

13961400
kmp_dyna_lock_t *lk = (kmp_dyna_lock_t *)crit;
13971401
// Check if it is initialized.
@@ -1607,8 +1611,8 @@ this function.
16071611
*/
16081612
kmp_int32 __kmpc_barrier_master(ident_t *loc, kmp_int32 global_tid) {
16091613
int status;
1610-
16111614
KC_TRACE(10, ("__kmpc_barrier_master: called T#%d\n", global_tid));
1615+
__kmp_assert_valid_gtid(global_tid);
16121616

16131617
if (!TCR_4(__kmp_init_parallel))
16141618
__kmp_parallel_initialize();
@@ -1651,7 +1655,7 @@ still be waiting at the barrier and this call releases them.
16511655
*/
16521656
void __kmpc_end_barrier_master(ident_t *loc, kmp_int32 global_tid) {
16531657
KC_TRACE(10, ("__kmpc_end_barrier_master: called T#%d\n", global_tid));
1654-
1658+
__kmp_assert_valid_gtid(global_tid);
16551659
__kmp_end_split_barrier(bs_plain_barrier, global_tid);
16561660
}
16571661

@@ -1667,8 +1671,8 @@ There is no equivalent "end" function, since the
16671671
*/
16681672
kmp_int32 __kmpc_barrier_master_nowait(ident_t *loc, kmp_int32 global_tid) {
16691673
kmp_int32 ret;
1670-
16711674
KC_TRACE(10, ("__kmpc_barrier_master_nowait: called T#%d\n", global_tid));
1675+
__kmp_assert_valid_gtid(global_tid);
16721676

16731677
if (!TCR_4(__kmp_init_parallel))
16741678
__kmp_parallel_initialize();
@@ -1706,14 +1710,9 @@ kmp_int32 __kmpc_barrier_master_nowait(ident_t *loc, kmp_int32 global_tid) {
17061710
if (__kmp_env_consistency_check) {
17071711
/* there's no __kmpc_end_master called; so the (stats) */
17081712
/* actions of __kmpc_end_master are done here */
1709-
1710-
if (global_tid < 0) {
1711-
KMP_WARNING(ThreadIdentInvalid);
1712-
}
17131713
if (ret) {
17141714
/* only one thread should do the pop since only */
17151715
/* one did the push (see __kmpc_master()) */
1716-
17171716
__kmp_pop_sync(global_tid, ct_master, loc);
17181717
}
17191718
}
@@ -1734,6 +1733,7 @@ should introduce an explicit barrier if it is required.
17341733
*/
17351734

17361735
kmp_int32 __kmpc_single(ident_t *loc, kmp_int32 global_tid) {
1736+
__kmp_assert_valid_gtid(global_tid);
17371737
kmp_int32 rc = __kmp_enter_single(global_tid, loc, TRUE);
17381738

17391739
if (rc) {
@@ -1786,6 +1786,7 @@ only be called by the thread that executed the block of code protected
17861786
by the `single` construct.
17871787
*/
17881788
void __kmpc_end_single(ident_t *loc, kmp_int32 global_tid) {
1789+
__kmp_assert_valid_gtid(global_tid);
17891790
__kmp_exit_single(global_tid);
17901791
KMP_POP_PARTITIONED_TIMER();
17911792

@@ -2065,8 +2066,8 @@ void __kmpc_copyprivate(ident_t *loc, kmp_int32 gtid, size_t cpy_size,
20652066
void *cpy_data, void (*cpy_func)(void *, void *),
20662067
kmp_int32 didit) {
20672068
void **data_ptr;
2068-
20692069
KC_TRACE(10, ("__kmpc_copyprivate: called T#%d\n", gtid));
2070+
__kmp_assert_valid_gtid(gtid);
20702071

20712072
KMP_MB();
20722073

@@ -3382,6 +3383,7 @@ __kmpc_reduce_nowait(ident_t *loc, kmp_int32 global_tid, kmp_int32 num_vars,
33823383
kmp_team_t *team;
33833384
int teams_swapped = 0, task_state;
33843385
KA_TRACE(10, ("__kmpc_reduce_nowait() enter: called T#%d\n", global_tid));
3386+
__kmp_assert_valid_gtid(global_tid);
33853387

33863388
// why do we need this initialization here at all?
33873389
// Reduction clause can not be used as a stand-alone directive.
@@ -3535,6 +3537,7 @@ void __kmpc_end_reduce_nowait(ident_t *loc, kmp_int32 global_tid,
35353537
PACKED_REDUCTION_METHOD_T packed_reduction_method;
35363538

35373539
KA_TRACE(10, ("__kmpc_end_reduce_nowait() enter: called T#%d\n", global_tid));
3540+
__kmp_assert_valid_gtid(global_tid);
35383541

35393542
packed_reduction_method = __KMP_GET_REDUCTION_METHOD(global_tid);
35403543

@@ -3609,6 +3612,7 @@ kmp_int32 __kmpc_reduce(ident_t *loc, kmp_int32 global_tid, kmp_int32 num_vars,
36093612
int teams_swapped = 0, task_state;
36103613

36113614
KA_TRACE(10, ("__kmpc_reduce() enter: called T#%d\n", global_tid));
3615+
__kmp_assert_valid_gtid(global_tid);
36123616

36133617
// why do we need this initialization here at all?
36143618
// Reduction clause can not be a stand-alone directive.
@@ -3727,6 +3731,7 @@ void __kmpc_end_reduce(ident_t *loc, kmp_int32 global_tid,
37273731
int teams_swapped = 0, task_state;
37283732

37293733
KA_TRACE(10, ("__kmpc_end_reduce() enter: called T#%d\n", global_tid));
3734+
__kmp_assert_valid_gtid(global_tid);
37303735

37313736
th = __kmp_thread_from_gtid(global_tid);
37323737
teams_swapped = __kmp_swap_teams_for_teams_reduction(th, &team, &task_state);
@@ -3883,6 +3888,7 @@ e.g. for(i=2;i<9;i+=2) lo=2, up=8, st=2.
38833888
*/
38843889
void __kmpc_doacross_init(ident_t *loc, int gtid, int num_dims,
38853890
const struct kmp_dim *dims) {
3891+
__kmp_assert_valid_gtid(gtid);
38863892
int j, idx;
38873893
kmp_int64 last, trace_count;
38883894
kmp_info_t *th = __kmp_threads[gtid];
@@ -4002,6 +4008,7 @@ void __kmpc_doacross_init(ident_t *loc, int gtid, int num_dims,
40024008
}
40034009

40044010
void __kmpc_doacross_wait(ident_t *loc, int gtid, const kmp_int64 *vec) {
4011+
__kmp_assert_valid_gtid(gtid);
40054012
kmp_int32 shft, num_dims, i;
40064013
kmp_uint32 flag;
40074014
kmp_int64 iter_number; // iteration number of "collapsed" loop nest
@@ -4112,6 +4119,7 @@ void __kmpc_doacross_wait(ident_t *loc, int gtid, const kmp_int64 *vec) {
41124119
}
41134120

41144121
void __kmpc_doacross_post(ident_t *loc, int gtid, const kmp_int64 *vec) {
4122+
__kmp_assert_valid_gtid(gtid);
41154123
kmp_int32 shft, num_dims, i;
41164124
kmp_uint32 flag;
41174125
kmp_int64 iter_number; // iteration number of "collapsed" loop nest
@@ -4183,6 +4191,7 @@ void __kmpc_doacross_post(ident_t *loc, int gtid, const kmp_int64 *vec) {
41834191
}
41844192

41854193
void __kmpc_doacross_fini(ident_t *loc, int gtid) {
4194+
__kmp_assert_valid_gtid(gtid);
41864195
kmp_int32 num_done;
41874196
kmp_info_t *th = __kmp_threads[gtid];
41884197
kmp_team_t *team = th->th.th_team;

openmp/runtime/src/kmp_dispatch.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,7 @@ __kmp_dispatch_init(ident_t *loc, int gtid, enum sched_type schedule, T lb,
773773
sizeof(dispatch_private_info));
774774
KMP_BUILD_ASSERT(sizeof(dispatch_shared_info_template<UT>) ==
775775
sizeof(dispatch_shared_info));
776+
__kmp_assert_valid_gtid(gtid);
776777

777778
if (!TCR_4(__kmp_init_parallel))
778779
__kmp_parallel_initialize();
@@ -997,6 +998,7 @@ __kmp_dispatch_init(ident_t *loc, int gtid, enum sched_type schedule, T lb,
997998
template <typename UT>
998999
static void __kmp_dispatch_finish(int gtid, ident_t *loc) {
9991000
typedef typename traits_t<UT>::signed_t ST;
1001+
__kmp_assert_valid_gtid(gtid);
10001002
kmp_info_t *th = __kmp_threads[gtid];
10011003

10021004
KD_TRACE(100, ("__kmp_dispatch_finish: T#%d called\n", gtid));
@@ -1060,6 +1062,7 @@ static void __kmp_dispatch_finish(int gtid, ident_t *loc) {
10601062
template <typename UT>
10611063
static void __kmp_dispatch_finish_chunk(int gtid, ident_t *loc) {
10621064
typedef typename traits_t<UT>::signed_t ST;
1065+
__kmp_assert_valid_gtid(gtid);
10631066
kmp_info_t *th = __kmp_threads[gtid];
10641067

10651068
KD_TRACE(100, ("__kmp_dispatch_finish_chunk: T#%d called\n", gtid));
@@ -1900,6 +1903,7 @@ static int __kmp_dispatch_next(ident_t *loc, int gtid, kmp_int32 *p_last,
19001903

19011904
int status;
19021905
dispatch_private_info_template<T> *pr;
1906+
__kmp_assert_valid_gtid(gtid);
19031907
kmp_info_t *th = __kmp_threads[gtid];
19041908
kmp_team_t *team = th->th.th_team;
19051909

@@ -2192,6 +2196,7 @@ static void __kmp_dist_get_bounds(ident_t *loc, kmp_int32 gtid,
21922196
__kmp_error_construct(kmp_i18n_msg_CnsLoopIncrIllegal, ct_pdo, loc);
21932197
}
21942198
}
2199+
__kmp_assert_valid_gtid(gtid);
21952200
th = __kmp_threads[gtid];
21962201
team = th->th.th_team;
21972202
KMP_DEBUG_ASSERT(th->th.th_teams_microtask); // we are in the teams construct

openmp/runtime/src/kmp_error.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -415,9 +415,6 @@ void __kmp_pop_sync(int gtid, enum cons_type ct, ident_t const *ident) {
415415
__kmp_error_construct2(kmp_i18n_msg_CnsExpectedEnd, ct, ident,
416416
&p->stack_data[tos]);
417417
}
418-
if (gtid < 0) {
419-
__kmp_check_null_func();
420-
}
421418
KE_TRACE(100, (POP_MSG(p)));
422419
p->s_top = p->stack_data[tos].prev;
423420
p->stack_data[tos].type = ct_none;

openmp/runtime/src/kmp_sched.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ static void __kmp_for_static_init(ident_t *loc, kmp_int32 global_tid,
8585
kmp_uint32 nth;
8686
UT trip_count;
8787
kmp_team_t *team;
88+
__kmp_assert_valid_gtid(gtid);
8889
kmp_info_t *th = __kmp_threads[gtid];
8990

9091
#if OMPT_SUPPORT && OMPT_OPTIONAL
@@ -438,6 +439,7 @@ static void __kmp_dist_for_static_init(ident_t *loc, kmp_int32 gtid,
438439

439440
KMP_DEBUG_ASSERT(plastiter && plower && pupper && pupperDist && pstride);
440441
KE_TRACE(10, ("__kmpc_dist_for_static_init called (%d)\n", gtid));
442+
__kmp_assert_valid_gtid(gtid);
441443
#ifdef KMP_DEBUG
442444
{
443445
char *buff;
@@ -681,6 +683,7 @@ static void __kmp_team_static_init(ident_t *loc, kmp_int32 gtid,
681683

682684
KMP_DEBUG_ASSERT(p_last && p_lb && p_ub && p_st);
683685
KE_TRACE(10, ("__kmp_team_static_init called (%d)\n", gtid));
686+
__kmp_assert_valid_gtid(gtid);
684687
#ifdef KMP_DEBUG
685688
{
686689
char *buff;

openmp/runtime/src/kmp_taskdeps.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ kmp_int32 __kmpc_omp_task_with_deps(ident_t *loc_ref, kmp_int32 gtid,
514514
kmp_taskdata_t *new_taskdata = KMP_TASK_TO_TASKDATA(new_task);
515515
KA_TRACE(10, ("__kmpc_omp_task_with_deps(enter): T#%d loc=%p task=%p\n", gtid,
516516
loc_ref, new_taskdata));
517-
517+
__kmp_assert_valid_gtid(gtid);
518518
kmp_info_t *thread = __kmp_threads[gtid];
519519
kmp_taskdata_t *current_task = thread->th.th_current_task;
520520

@@ -677,7 +677,7 @@ void __kmpc_omp_wait_deps(ident_t *loc_ref, kmp_int32 gtid, kmp_int32 ndeps,
677677
gtid, loc_ref));
678678
return;
679679
}
680-
680+
__kmp_assert_valid_gtid(gtid);
681681
kmp_info_t *thread = __kmp_threads[gtid];
682682
kmp_taskdata_t *current_task = thread->th.th_current_task;
683683

0 commit comments

Comments
 (0)