From: Shamus Hammons Date: Sat, 9 Nov 2013 15:01:57 +0000 (-0600) Subject: Fixed subtle bug in expr(). X-Git-Tag: 1.3.0~7 X-Git-Url: http://shamusworld.gotdns.org/cgi-bin/gitweb.cgi?p=rmac;a=commitdiff_plain;h=75969398d9b8a9f82ea76fc4e4cbfb97b11160a4;ds=sidebyside Fixed subtle bug in expr(). Basically, expr() was looking at the token following the one it was looking at and bypassing the longer parse path if it found an EOL token there. Problem is, some tokens have follow on values and so can be considered compound tokens. In this case, the EOL token codes to 101, having a constant with a value of 101 will not evaluate correctly in this case as the CONST token is a compound token. The short of this is that making assumptions is BAD! Don't do it! It WILL come around to bite you in the ass eventually, in the form of subtle bugs that are difficult to chase down. Assume nothing! --- diff --git a/direct.c b/direct.c index 31852c8..7673d29 100644 --- a/direct.c +++ b/direct.c @@ -227,7 +227,8 @@ int d_ccundef(void) if (*tok != SYMBOL) { - error(syntax_error); +// error(syntax_error); + error("syntax error; expected symbol"); return ERROR; } @@ -270,7 +271,8 @@ int d_equrundef(void) // Check we are dealing with a symbol if (*tok != SYMBOL) { - error(syntax_error); +// error(syntax_error); + error("syntax error; expected symbol"); return ERROR; } @@ -317,7 +319,8 @@ int d_incbin(void) if (*tok != STRING) { - error(syntax_error); +// error(syntax_error); + error("syntax error; string missing"); return ERROR; } diff --git a/error.c b/error.c index 300a07e..4f65cc4 100644 --- a/error.c +++ b/error.c @@ -22,8 +22,14 @@ static long unused; // For supressing 'write' warnings // int at_eol(void) { + char msg[256]; + if (*tok != EOL) - error("syntax error"); + { +// error("syntax error"); + sprintf(msg, "syntax error. expected EOL, found $%X ('%c')", *tok, *tok); + error(msg); + } return 0; } diff --git a/expr.c b/expr.c index 6f0b619..f582408 100644 --- a/expr.c +++ b/expr.c @@ -241,13 +241,9 @@ int expr2(void) } *evalTokenBuffer++ = SYMBOL; -#if 0 - *evalTokenBuffer++ = (TOKEN)sy; -#else *evalTokenBuffer++ = symbolNum; symbolPtr[symbolNum] = sy; symbolNum++; -#endif break; case STRING: *evalTokenBuffer++ = CONST; @@ -277,10 +273,15 @@ int expr2(void) case '*': *evalTokenBuffer++ = ACONST; // Attributed const +#if 0 if (orgactive) *evalTokenBuffer++ = orgaddr; else *evalTokenBuffer++ = pcloc; // Location at start of line +#else + // pcloc == location at start of line + *evalTokenBuffer++ = (orgactive ? orgaddr : pcloc); +#endif *evalTokenBuffer++ = ABS | DEFINED; // Store attribs break; @@ -308,8 +309,14 @@ int expr(TOKEN * otk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) evalTokenBuffer = otk; // Set token pointer to 'exprbuf' (direct.c) // Also set in various other places too (riscasm.c, e.g.) +//printf("expr(): tokens 0-2: %i %i %i (%c %c %c); tc[2] = %i\n", tok[0], tok[1], tok[2], tok[0], tok[1], tok[2], tokenClass[tok[2]]); // Optimize for single constant or single symbol. - if ((tok[1] == EOL) + // Shamus: Subtle bug here. EOL token is 101; if you have a constant token + // followed by the value 101, it will trigger a bad evaluation here. + // This is probably a really bad assumption to be making here...! + // (assuming tok[1] == EOL is a single token that is) +// if ((tok[1] == EOL) + if ((tok[1] == EOL && tok[0] != CONST) || (((*tok == CONST || *tok == SYMBOL) || (*tok >= KW_R0 && *tok <= KW_R31)) && (tokenClass[tok[2]] < UNARY))) { @@ -323,8 +330,6 @@ int expr(TOKEN * otk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) *a_esym = NULL; tok++; -// *evalTokenBuffer++ = ENDEXPR; -// return OK; } else if (*tok == CONST) { @@ -336,6 +341,7 @@ int expr(TOKEN * otk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) *a_esym = NULL; tok += 2; +//printf("Quick eval in expr(): CONST = %i, tokenClass[tok[2]] = %i\n", *a_value, tokenClass[*tok]); } else if (*tok == '*') { @@ -351,7 +357,6 @@ int expr(TOKEN * otk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) if (a_esym != NULL) *a_esym = NULL; -// tok--; tok++; } else if (*tok == STRING || *tok == SYMBOL) @@ -425,7 +430,6 @@ thrown away right here. What the hell is it for? return ERROR; } -// tok += 2; *evalTokenBuffer++ = ENDEXPR; return OK; } @@ -463,7 +467,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) switch ((int)*tk++) { case SYMBOL: -// sy = (SYM *)*tk++; +//printf("evexpr(): SYMBOL\n"); sy = symbolPtr[*tk++]; sy->sattr |= REFERENCED; // Set "referenced" bit @@ -496,10 +500,12 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) sym_seg = (WORD)(sy->sattr & (TEXT | DATA | BSS)); break; case CONST: +//printf("evexpr(): CONST = %i\n", *tk); *++sval = *tk++; // Push value *++sattr = ABS | DEFINED; // Push simple attribs break; case ACONST: +//printf("evexpr(): ACONST = %i\n", *tk); *++sval = *tk++; // Push value *++sattr = (WORD)*tk++; // Push attribs break; @@ -516,6 +522,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) // [1] + : Error // - : ABS case '+': +//printf("evexpr(): +\n"); --sval; // Pop value --sattr; // Pop attrib *sval += sval[1]; // Compute value @@ -527,6 +534,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) break; case '-': +//printf("evexpr(): -\n"); --sval; // Pop value --sattr; // Pop attrib *sval -= sval[1]; // Compute value @@ -546,6 +554,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) break; // Unary operators only work on ABS items case UNMINUS: +//printf("evexpr(): UNMINUS\n"); if (*sattr & (TEXT | DATA | BSS)) error(seg_error); @@ -553,6 +562,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) *sattr = ABS | DEFINED; // Expr becomes absolute break; case '!': +//printf("evexpr(): !\n"); if (*sattr & (TEXT | DATA | BSS)) error(seg_error); @@ -560,6 +570,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) *sattr = ABS | DEFINED; // Expr becomes absolute break; case '~': +//printf("evexpr(): ~\n"); if (*sattr & (TEXT | DATA | BSS)) error(seg_error); @@ -569,6 +580,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) // Comparison operators must have two values that // are in the same segment, but that's the only requirement. case LE: +//printf("evexpr(): LE\n"); --sattr; --sval; @@ -579,6 +591,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) *sval = *sval <= sval[1]; break; case GE: +//printf("evexpr(): GE\n"); --sattr; --sval; @@ -589,6 +602,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) *sval = *sval >= sval[1]; break; case '>': +//printf("evexpr(): >\n"); --sattr; --sval; @@ -599,6 +613,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) *sval = *sval > sval[1]; break; case '<': +//printf("evexpr(): <\n"); --sattr; --sval; @@ -609,6 +624,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) *sval = *sval < sval[1]; break; case NE: +//printf("evexpr(): NE\n"); --sattr; --sval; @@ -619,6 +635,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) *sval = *sval != sval[1]; break; case '=': +//printf("evexpr(): =\n"); --sattr; --sval; @@ -631,6 +648,7 @@ int evexpr(TOKEN * tk, VALUE * a_value, WORD * a_attr, SYM ** a_esym) // All other binary operators must have two ABS items // to work with. They all produce an ABS value. default: +//printf("evexpr(): default\n"); // GH - Removed for v1.0.15 as part of the fix for indexed loads. //if ((*sattr & (TEXT|DATA|BSS)) || (*--sattr & (TEXT|DATA|BSS))) //error(seg_error); diff --git a/mach.c b/mach.c index e0640e5..550db61 100644 --- a/mach.c +++ b/mach.c @@ -545,7 +545,10 @@ int m_br(WORD inst, WORD siz) if (a0exattr & DEFINED) { if ((a0exattr & TDB) != cursect) +//{ +//printf("m_br(): a0exattr = %X, cursect = %X, a0exval = %X, sloc = %X\n", a0exattr, cursect, a0exval, sloc); return error(rel_error); +//} v = a0exval - (sloc + 2); diff --git a/parmode.h b/parmode.h index 11b16bf..5ef768a 100644 --- a/parmode.h +++ b/parmode.h @@ -246,7 +246,7 @@ goto AnOK; } - ++tok; + tok++; if ((*tok >= KW_A0) && (*tok <= KW_A7)) { diff --git a/procln.c b/procln.c index 9987aad..5ae5c5e 100644 --- a/procln.c +++ b/procln.c @@ -171,7 +171,8 @@ loop1: // Internal line processing loop // First token MUST be a symbol if (*tok != SYMBOL) { - error(syntax_error); +// error(syntax_error); + error("syntax error; expected symbol"); goto loop; } @@ -215,7 +216,8 @@ as68label: // Next token MUST be a symbol if (*tok++ != SYMBOL) { - error(syntax_error); +// error(syntax_error); + error("syntax error; expected symbol"); goto loop; } diff --git a/token.c b/token.c index 4a06efe..e35e9bf 100644 --- a/token.c +++ b/token.c @@ -452,7 +452,8 @@ arg_num: continue; } - if (tk != NULL) // arg # is in range, so expand it + // Argument # is in range, so expand it + if (tk != NULL) { while (*tk != EOL) { @@ -1438,6 +1439,7 @@ if (verb_flag) printf("TokenizeLine: Calling fpop() from SRC_IREPT...\n"); *tk++ = CONST; *tk++ = v; +//printf("CONST: %i\n", v); continue; } diff --git a/version.h b/version.h index 52172c0..b071eb7 100644 --- a/version.h +++ b/version.h @@ -13,6 +13,6 @@ #define MAJOR 1 // Major version number #define MINOR 2 // Minor version number -#define PATCH 7 // Patch release number +#define PATCH 8 // Patch release number #endif // __VERSION_H__