[Openmp-commits] [openmp] 917f842 - [OpenMP] libomp cleanup: add checks of bad memory access

[via Openmp-commits]

Author: AndreyChurbanov
Date: 2020-07-21T00:12:46+03:00
New Revision: 917f8421591d2cc53c48ebd17d88b0dd3255aa2c

URL: https://github.com/llvm/llvm-project/commit/917f8421591d2cc53c48ebd17d88b0dd3255aa2c
DIFF: https://github.com/llvm/llvm-project/commit/917f8421591d2cc53c48ebd17d88b0dd3255aa2c.diff

LOG: [OpenMP] libomp cleanup: add checks of bad memory access

Add check of frm to prevent array out-of-bound access;
add check of new_nproc to prevent access of unallocated hot_teams array;
add check of location info pointer to prevent NULL dereference;
add check of d_tn pointer to prevent NULL dereference in release build.
These checks make static analyzers happier.

This is second part of the patch from https://reviews.llvm.org/D84062.

Added: 
    

Modified: 
    openmp/runtime/src/kmp_itt.inl
    openmp/runtime/src/kmp_runtime.cpp
    openmp/runtime/src/kmp_sched.cpp
    openmp/runtime/src/kmp_threadprivate.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/runtime/src/kmp_itt.inl b/openmp/runtime/src/kmp_itt.inl
index 04f00285ba35..29a05cb86a3f 100644
--- a/openmp/runtime/src/kmp_itt.inl
+++ b/openmp/runtime/src/kmp_itt.inl
@@ -230,8 +230,9 @@ LINKAGE void __kmp_itt_frame_submit(int gtid, __itt_timestamp begin,
       // Check if team size was changed. Then create new region domain for this
       // location
       unsigned int frm = (loc->reserved_2 & 0x0000FFFF) - 1;
-      if ((frm < KMP_MAX_FRAME_DOMAINS) &&
-          (__kmp_itt_region_team_size[frm] != team_size)) {
+      if (frm >= KMP_MAX_FRAME_DOMAINS)
+        return; // something's gone wrong, returning
+      if (__kmp_itt_region_team_size[frm] != team_size) {
         char *buff = NULL;
         kmp_str_loc_t str_loc = __kmp_str_loc_init(loc->psource, 1);
         buff = __kmp_str_format("%s$omp$parallel:%d@%s:%d:%d", str_loc.func,

diff  --git a/openmp/runtime/src/kmp_runtime.cpp b/openmp/runtime/src/kmp_runtime.cpp
index 00e0c7db4536..305ec4051cdb 100644
--- a/openmp/runtime/src/kmp_runtime.cpp
+++ b/openmp/runtime/src/kmp_runtime.cpp
@@ -4950,12 +4950,15 @@ __kmp_allocate_team(kmp_root_t *root, int new_nproc, int max_nproc,
     }
     hot_teams = master->th.th_hot_teams;
     if (level < __kmp_hot_teams_max_level && hot_teams &&
-        hot_teams[level]
-            .hot_team) { // hot team has already been allocated for given level
+        hot_teams[level].hot_team) {
+      // hot team has already been allocated for given level
       use_hot_team = 1;
     } else {
       use_hot_team = 0;
     }
+  } else {
+    // check we won't access uninitialized hot_teams, just in case
+    KMP_DEBUG_ASSERT(new_nproc == 1);
   }
 #endif
   // Optimization to use a "hot" team

diff  --git a/openmp/runtime/src/kmp_sched.cpp b/openmp/runtime/src/kmp_sched.cpp
index 129dc1946b5a..2d8f644c88b2 100644
--- a/openmp/runtime/src/kmp_sched.cpp
+++ b/openmp/runtime/src/kmp_sched.cpp
@@ -61,6 +61,12 @@ char const *traits_t<long>::spec = "ld";
 #define KMP_STATS_LOOP_END(stat) /* Nothing */
 #endif
 
+static ident_t loc_stub = {0, KMP_IDENT_KMPC, 0, 0, ";unknown;unknown;0;0;;"};
+static inline void check_loc(ident_t *&loc) {
+  if (loc == NULL)
+    loc = &loc_stub; // may need to report location info to ittnotify
+}
+
 template <typename T>
 static void __kmp_for_static_init(ident_t *loc, kmp_int32 global_tid,
                                   kmp_int32 schedtype, kmp_int32 *plastiter,
@@ -382,6 +388,7 @@ static void __kmp_for_static_init(ident_t *loc, kmp_int32 global_tid,
       __kmp_forkjoin_frames_mode == 3 && th->th.th_teams_microtask == NULL &&
       team->t.t_active_level == 1) {
     kmp_uint64 cur_chunk = chunk;
+    check_loc(loc);
     // Calculate chunk in case it was not specified; it is specified for
     // kmp_sch_static_chunked
     if (schedtype == kmp_sch_static) {

diff  --git a/openmp/runtime/src/kmp_threadprivate.cpp b/openmp/runtime/src/kmp_threadprivate.cpp
index 87bfff39e70e..a0256d6b9fc2 100644
--- a/openmp/runtime/src/kmp_threadprivate.cpp
+++ b/openmp/runtime/src/kmp_threadprivate.cpp
@@ -244,9 +244,8 @@ void __kmp_common_destroy_gtid(int gtid) {
 
         d_tn = __kmp_find_shared_task_common(&__kmp_threadprivate_d_table, gtid,
                                              tn->gbl_addr);
-
-        KMP_DEBUG_ASSERT(d_tn);
-
+        if (d_tn == NULL)
+          continue;
         if (d_tn->is_vec) {
           if (d_tn->dt.dtorv != 0) {
             (void)(*d_tn->dt.dtorv)(tn->par_addr, d_tn->vec_len);