[flang-commits] [flang] [flang] Fix segfault in CSHIFT/EOSHIFT with dynamically optional DIM (PR #184431)
Sairudra More via flang-commits
flang-commits at lists.llvm.org
Wed Mar 4 07:47:44 PST 2026
https://github.com/Saieiei updated https://github.com/llvm/llvm-project/pull/184431
>From 784e752a855ef8a2fc10169c91a06b80d306ed6c Mon Sep 17 00:00:00 2001
From: Sairudra More <moresair at pe31.hpc.amslabs.hpecorp.net>
Date: Tue, 3 Mar 2026 12:03:38 -0600
Subject: [PATCH] [flang] Fix segfault in CSHIFT/EOSHIFT with dynamically
optional DIM
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When DIM is an optional dummy argument that is absent at runtime,
the HLFIR lowering of CSHIFT/EOSHIFT was unconditionally loading the
DIM reference without checking for presence, causing a null pointer
dereference (segfault).
Root cause: The argLowering rules for 'cshift'/'eoshift' did not mark
the 'dim' argument with handleDynamicOptional. As a result, in
genIntrinsicRef(), 'isPresent' was never populated for the DIM
PreparedActualArgument, causing handleDynamicOptional() to return false
in HlfirCShiftLowering/HlfirEOShiftLowering::lowerImpl(), which then
fell through to an unconditional loadTrivialScalar (fir.load) even
when DIM may be absent at runtime.
Fix:
1. Add handleDynamicOptional to the 'dim' argLowering rule in the
'cshift' and 'eoshift' entries in IntrinsicCall.cpp. This causes
genIntrinsicRef() to populate isPresent for the DIM argument,
which causes getOperandVector() to emit a guarded fir.if load
that returns 0 when DIM is absent.
2. Update HlfirCShiftLowering::lowerImpl and
HlfirEOShiftLowering::lowerImpl to detect the dynamic optional
case (loweredActuals[N]->handleDynamicOptional()) and replace the
0-when-absent value with 1, per Fortran standard §16.9.48 and
§16.9.77 which specify that absent DIM defaults to 1.
Fixes: CPE-15069 (optarg_cshift.f90 segfault)
---
flang/lib/Lower/HlfirIntrinsics.cpp | 57 +++++++++++++++++--
flang/lib/Optimizer/Builder/IntrinsicCall.cpp | 6 +-
.../test/Lower/HLFIR/cshift-optional-dim.f90 | 48 ++++++++++++++++
3 files changed, 105 insertions(+), 6 deletions(-)
create mode 100644 flang/test/Lower/HLFIR/cshift-optional-dim.f90
diff --git a/flang/lib/Lower/HlfirIntrinsics.cpp b/flang/lib/Lower/HlfirIntrinsics.cpp
index f63628d7824ff..f0af66ee0df7a 100644
--- a/flang/lib/Lower/HlfirIntrinsics.cpp
+++ b/flang/lib/Lower/HlfirIntrinsics.cpp
@@ -322,10 +322,11 @@ llvm::SmallVector<mlir::Value> HlfirTransformationalIntrinsic::getOperandVector(
valArg = loadBoxAddress(arg);
} else if (argRules.handleDynamicOptional) {
if (argRules.lowerAs == fir::LowerIntrinsicArgAs::Value) {
- if (arg->handleDynamicOptional())
+ if (arg->handleDynamicOptional()) {
valArg = loadOptionalValue(*arg);
- else
+ } else {
valArg = loadTrivialScalar(*arg);
+ }
} else {
TODO(loc, "hlfir transformational intrinsic dynamically optional "
"argument without box lowering");
@@ -489,6 +490,30 @@ mlir::Value HlfirCShiftLowering::lowerImpl(
if (!dim) {
// If DIM is not present, drop the last element which is a null Value.
operands.truncate(2);
+ } else if (loweredActuals[2] && loweredActuals[2]->handleDynamicOptional()) {
+ // DIM is a dynamically optional dummy argument.
+ // Use getIsPresent() directly (per eugeneepshteyn's review) rather than a
+ // 0-sentinel arithmetic check. Building our own fir.if ensures an
+ // explicitly-passed DIM=0 loads the actual value and reaches the runtime
+ // bounds check (per vzakhari's review), while an absent DIM defaults to 1
+ // per Fortran §16.9.68.
+ mlir::Value isPresent = loweredActuals[2]->getIsPresent();
+ hlfir::Entity dimEntity = loweredActuals[2]->getActual(loc, builder);
+ mlir::Value dimRef =
+ hlfir::derefPointersAndAllocatables(loc, builder, dimEntity);
+ mlir::Type dimType = fir::unwrapRefType(dimRef.getType());
+ dim = builder.genIfOp(loc, {dimType}, isPresent, /*withElseRegion=*/true)
+ .genThen([&]() {
+ mlir::Value loaded = fir::LoadOp::create(builder, loc, dimRef);
+ fir::ResultOp::create(builder, loc, loaded);
+ })
+ .genElse([&]() {
+ mlir::Value one =
+ builder.createIntegerConstant(loc, dimType, 1);
+ fir::ResultOp::create(builder, loc, one);
+ })
+ .getResults()[0];
+ operands[2] = dim;
} else {
// If DIM is present, then dereference it if it is a ref.
dim = hlfir::loadTrivialScalar(loc, builder, hlfir::Entity{dim});
@@ -509,9 +534,33 @@ mlir::Value HlfirEOShiftLowering::lowerImpl(
mlir::Value shift = operands[1];
mlir::Value boundary = operands[2];
mlir::Value dim = operands[3];
- // If DIM is present, then dereference it if it is a ref.
- if (dim)
+ if (loweredActuals[3] && loweredActuals[3]->handleDynamicOptional()) {
+ // DIM is a dynamically optional dummy argument.
+ // Use getIsPresent() directly (per eugeneepshteyn's review) rather than a
+ // 0-sentinel arithmetic check. Building our own fir.if ensures an
+ // explicitly-passed DIM=0 loads the actual value and reaches the runtime
+ // bounds check (per vzakhari's review), while an absent DIM defaults to 1
+ // per Fortran §16.9.77.
+ mlir::Value isPresent = loweredActuals[3]->getIsPresent();
+ hlfir::Entity dimEntity = loweredActuals[3]->getActual(loc, builder);
+ mlir::Value dimRef =
+ hlfir::derefPointersAndAllocatables(loc, builder, dimEntity);
+ mlir::Type dimType = fir::unwrapRefType(dimRef.getType());
+ dim = builder.genIfOp(loc, {dimType}, isPresent, /*withElseRegion=*/true)
+ .genThen([&]() {
+ mlir::Value loaded = fir::LoadOp::create(builder, loc, dimRef);
+ fir::ResultOp::create(builder, loc, loaded);
+ })
+ .genElse([&]() {
+ mlir::Value one =
+ builder.createIntegerConstant(loc, dimType, 1);
+ fir::ResultOp::create(builder, loc, one);
+ })
+ .getResults()[0];
+ } else if (dim) {
+ // If DIM is statically present, dereference it if it is a ref.
dim = hlfir::loadTrivialScalar(loc, builder, hlfir::Entity{dim});
+ }
mlir::Type resultType = computeResultType(array, stmtResultType);
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 090236fe3e5c6..e04649b600fd9 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -254,7 +254,9 @@ static constexpr IntrinsicHandler handlers[]{
/*isElemental=*/false},
{"cshift",
&I::genCshift,
- {{{"array", asAddr}, {"shift", asAddr}, {"dim", asValue}}},
+ {{{"array", asAddr},
+ {"shift", asAddr},
+ {"dim", asValue, handleDynamicOptional}}},
/*isElemental=*/false},
{"date_and_time",
&I::genDateAndTime,
@@ -281,7 +283,7 @@ static constexpr IntrinsicHandler handlers[]{
{{{"array", asBox},
{"shift", asAddr},
{"boundary", asBox, handleDynamicOptional},
- {"dim", asValue}}},
+ {"dim", asValue, handleDynamicOptional}}},
/*isElemental=*/false},
{"erfc_scaled", &I::genErfcScaled},
{"etime",
diff --git a/flang/test/Lower/HLFIR/cshift-optional-dim.f90 b/flang/test/Lower/HLFIR/cshift-optional-dim.f90
new file mode 100644
index 0000000000000..a5d1b12d90d96
--- /dev/null
+++ b/flang/test/Lower/HLFIR/cshift-optional-dim.f90
@@ -0,0 +1,48 @@
+! 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_VAL:.*]] = 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 1 : i32
+! CHECK: fir.result
+! CHECK: }
+! CHECK-NOT: arith.cmpi
+! CHECK-NOT: arith.select
+! CHECK: hlfir.cshift {{.*}} dim %[[DIM_VAL]] :
+
+! EOSHIFT 2D with optional DIM - DIM may be absent at runtime (defaults to 1).
+subroutine eoshift_optional_dim(a, sh, dim)
+ integer :: a(:,:), sh(:)
+ integer, optional :: dim
+ a = EOSHIFT(a, sh, dim=dim)
+end subroutine
+! CHECK-LABEL: func.func @_QPeoshift_optional_dim(
+! CHECK-SAME: {{.*}}: !fir.ref<i32> {fir.bindc_name = "dim", fir.optional})
+! CHECK: %[[DECL_DIM2:.*]]:2 = hlfir.declare {{.*}} {fortran_attrs = #fir.var_attrs<optional>
+! CHECK: %[[IS_PRESENT2:.*]] = fir.is_present %[[DECL_DIM2]]#0 : (!fir.ref<i32>) -> i1
+! CHECK: %[[DIM_VAL2:.*]] = fir.if %[[IS_PRESENT2]] -> (i32) {
+! CHECK: %[[LOADED2:.*]] = fir.load %[[DECL_DIM2]]#0 : !fir.ref<i32>
+! CHECK: fir.result %[[LOADED2]] : i32
+! CHECK: } else {
+! CHECK: arith.constant 1 : i32
+! CHECK: fir.result
+! CHECK: }
+! CHECK-NOT: arith.cmpi
+! CHECK-NOT: arith.select
+! CHECK: hlfir.eoshift {{.*}} dim %[[DIM_VAL2]] :
More information about the flang-commits
mailing list