[flang-commits] [flang] [Flang] Allow Intrinsic simpification with min/maxloc dim and scalar result (PR #76194)

[via flang-commits]

jeanPerier wrote:

Hi @davemgreen, I was able to reproduce the issue and this is indeed a bug with this patch.

For the following program, the simplified maxloc function stores into the result descriptor as if the result descriptor is a rank 1 array descriptor, but it is provided a scalar descriptor. This causes some memory that do not belong to the result descriptor to be written with bounds and stride values. The end-to-end bug is hard to reproduce because it depends on how the stack is laid out and if inlining occurs in LLVM it may "fix" the bug.

```
! src.f90
module test
contains
subroutine bug(a)
  real :: a (:)
  i = maxloc(a,dim=1)
  ! Memory bug around result descriptor of the simplified intrinsic function
  ! overwrites A descriptor and leads
  ! to an invalid a(i) address to be read inside crash.
  call crash(a(i))
end subroutine
end module

subroutine crash(x)
 real :: x
 print *, x
end subroutine

  use test
  real :: a(10) = 0.
  call bug(a)
end
```

Here is the following flow that expose the bug end-to-end (on X86-64 at least). The executable segfaults.
```
flang-new -fc1 -emit-fir src.f90 -o ok.fir
fir-opt --simplify-intrinsics ok.fir -o bad.fir
flang-new -fc1 -emit-llvm bad.fir && /proj/nv/llvm/Linux_x86_64/llvm-2409/bin/flang-new bad.ll
./a.out
```

In bad.fir, you can see that the bug is caused by the descriptor type mismatch between the `@_FortranAMaxlocDimx1_i32_f32_contract_simplified` and its call site:

```
 func.func @_QMtestPbug(%arg0: !fir.box<!fir.array<?xf32>> {fir.bindc_name = "a"}) {
    %0 = fir.alloca !fir.box<!fir.heap<i32>>
    // ...
    %9 = fir.convert %0 : (!fir.ref<!fir.box<!fir.heap<i32>>>) -> !fir.ref<!fir.box<none>>
    // ...
    fir.call @_FortranAMaxlocDimx1_i32_f32_contract_simplified(%9, %10, %12)
   // ...
  }
 func.func private @_FortranAMaxlocDimx1_i32_f32_contract_simplified(%arg0: !fir.ref<!fir.box<none>>, %arg1: !fir.box<none>, %arg2: !fir.box<none>) {
    // ... computing maxloc
    // inconsistent cast given the scalar descriptor passed on the call site.
    %11 = fir.convert %arg0 : (!fir.ref<!fir.box<none>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<1xi32>>>>
    fir.store %3 to %11 : !fir.ref<!fir.box<!fir.heap<!fir.array<1xi32>>>>
 }
```

Also, please beware that MAXLOC/MINLOC current runtime implementation is incorrect with regards to NANs and is being fixed in https://github.com/llvm/llvm-project/pull/76999. The inlined version should also behave correctly with NANs (I did not check if this is the case or not).

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