[OpenMP] libomptarget: Protect parent struct from being deallocated

[grokos]

Summary

Under certain circumstances, a parent struct may be deallocated accidentally when one of the mapped members is being unmapped. This patch fixes the problem.

[Hahnfeld]

LGTM - here we have the first problem with this legacy code :-(

[grokos]

Thanks for the review. As for the problem with legacy code, I'm now working on updating clang to use the new map interface, so this legacy code will soon be removed.

openmp/trunk/libomptarget/src/omptarget.cpp

static void translate_map(int32_t arg_num, void **args_base, void **args,
   combined_entry_t *cmb_entries =
       (combined_entry_t *) alloca(arg_num * sizeof(combined_entry_t));
   // number of combined entries
   long num_combined = 0;
   // old entry is MAP_PTR?
   bool *is_ptr_old = (bool *) alloca(arg_num * sizeof(bool));
   // old entry is member of member_of[old] cmb_entry
   int *member_of = (int *) alloca(arg_num * sizeof(int));
+  // temporary storage for modifications of the original arg_types
+  int32_t *mod_arg_types = (int32_t *) alloca(arg_num  *sizeof(int32_t));
 
   DP("Translating %d map entries\n", arg_num);
   for (int i = 0; i < arg_num; ++i) {
     member_of[i] = -1;
     is_ptr_old[i] = false;
+    mod_arg_types[i] = arg_types[i];
     // Scan previous entries to see whether this entry shares the same base
     for (int j = 0; j < i; ++j) {
       void *new_begin_addr = NULL;
       void *new_end_addr = NULL;
 
-      if (arg_types[i] & OMP_TGT_OLDMAPTYPE_MAP_PTR) {
+      if (mod_arg_types[i] & OMP_TGT_OLDMAPTYPE_MAP_PTR) {
         if (args_base[i] == args[j]) {
-          if (!(arg_types[j] & OMP_TGT_OLDMAPTYPE_MAP_PTR)) {
+          if (!(mod_arg_types[j] & OMP_TGT_OLDMAPTYPE_MAP_PTR)) {
             DP("Entry %d has the same base as entry %d's begin address\n", i,
                 j);
             new_begin_addr = args_base[i];
             new_end_addr = (char *)args_base[i] + sizeof(void *);
             assert(arg_sizes[j] == sizeof(void *));
             is_ptr_old[j] = true;
           } else {
             DP("Entry %d has the same base as entry %d's begin address, but "
                 "%d's base was a MAP_PTR too\n", i, j, j);
+            int32_t to_from_always_delete =
+                OMP_TGT_OLDMAPTYPE_TO | OMP_TGT_OLDMAPTYPE_FROM |
+                OMP_TGT_OLDMAPTYPE_ALWAYS | OMP_TGT_OLDMAPTYPE_DELETE;
+            if (mod_arg_types[j] & to_from_always_delete) {
+              DP("Resetting to/from/always/delete flags for entry %d because "
+                  "it is only a pointer to pointer\n", j);
+              mod_arg_types[j] &= ~to_from_always_delete;
+            }
           }
         }
       } else {
-        if (!(arg_types[i] & OMP_TGT_OLDMAPTYPE_FIRST_MAP) &&
+        if (!(mod_arg_types[i] & OMP_TGT_OLDMAPTYPE_FIRST_MAP) &&
             args_base[i] == args_base[j]) {
           DP("Entry %d has the same base address as entry %d\n", i, j);
           new_begin_addr = args[i];
           new_end_addr = (char *)args[i] + arg_sizes[i];
         }
       }
 
       // If we have combined the entry with a previous one
       if (new_begin_addr) {
         int id;
         if(member_of[j] == -1) {
           // We have a new entry
           id = num_combined++;
           DP("Creating new combined entry %d for old entry %d\n", id, j);
           // Initialize new entry
           cmb_entries[id].num_members = 1;
           cmb_entries[id].base_addr = args_base[j];
-          if (arg_types[j] & OMP_TGT_OLDMAPTYPE_MAP_PTR) {
+          if (mod_arg_types[j] & OMP_TGT_OLDMAPTYPE_MAP_PTR) {
             cmb_entries[id].begin_addr = args_base[j];
             cmb_entries[id].end_addr = (char *)args_base[j] + arg_sizes[j];
           } else {
             cmb_entries[id].begin_addr = args[j];
             cmb_entries[id].end_addr = (char *)args[j] + arg_sizes[j];
           }
           member_of[j] = id;
         } else {
for (int i = 0; i < arg_num; ++i) {
     // Now that the combined entry (the one the old entry was a member of) has
     // been inserted into the new arguments list, proceed with the old entry.
     int nid = next_id++;
     DP("Old entry %3d will become new entry %3d\n", i, nid);
 
     new_args_base[nid] = args_base[i];
     new_args[nid] = args[i];
     new_arg_sizes[nid] = arg_sizes[i];
-    int64_t old_type = arg_types[i];
+    int64_t old_type = mod_arg_types[i];
 
     if (is_ptr_old[i]) {
       // Reset TO and FROM flags
       old_type &= ~(OMP_TGT_OLDMAPTYPE_TO | OMP_TGT_OLDMAPTYPE_FROM);
     }
 
     if (member_of[i] == -1) {
       if (!is_target_construct)
for (int32_t i = arg_num - 1; i >= 0; --i) {
 
     // If PTR_AND_OBJ, HstPtrBegin is address of pointee
     void *TgtPtrBegin = Device.getTgtPtrBegin(HstPtrBegin, arg_sizes[i], IsLast,
         UpdateRef);
     DP("There are %" PRId64 " bytes allocated at target address " DPxMOD
         " - is%s last\n", arg_sizes[i], DPxPTR(TgtPtrBegin),
         (IsLast ? "" : " not"));
 
+    bool DelEntry = IsLast || ForceDelete;
+
     if ((arg_types[i] & OMP_TGT_MAPTYPE_MEMBER_OF) &&
         !(arg_types[i] & OMP_TGT_MAPTYPE_PTR_AND_OBJ)) {
-      IsLast = false; // protect parent struct from being deallocated
+      DelEntry = false; // protect parent struct from being deallocated
     }
 
-    bool DelEntry = IsLast || ForceDelete;
-
     if ((arg_types[i] & OMP_TGT_MAPTYPE_FROM) || DelEntry) {
       // Move data back to the host
       if (arg_types[i] & OMP_TGT_MAPTYPE_FROM) {
         bool Always = arg_types[i] & OMP_TGT_MAPTYPE_ALWAYS;
         bool CopyMember = false;
         if ((arg_types[i] & OMP_TGT_MAPTYPE_MEMBER_OF) &&
             !(arg_types[i] & OMP_TGT_MAPTYPE_PTR_AND_OBJ)) {
           // Copy data only if the "parent" struct has RefCount==1.