Skip to content

Commit

Permalink
[NARS1] [estess] Do not play with triple chains in case of compile-ti…
Browse files Browse the repository at this point in the history
…me errors as they could cause SIG-11 #90

A few issues related to compile-time errors were discovered.

1) The below M program correctly issues a PATNOTFOUND error when compiling. But if one tries to run the compiled object code (which should be okay since the execution does not reach the portions of the M code where the compiler error was found), a SIG-11 is observed. This happens only with GT.M V63002 (and in turn YottaDB r1.10) but not with V63001A (and in turn YottaDB r1.00).

```
> cat test.m
main    ;
        do good
        quit
bad     ;
        if 1?1B
        quit
good    ;
        write "hello",!
        quit
```

Related to the above, the below M program test1.m produces a GTMASSERT when run with a debug build. Unlike the previous test case (test.m), the production build did not have problems with test1.m.

```
> cat test1.m
        if 1?1B

> $gtm_dist/mumps -run test1
%GTM-F-GTMASSERT, GT.M V6.3-002 Linux x86_64 - Assert failed /Distrib/GT.M/V63002/sr_port/chktchain.c line 28
```

The primary issue in both the above tests was in bx_boollit() which noticed a pattern match operator usage with both operands being literals and hence invoked do_patfixed() which encountered a PATNOTFOUND error. That caused ins_errtriple() to be invoked which in turn removed all triples corresponding to the current M line (dqdelchain() call) and returned back to bx_boollit() which did not realize this and went ahead with manipulating the triple chains (dqrins() call etc.) and returned to its caller bool_expr() which also did triple chain manipulation (dqdel() call etc.) all the while operating on triples that were no longer part of the execution chain (due to the prior delqchain() call). This caused a corruption in the doubly-linked triple list in "t_orig" which resulted in incorrect object code being generated that later ended up as the SIG-11 when one tried running this M program.

In GT.M V63002, boolean expression evaluation and literal optimization got a significant rework. As part of that change, the macros RETURN_IF_RTS_ERROR and RETURN_EXPR_IF_RTS_ERROR were introduced to check for compile-time errors and if so return from functions right away instead of manipulating triple chains. These safety checks needed to be added in a few more places. That fixed the primary issue.

2) In addition, it was noticed that the following M program fails an assert when run with the debug build.

```
> cat test2.m
        xecute "if ""a""?1B"

> mumps -run test2
%GTM-F-ASSERT, Assert failed in /Distrib/GT.M/V63002/sr_port/zlcompile.c line 81 for expression ((FALSE == run_time) && (TRUE == TREF(compile_time)))
```

Below is the corresponding C-stack.

```
 #0  0x00007ff2e6988767 in kill () at ../sysdeps/unix/syscall-template.S:84
 #1  0x00007ff2e6014a5c in gtm_dump_core () at /Distrib/GT.M/V63002/sr_unix/gtm_dump_core.c:69
 #2  0x00007ff2e5f1de97 in gtm_fork_n_core () at /Distrib/GT.M/V63002/sr_unix/gtm_fork_n_core.c:211
 #3  0x00007ff2e6007f2b in ch_cond_core () at /Distrib/GT.M/V63002/sr_unix/ch_cond_core.c:59
 #4  0x00007ff2e5f443a2 in rts_error_va (csa=0x0, argcnt=7, var=0x7ffffc4b0a90) at /Distrib/GT.M/V63002/sr_unix/rts_error.c:153
 #5  0x00007ff2e5f439b8 in rts_error_csa (csa=0x0, argcnt=7) at /Distrib/GT.M/V63002/sr_unix/rts_error.c:85
 #6  0x00007ff2e636d64c in zlcompile (len=48 '0', addr=0x7ffffc4b0e30 "/extra1/testarea1/nars/test/temp/tmp/tmp/test2.m") at /Distrib/GT.M/V63002/sr_port/zlcompile.c:81
 #7  0x00007ff2e60e6f1c in op_zlink (v=0x7ffffc4b14a0, quals=0x7ffffc4b0cf0) at /Distrib/GT.M/V63002/sr_unix/op_zlink.c:443
 #8  0x00007ff2e5f6a2d7 in job_addr (rtn=0x7ffffc4b1590, label=0x7ffffc4b15a0, offset=0, hdr=0x7ffffc4b1518, labaddr=0x7ffffc4b1510, need_rtnobj_shm_free=0x7ffffc4b14e4) at /Distrib/GT.M/V63002/sr_port/job_addr.c:41
 #9  0x00007ff2e5f40b48 in jobchild_init () at /Distrib/GT.M/V63002/sr_unix/jobchild_init.c:146
 #10 0x00007ff2e5f3835d in gtm_startup (svec=0x7ffffc4b1d30) at /Distrib/GT.M/V63002/sr_unix/gtm_startup.c:252
 #11 0x00007ff2e5f3b2f6 in init_gtm () at /Distrib/GT.M/V63002/sr_unix/init_gtm.c:201
 #12 0x00007ff2e5f072ea in gtm_main (argc=3, argv=0x7ffffc4b4048, envp=0x7ffffc4b4068) at /Distrib/GT.M/V63002/sr_unix/gtm_main.c:162
 #13 0x0000000000400cbe in main (argc=3, argv=0x7ffffc4b4048, envp=0x7ffffc4b4068) at /Distrib/GT.M/V63002/sr_unix/gtm.c:131
```

