[Openmp-commits] [openmp] r341371 - [libomptarget] PR38704: Fix erase of ShadowPtrMap

[Jonas Hahnfeld via Openmp-commits]

Author: hahnfeld
Date: Tue Sep  4 08:13:23 2018
New Revision: 341371

URL: http://llvm.org/viewvc/llvm-project?rev=341371&view=rev
Log:
[libomptarget] PR38704: Fix erase of ShadowPtrMap

erase() invalidates the iterator and returns a new one pointing
to the following element. The code now follows the example at
https://en.cppreference.com/w/cpp/container/map/erase.
(The added testcase crashes without this patch.)

Reported by David Binderman (https://llvm.org/PR38704)!

Differential Revision: https://reviews.llvm.org/D51623

Added:
    openmp/trunk/libomptarget/test/mapping/
    openmp/trunk/libomptarget/test/mapping/pr38704.c
Modified:
    openmp/trunk/libomptarget/src/omptarget.cpp

Modified: openmp/trunk/libomptarget/src/omptarget.cpp
URL: http://llvm.org/viewvc/llvm-project/openmp/trunk/libomptarget/src/omptarget.cpp?rev=341371&r1=341370&r2=341371&view=diff
==============================================================================
--- openmp/trunk/libomptarget/src/omptarget.cpp (original)
+++ openmp/trunk/libomptarget/src/omptarget.cpp Tue Sep  4 08:13:23 2018
@@ -419,7 +419,7 @@ int target_data_end(DeviceTy &Device, in
       uintptr_t ub = (uintptr_t) HstPtrBegin + data_size;
       Device.ShadowMtx.lock();
       for (ShadowPtrListTy::iterator it = Device.ShadowPtrMap.begin();
-          it != Device.ShadowPtrMap.end(); ++it) {
+           it != Device.ShadowPtrMap.end();) {
         void **ShadowHstPtrAddr = (void**) it->first;
 
         // An STL map is sorted on its keys; use this property
@@ -439,7 +439,9 @@ int target_data_end(DeviceTy &Device, in
         // If the struct is to be deallocated, remove the shadow entry.
         if (DelEntry) {
           DP("Removing shadow pointer " DPxMOD "\n", DPxPTR(ShadowHstPtrAddr));
-          Device.ShadowPtrMap.erase(it);
+          it = Device.ShadowPtrMap.erase(it);
+        } else {
+          ++it;
         }
       }
       Device.ShadowMtx.unlock();

Added: openmp/trunk/libomptarget/test/mapping/pr38704.c
URL: http://llvm.org/viewvc/llvm-project/openmp/trunk/libomptarget/test/mapping/pr38704.c?rev=341371&view=auto
==============================================================================
--- openmp/trunk/libomptarget/test/mapping/pr38704.c (added)
+++ openmp/trunk/libomptarget/test/mapping/pr38704.c Tue Sep  4 08:13:23 2018
@@ -0,0 +1,41 @@
+// RUN: %libomptarget-compile-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compile-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compile-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compile-run-and-check-x86_64-pc-linux-gnu
+
+// Clang 6.0 doesn't use the new map interface, undefined behavior when
+// the compiler emits "old" interface code for structures.
+// UNSUPPORTED: clang-6
+
+#include <stdio.h>
+#include <stdlib.h>
+
+typedef struct {
+  int *ptr1;
+  int *ptr2;
+} StructWithPtrs;
+
+int main(int argc, char *argv[]) {
+  StructWithPtrs s;
+  s.ptr1 = malloc(sizeof(int));
+  s.ptr2 = malloc(2 * sizeof(int));
+
+#pragma omp target map(s.ptr1[0:1], s.ptr2[0:2])
+  {
+    s.ptr1[0] = 1;
+    s.ptr2[0] = 2;
+    s.ptr2[1] = 3;
+  }
+
+  // CHECK: s.ptr1[0] = 1
+  // CHECK: s.ptr2[0] = 2
+  // CHECK: s.ptr2[1] = 3
+  printf("s.ptr1[0] = %d\n", s.ptr1[0]);
+  printf("s.ptr2[0] = %d\n", s.ptr2[0]);
+  printf("s.ptr2[1] = %d\n", s.ptr2[1]);
+
+  free(s.ptr1);
+  free(s.ptr2);
+
+  return 0;
+}