[flang-commits] [flang] [flang] Fix segfault in CSHIFT/EOSHIFT with dynamically optional DIM (PR #184431)

[Sairudra More via flang-commits]

================
@@ -0,0 +1,52 @@
+! Test lowering of CSHIFT/EOSHIFT with a dynamically optional DIM argument.
+! When DIM is an optional dummy argument that is absent at runtime, it must
+! default to 1 (Fortran standard §16.9.68, §16.9.77). Prior to the fix, the
+! absent case caused a segmentation fault because the optional reference was
+! unconditionally dereferenced without checking for presence.
+! RUN: bbc -emit-hlfir -o - -I nowhere %s 2>&1 | FileCheck %s
+
+! CSHIFT 2D with optional DIM - DIM may be absent at runtime (defaults to 1).
+subroutine cshift_optional_dim(a, sh, dim)
+  integer :: a(:,:), sh(:)
+  integer, optional :: dim
+  a = CSHIFT(a, sh, dim)
+end subroutine
+! CHECK-LABEL: func.func @_QPcshift_optional_dim(
+! CHECK-SAME:    {{.*}}: !fir.ref<i32> {fir.bindc_name = "dim", fir.optional})
+! CHECK:         %[[DECL_DIM:.*]]:2 = hlfir.declare {{.*}} {fortran_attrs = #fir.var_attrs<optional>
+! CHECK:         %[[IS_PRESENT:.*]] = fir.is_present %[[DECL_DIM]]#0 : (!fir.ref<i32>) -> i1
+! CHECK:         %[[DIM_OR_ZERO:.*]] = fir.if %[[IS_PRESENT]] -> (i32) {
+! CHECK:           %[[LOADED:.*]] = fir.load %[[DECL_DIM]]#0 : !fir.ref<i32>
+! CHECK:           fir.result %[[LOADED]] : i32
+! CHECK:         } else {
+! CHECK:           arith.constant 0 : i32
+! CHECK:           fir.result
+! CHECK:         }
+! CHECK:         %[[ZERO:.*]] = arith.constant 0 : i32
+! CHECK:         %[[ONE:.*]] = arith.constant 1 : i32
+! CHECK:         %[[IS_ABSENT:.*]] = arith.cmpi eq, %[[DIM_OR_ZERO]], %[[ZERO]] : i32
----------------
Saieiei wrote:

Done.
I’ve reworked this to rely on the actual presence flag (`getIsPresent()` in a `fir.if`): we only take the default when DIM is truly absent, and if DIM is present we load and forward the real value unchanged, so `DIM=0` now correctly reaches flang-rt and triggers the expected runtime error (`CSHIFT: DIM=0 must be >= 1 ...`). While I was here, I also fixed a related type issue where the DIM path was hardcoded to `i32`. It now uses `fir::unwrapRefType(dimRef.getType())`, so `integer(kind=8)` optional DIM cases behave correctly too.
https://github.com/llvm/llvm-project/pull/184431/commits/784e752a855ef8a2fc10169c91a06b80d306ed6c

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