In this case, run_time was TRUE and caused the assert failure. Turns out this was due to m_xecute() function (invoked by zlcompile()) temporarily setting run_time to FALSE but when a PATNOTFOUND error was encountered, the condition handler compiler_ch() was invoked which did an UNWIND back to zlcompile() incorrectly persisting the global variable changes done by the interim function call of m_xecute().

The fix for this was to reset the run_time and TREF(xecute_literal_parse) global variables just like is being done in mdb_condition_handler().
  • Loading branch information
nars1 committed Nov 15, 2017
1 parent 49f59d9 commit abacef8
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 10 deletions.
9 changes: 9 additions & 0 deletions sr_port/bool_expr.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
* *
* Copyright (c) 2001-2017 Fidelity National Information *
* Services, Inc. and/or its subsidiaries. All rights reserved. *
* *
* Copyright (c) 2017 YottaDB LLC. and/or its subsidiaries. *
* All rights reserved. *
* *
* This source code contains the intellectual property *
* of its copyright holder(s), and is made available *
Expand Down Expand Up @@ -54,6 +57,12 @@ int bool_expr(boolean_t sense, oprtype *addr)
ex_tail(&t2->operand[0]);
else if (OCT_BOOL & oc_tab[t1->opcode].octype)
bx_boollit(t1);
/* It is possible "ex_tail" or "bx_boollit" is invoked above and has a compile-time error. In that case,
* "ins_errtriple" would be invoked which does a "dqdelchain" that could remove "t1" from the "t_orig"
* triple execution chain and so it is no longer safe to do "dqdel" etc. on "t1".
* Hence the RETURN_EXPR_IF_RTS_ERROR check below.
*/
RETURN_EXPR_IF_RTS_ERROR;
for (t1 = x.oprval.tref; OC_NOOP == t1->opcode; t1 = t1->exorder.bl)
;
if ((OC_COBOOL == t1->opcode) && (OC_LIT == (t2 = t1->operand[0].oprval.tref)->opcode)
Expand Down
11 changes: 10 additions & 1 deletion sr_port/bx_boollit.c
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@

/****************************************************************
* *
* Copyright (c) 2001-2017 Fidelity National Information *
* Services, Inc. and/or its subsidiaries. All rights reserved. *
* *
* Copyright (c) 2017 YottaDB LLC. and/or its subsidiaries. *
* All rights reserved. *
* *
* This source code contains the intellectual property *
* of its copyright holder(s), and is made available *
* under a license. If you do not know the terms of *
Expand Down Expand Up @@ -61,6 +63,7 @@ void bx_boollit(triple *t)
ex_tail(p); /* chained arithmetic */
else if (OCT_BOOL & oc_tab[c].octype)
bx_boollit(optrip[j]);
RETURN_IF_RTS_ERROR;
assert(OC_COMVAL != optrip[j]->opcode);
neg = num = 0;
UNARY_TAIL(opr);
Expand Down Expand Up @@ -176,6 +179,12 @@ void bx_boollit(triple *t)
case OC_NPATTERN:
case OC_PATTERN:
tvr = !(*(uint4 *)v[1]->str.addr) ? do_pattern(v[0], v[1]) : do_patfixed(v[0], v[1]);
/* It is possible "do_patfixed" is invoked above and has a compile-time error. In that case,
* "ins_errtriple" would be invoked which does a "dqdelchain" that could remove "t" from
* the "t_orig" triple execution chain and so it is no longer safe to play with "t".
* Hence the RETURN_IF_RTS_ERROR check below.
*/
RETURN_IF_RTS_ERROR;
break;
case OC_NSORTS_AFTER:
case OC_SORTS_AFTER:
Expand Down
19 changes: 10 additions & 9 deletions sr_port/compiler_ch.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
/****************************************************************
* *
* Copyright 2001, 2013 Fidelity Information Services, Inc *
* Copyright 2001, 2013 Fidelity Information Services, Inc *
* *
* Copyright (c) 2017 YottaDB LLC. and/or its subsidiaries. *
* All rights reserved. *
* *
* This source code contains the intellectual property *
* of its copyright holder(s), and is made available *
Expand Down Expand Up @@ -39,31 +42,29 @@ error_def(ERR_OUTOFSPACE);

CONDITION_HANDLER(compiler_ch)
{

START_CH(TRUE);
if (DUMPABLE)
{
NEXTCH;
if (TREF(xecute_literal_parse))
{ /* This implies we had an error while inside "m_xecute" in the middle of a compile.
* Reset whatever global variables we had set temporarily there before doing an UNWIND
* (that transfers control back to a parent caller function).
*/
run_time = TREF(xecute_literal_parse) = FALSE;
}

