flang
flang copied to clipboard
rewrite_sub_ast mishandles convert of a convert
When testing PR #962, I ran into a test that causes a crash in a debug build. The test case looks like this:
REAL, ALLOCATABLE :: CSUB(:,:,:)
REAL :: C(:,:,:,:)
INTEGER :: I
CSUB = ABS(I) * C(:,:,:,1)
END
After applying PR #962, this case crashes with the following call stack:
Program received signal SIGSEGV, Segmentation fault.
0x000000000042cd48 in mk_binop (optype=2, lop=16777217, rop=0, dtype=DT_INT8) at /home/bryanpkc/src/llvm/flang/tools/flang1/flang1exe/ast.c:690
690 if (A_TYPEG(lop) == A_TRIPLE || A_TYPEG(rop) == A_TRIPLE) {
(gdb) bt
#0 0x000000000042cd48 in mk_binop (optype=2, lop=16777217, rop=0, dtype=DT_INT8) at /home/bryanpkc/src/llvm/flang/tools/flang1/flang1exe/ast.c:690
#1 0x0000000000436efb in mk_extent_expr (lb=0, ub=16777217) at /home/bryanpkc/src/llvm/flang/tools/flang1/flang1exe/ast.c:2593
#2 0x00000000007c3fd4 in build_conformable_func_node (astdest=67, astsrc=74) at /home/bryanpkc/src/llvm/flang/tools/flang1/flang1exe/transfrm.c:3206
#3 0x00000000007c32ac in mk_conformable_test (dest=67, src=74, optype=22) at /home/bryanpkc/src/llvm/flang/tools/flang1/flang1exe/transfrm.c:3263
#4 0x00000000007cbe4d in rewrite_allocatable_assignment (astasgn=68, std=1, non_conformable=false, handle_alloc_members=false)
at /home/bryanpkc/src/llvm/flang/tools/flang1/flang1exe/transfrm.c:4555
#5 0x00000000007bfe33 in find_allocatable_assignment () at /home/bryanpkc/src/llvm/flang/tools/flang1/flang1exe/transfrm.c:4772
#6 0x00000000007bcfcf in transform () at /home/bryanpkc/src/llvm/flang/tools/flang1/flang1exe/transfrm.c:136
#7 0x00000000005b613a in main (argc=170, argv=0x7fffffffd908) at /home/bryanpkc/src/llvm/flang/tools/flang1/flang1exe/main.c:366
After some debugging, I found that the problem is caused by rewrite_calls duplicating some AST nodes incorrectly. The AST for the multiplication (minus the right operand for brevity) was originally:
ast:56 atype:1:ident hashlk/std:0 shape:0 dtype:6:integer sptr:658=i
ast:57 atype:1:ident hashlk/std:0 shape:0 dtype:9:real sptr:342=abs
ast:58 atype:14:intr-func-call hashlk/std:0 shape:0 dtype:6:integer lop:57 argcnt:1 args:1 optype:36 0:56
ast:64 atype:7:convert hashlk/std:0 shape:1 dtype:61:array deferred nobounds (z_b_10:z_b_11,z_b_13:z_b_14,z_b_16:z_b_17,z_b_19:z_b_20) of real lop:58
ast:65 atype:7:convert hashlk/std:0 shape:6 dtype:61:array deferred nobounds (z_b_10:z_b_11,z_b_13:z_b_14,z_b_16:z_b_17,z_b_19:z_b_20) of real lop:64
ast:66 atype:4:binop hashlk/std:0 shape:6 dtype:61:array deferred nobounds (z_b_10:z_b_11,z_b_13:z_b_14,z_b_16:z_b_17,z_b_19:z_b_20) of real optype:mul lop:65 rop:63
Note the two convert nodes. Both are generated by chkopnds which is meant to ensure that the operands to the multiplication have the same type and shape. It calls cngtyp to generate the first convert, then calls cngshape to generate the second.
During rewrite_calls, the re-constructed AST nodes become:
ast:71 atype:14:intr-func-call hashlk/std:0 shape:0 dtype:6:integer lop:57 argcnt:1 args:3 optype:36 0:56
ast:72 atype:7:convert hashlk/std:0 shape:1 dtype:61:array deferred nobounds (z_b_10:z_b_11,z_b_13:z_b_14,z_b_16:z_b_17,z_b_19:z_b_20) of real lop:71
ast:73 atype:7:convert hashlk/std:0 shape:1 dtype:61:array deferred nobounds (z_b_10:z_b_11,z_b_13:z_b_14,z_b_16:z_b_17,z_b_19:z_b_20) of real lop:72
ast:74 atype:4:binop hashlk/std:0 shape:1 dtype:61:array deferred nobounds (z_b_10:z_b_11,z_b_13:z_b_14,z_b_16:z_b_17,z_b_19:z_b_20) of real optype:mul lop:73 rop:63
rewrite_sub_ast is unable to deal with a convert of another convert with a different shape than expected. It calls mk_convert which incorrectly adopts the shape of the operand (i.e. shape 1 instead of shape 6). Apparently, mk_binop assumes that both of its operands have the same type and shape, and simply adopts the shape of its left operand, propagating the error that ultimately triggers the crash.
I came up with a localized fix in cngshape to merge the convert nodes if possible, which will avoid the problem for the above test case. However, a more fundamental fix in either rewrite_sub_ast or mk_convert seems to be necessary. mk_convert is called in a variety of contexts and changing it seems quite risky. I could attempt a fix of the A_CONV handling code in rewrite_sub_ast. What do you guys think?