This is an archive of the discontinued LLVM Phabricator instance.

[flang] Diagnostic for shape argument in c_f_pointer
ClosedPublic

Authored by realqhc on Nov 26 2022, 2:47 AM.

Details

Summary

Fix #59177, add check for dimensionality for shape argument against rank of FPTR argument in c_f_pointer

Diff Detail

Event Timeline

realqhc created this revision.Nov 26 2022, 2:47 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 26 2022, 2:47 AM
realqhc requested review of this revision.Nov 26 2022, 2:47 AM
peixin requested changes to this revision.Nov 28 2022, 12:03 AM

Please check Fortran 2018 18.2.3.3: SHAPE shall be present if and only if FPTR is an array; its size shall be equal to the rank of FPTR.

I guess you want to add the check when shape has constant size. Please notice that it cannot be checked when the size of shape is compile-time unknown.

This revision now requires changes to proceed.Nov 28 2022, 12:03 AM
realqhc updated this revision to Diff 478283.Nov 28 2022, 10:53 AM

fix the commit to correctly compare size of array against rank, note that expr->Rank() ==1 is required to fix a potential issue when a non-scalar shape is passed into the function, the program crashes after context.message

realqhc updated this revision to Diff 478286.Nov 28 2022, 11:00 AM

fix conflicting naming with previous part in the function

realqhc updated this revision to Diff 478496.Nov 29 2022, 2:45 AM

run clang-format

jeanPerier added inline comments.Dec 1 2022, 2:59 AM
flang/lib/Evaluate/intrinsics.cpp
2578–2579
2583

The message reads as if SHAPE's rank should be the same as FPTR's rank. I think it may be misleading given it is SHAPE's size that must be the equal to FPTR's rank.

jeanPerier added inline comments.Dec 1 2022, 3:00 AM
flang/lib/Evaluate/intrinsics.cpp
2578–2579

actually, also probably better to use *argExpr instead of *arguments[2] in the GetConstantShape call.

realqhc updated this revision to Diff 479415.Dec 1 2022, 1:43 PM

replace redundant if statements by GetConstantShape, change argument is GetConstantShape call, rewrite error message

realqhc marked 3 inline comments as done.Dec 1 2022, 1:44 PM
realqhc updated this revision to Diff 479424.Dec 1 2022, 2:04 PM

fix test case

realqhc updated this revision to Diff 479425.Dec 1 2022, 2:06 PM

add back the ERROR prefix for the test case

peixin added inline comments.Dec 1 2022, 6:11 PM
flang/lib/Evaluate/intrinsics.cpp
2577

Can you also add another check here?

if (argExpr->Rank() != 1)  {
  error: shape argument to c_f_pointer() must be a rank-one array.
} else {
  ...
}
2581
realqhc added inline comments.Dec 2 2022, 7:37 AM
flang/lib/Evaluate/intrinsics.cpp
2577

I'm afraid this will conflict with check-call error, as it also check if the argument is a scalar (albeit not checking the rank).

realqhc updated this revision to Diff 479641.Dec 2 2022, 7:50 AM

add check for size= array rank

realqhc marked an inline comment as done.Dec 2 2022, 7:51 AM
realqhc added inline comments.Dec 2 2022, 7:55 AM
flang/lib/Evaluate/intrinsics.cpp
2577
error: Semantic errors in test.f90
./test.f90:17:51: error: SHAPE= argument to C_F_POINTER() must be a rank-one array.
      call c_f_pointer(scalarC, multiDimIntF, shape=1)
                                                    ^
./test.f90:17:51: error: Whole scalar actual argument may not be associated with a dummy argument 'shape=' array
      call c_f_pointer(scalarC, multiDimIntF, shape=1)
                                                    ^

which will reult in two reports being issued for the same error.

peixin accepted this revision.Dec 2 2022, 7:36 PM

LGTM now except one clang-format problem in flang/lib/Evaluate/intrinsics.cpp.

flang/lib/Evaluate/intrinsics.cpp
2577

Thanks. This has already been checked elsewhere.

This revision is now accepted and ready to land.Dec 2 2022, 7:36 PM
realqhc updated this revision to Diff 479958.Dec 4 2022, 6:14 PM

run clang-format