if (cmd_qlf.qlf & CQ_WARNINGS)
PRN_ERROR;

COMPILE_HASHTAB_CLEANUP;
reinit_externs();
mstr_native_align = save_mstr_native_align;

if (cg_phase == CGP_MACHINE)
drop_object_file();

if (cg_phase > CGP_NOSTATE)
{
if (cg_phase < CGP_RESOLVE)
close_source_file();
if (cg_phase < CGP_FINI && (cmd_qlf.qlf & CQ_LIST || cmd_qlf.qlf & CQ_CROSS_REFERENCE))
{
close_list_file();
}
}
UNWIND(NULL, NULL);
}
6 changes: 6 additions & 0 deletions sr_port/ex_tail.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* Copyright (c) 2001-2017 Fidelity National Information *
* Services, Inc. and/or its subsidiaries. All rights reserved. *
* *
* Copyright (c) 2017 YottaDB LLC. and/or its subsidiaries. *
* All rights reserved. *
* *
* This source code contains the intellectual property *
* of its copyright holder(s), and is made available *
* under a license. If you do not know the terms of *
Expand Down Expand Up @@ -59,7 +62,10 @@ void ex_tail(oprtype *opr)
for (t0 = i->oprval.tref; OCT_UNARY & oc_tab[t0->opcode].octype; t0 = t0->operand[0].oprval.tref)
;
if (OCT_BOOL & oc_tab[t0->opcode].octype)
{
bx_boollit(t0);
RETURN_IF_RTS_ERROR;
}
ex_tail(i); /* chained Boolean or arithmetic */
RETURN_IF_RTS_ERROR;
}
Expand Down
8 changes: 8 additions & 0 deletions sr_port/indirection.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* Copyright (c) 2001-2017 Fidelity National Information *
* Services, Inc. and/or its subsidiaries. All rights reserved. *
* *
* Copyright (c) 2017 YottaDB LLC. and/or its subsidiaries. *
* All rights reserved. *
* *
* This source code contains the intellectual property *
* of its copyright holder(s), and is made available *
* under a license. If you do not know the terms of *
Expand Down Expand Up @@ -54,6 +57,11 @@ int indirection(oprtype *a)
}
coerce(a, OCT_MVAL);
ex_tail(a);
/* Note: A RETURN_IF_RTS_ERROR check is usually present after all "ex_tail" calls. But that is not needed here.
* This is because we do not do any triple chain manipulations like is done in the other callers. And we want
* to proceed with executing the generated code even if it is going to issue an error (OC_RTERROR triple
* inserted by "ins_errtriple").
*/
ENCOUNTERED_SIDE_EFFECT;
DECREMENT_EXPR_DEPTH;
if ((TK_ATSIGN == TREF(window_token)) || ((TK_ATHASH == TREF(window_token)) && concat_athashes))
Expand Down

0 comments on commit abacef8

Please sign in to comment.