Fixed subtle bug in expr().
authorShamus Hammons <jlhamm@acm.org>
Sat, 9 Nov 2013 15:01:57 +0000 (09:01 -0600)
committerShamus Hammons <jlhamm@acm.org>
Sat, 9 Nov 2013 15:01:57 +0000 (09:01 -0600)
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!

direct.c
error.c
expr.c
mach.c
parmode.h
procln.c
token.c
version.h

index 31852c8e2f9213c3a3a0e3735406aa9c57b95fab..7673d29c18eb0089d848148b427b0ef5fd1992f0 100644 (file)
--- 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 300a07e9daabcb3851381ea568e0fe5fa91701ba..4f65cc42c50abc9601a051d688b508d51a99553e 100644 (file)
--- 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 6f0b619492824e65031011f6c9317e7e8cfcf5f8..f582408c5773be5f90d77c13220ebfefee679a58 100644 (file)
--- 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 e0640e554a6bdcbd2a1bf2772535fef15e8bf5e7..550db6129b6008d25d2cc1e06fb045e5e5762f70 100644 (file)
--- 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);
 
index 11b16bfbe81894d962029d18cf88d40a65930f77..5ef768a44175dbde77b08ffbc58cc078798877b8 100644 (file)
--- a/parmode.h
+++ b/parmode.h
                        goto AnOK;
                }
 
-               ++tok;
+               tok++;
 
                if ((*tok >= KW_A0) && (*tok <= KW_A7))
                {
index 9987aad70ef4c836d81a1fb08ea4fc8f95aae579..5ae5c5e080e501f7f0801220e6407306ef65ae83 100644 (file)
--- 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 4a06efedcbebf1a0fdb72aec48857e74d6286b5a..e35e9bfc787697ea21e767f4a94397fb7ba223bb 100644 (file)
--- 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;
                }
 
index 52172c069ff137cd6b7558cd0463134e6f857686..b071eb7d4a11fcb3c767036d8ea4279ed1ce2c13 100644 (file)
--- 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__