[flang-commits] [flang] [OpenMP][Flang] Fix OOB access for derived type mapping (PR #140948)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-fir-hlfir

Author: Akash Banerjee (TIFitis)

<details>
<summary>Changes</summary>

This patch fixes unintentional OOB acess when mapping members of derived type.

When `currentIndicesIdx` is `maxSize - 1`, i.e, it is the last member mapping, we can break from the loop. This was already the intended behaviour but using `continue` instead of `break` resulted in OOB access of `indices`.

This fixes the following test case:
```
module mod
   implicit none
   type :: mattype
      real(4), pointer :: array(:, :, :)
      integer(4) :: scalar
   end type
   type :: data
      type(mattype) :: memb
   end type
contains
   subroutine us_gpumem(dat)
      implicit none
      type(data), pointer :: dat
      !$omp target enter data map(to:dat%memb)
   end subroutine us_gpumem
end module mod
```

---
Full diff: https://github.com/llvm/llvm-project/pull/140948.diff


1 Files Affected:

- (modified) flang/lib/Lower/OpenMP/Utils.cpp (+12-10) 


``````````diff
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index 173dceb07b193..711d4af287691 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -362,16 +362,18 @@ mlir::Value createParentSymAndGenIntermediateMaps(
           clauseLocation, firOpBuilder.getRefType(memberTy), curValue,
           llvm::SmallVector<fir::IntOrValue, 1>{idxConst});
 
-      // Skip mapping and the subsequent load if we're the final member or not
-      // a type with a descriptor such as a pointer/allocatable. If we're a
-      // final member, the map will be generated by the processMap call that
-      // invoked this function, and if we're not a type with a descriptor then
-      // we have no need of generating an intermediate map for it, as we only
-      // need to generate a map if a member is a descriptor type (and thus
-      // obscures the members it contains via a pointer in which it's data needs
-      // mapped)
-      if ((currentIndicesIdx == indices.size() - 1) ||
-          !fir::isTypeWithDescriptor(memberTy)) {
+      // If we're a final member, the map will be generated by the processMap
+      // call that invoked this function.
+      if (currentIndicesIdx == indices.size() - 1)
+        break;
+
+      // Skip mapping and the subsequent load if we're not
+      // a type with a descriptor such as a pointer/allocatable. If we're not a
+      // type with a descriptor then we have no need of generating an
+      // intermediate map for it, as we only need to generate a map if a member
+      // is a descriptor type (and thus obscures the members it contains via a
+      // pointer in which it's data needs mapped).
+      if (!fir::isTypeWithDescriptor(memberTy)) {
         currentIndicesIdx++;
         continue;
       }

``````````

</details>


https://github.com/llvm/llvm-project/pull/140948