From d5405c4db138e814b6ca56c33b741210c544cd8e Mon Sep 17 00:00:00 2001
From: Steven Hugg <hugg@fasterlight.com>
Date: Mon, 10 Aug 2020 11:08:43 -0500
Subject: [PATCH] basic: input validation, array bounds check

---
 css/ui.css                   | 10 ++++-
 index.html                   |  2 +-
 presets/basic/sieve.bas      |  9 ++--
 src/common/basic/compiler.ts | 17 ++++---
 src/common/basic/run.ts      | 22 +++++++--
 src/common/basic/runtime.ts  | 86 ++++++++++++++++++++++++++----------
 src/common/teletype.ts       |  9 +++-
 src/platform/basic.ts        |  7 +--
 8 files changed, 116 insertions(+), 46 deletions(-)

diff --git a/css/ui.css b/css/ui.css
index 30d52329..4bf0d917 100644
--- a/css/ui.css
+++ b/css/ui.css
@@ -600,17 +600,23 @@ div.asset_toolbar {
   flex-shrink: 0;
 }
 .transcript-line {
-  line-height: 1.5;
+  line-height: 1.5em;
   min-height: 1em;
   white-space: pre-wrap;
 }
+.transcript-fanfold > .transcript-line {
+  min-height: 1.5em;
+}
+.transcript-fanfold > .transcript-line:nth-child(even) {
+  background: RGBA(225,245,225,1.0);
+}
 .transcript-style-1 {
   font-style: italic;
 }
 .transcript-style-2 {
   white-space: pre-wrap;       /* css-3 */
   font-family: "Andale Mono", "Menlo", "Lucida Console", monospace;
-  line-height: 1;
+  line-height: 1em;
   min-height: 0;
 }
 .transcript-style-4 {
diff --git a/index.html b/index.html
index 302be48e..5b4af060 100644
--- a/index.html
+++ b/index.html
@@ -214,7 +214,7 @@ if (window.location.host.endsWith('8bitworkshop.com')) {
              </ul>
           </li>
          <li class="dropdown dropdown-submenu">
-          <a tabindex="-1" href="#">Text-Based</a>
+          <a tabindex="-1" href="#">Interpreters</a>
           <ul class="dropdown-menu">
             <li><a class="dropdown-item" href="?platform=basic">BASIC</a></li>
             <li><a class="dropdown-item" href="?platform=zmachine">Z-Machine</a></li>
diff --git a/presets/basic/sieve.bas b/presets/basic/sieve.bas
index a4ed4c7b..b28e6e64 100644
--- a/presets/basic/sieve.bas
+++ b/presets/basic/sieve.bas
@@ -2,13 +2,14 @@
 010 REM***A PRIME NUMBER SIEVE BENCHMARK
 020 T = TIMER
 025 N = 8191
-030 DIM F(N+3)
-040 FOR I = 0 TO N : F(I) = 1 : NEXT I
+030 DIM F(N+1)
+040 FOR I = 0 TO N : F(I)=1 : NEXT I
 050 S = N
 060 FOR I = 0 TO S
-070   IF F(I) = 0 THEN GOTO 110
+070   IF F(I)=0 THEN GOTO 110
 080   P = I+I+3
-090   FOR K = I+P TO S STEP P : F(K) = 0 : NEXT K
+085   IF I+P>S THEN GOTO 110
+090   FOR K=I+P TO S STEP P : F(K)=0 : NEXT K
 100   C = C+1
 110 NEXT I
 120 T = TIMER-T
diff --git a/src/common/basic/compiler.ts b/src/common/basic/compiler.ts
index 6f90beb3..129eb5ca 100644
--- a/src/common/basic/compiler.ts
+++ b/src/common/basic/compiler.ts
@@ -288,7 +288,7 @@ export class BASICParser {
         throw new CompileError(`${msg} (line ${loc.line})`); // TODO: label too?
     }
     dialectError(what: string, loc?: SourceLocation) {
-        this.compileError(`The selected BASIC dialect doesn't support ${what}.`, loc); // TODO
+        this.compileError(`The selected BASIC dialect (${this.opts.dialectName}) doesn't support ${what}.`, loc); // TODO
     }
     consumeToken(): Token {
         var tok = this.lasttoken = (this.tokens.shift() || this.eol);
@@ -624,7 +624,9 @@ export class BASICParser {
     stmt__IF(): IF_Statement {
         var cond = this.parseExpr();
         var iftrue: Statement[];
-        this.expectToken('THEN');
+        // we accept GOTO or THEN if line number provided
+        if (this.peekToken().str == 'GOTO') this.consumeToken();
+        else this.expectToken('THEN');
         var lineno = this.peekToken();
         // assume GOTO if number given after THEN
         if (lineno.type == TokenType.Int) {
@@ -890,6 +892,7 @@ export const APPLESOFT_BASIC : BASICOptions = {
     sparseArrays : false,
     tickComments : false,
     validKeywords : [
+        'OPTION',
         'CLEAR','LET','DIM','DEF','FN','GOTO','GOSUB','RETURN','ON','POP',
         'FOR','TO','NEXT','IF','THEN','END','STOP','ONERR','RESUME',
         'PRINT','INPUT','GET','HOME','HTAB','VTAB',
@@ -912,12 +915,12 @@ export const APPLESOFT_BASIC : BASICOptions = {
     bitwiseLogic : false,
 }
 
-export const MAX8_BASIC : BASICOptions = {
-    dialectName: "MAX8",
+export const MODERN_BASIC : BASICOptions = {
+    dialectName: "MODERN",
     asciiOnly : false,
     uppercaseOnly : false,
     optionalLabels : true,
-    strictVarNames : false, // TODO: first two alphanum chars
+    strictVarNames : false,
     sharedArrayNamespace : false,
     defaultArrayBase : 0,
     defaultArraySize : 11,
@@ -925,7 +928,7 @@ export const MAX8_BASIC : BASICOptions = {
     stringConcat : true,
     typeConvert : true,
     maxDimensions : 255,
-    maxDefArgs : 255, // TODO: no string FNs
+    maxDefArgs : 255,
     maxStringLength : 1024, // TODO?
     sparseArrays : false,
     tickComments : true,
@@ -942,6 +945,7 @@ export const MAX8_BASIC : BASICOptions = {
 }
 
 // TODO: integer vars
+// TODO: short-circuit FOR loop
 
 export const DIALECTS = {
     "DEFAULT":      ALTAIR_BASIC40,
@@ -950,4 +954,5 @@ export const DIALECTS = {
     "ECMA55":       ECMA55_MINIMAL,
     "MINIMAL":      ECMA55_MINIMAL,
     "APPLESOFT":    APPLESOFT_BASIC,
+    "MODERN":       MODERN_BASIC,
 };
diff --git a/src/common/basic/run.ts b/src/common/basic/run.ts
index 042f4893..235eabfa 100644
--- a/src/common/basic/run.ts
+++ b/src/common/basic/run.ts
@@ -6,8 +6,15 @@ var readline = require('readline');
 var rl = readline.createInterface({
     input: process.stdin,
     output: process.stdout,
-    //terminal: false
+    terminal: false,
+    crlfDelay: Infinity,
 });
+var inputlines = [];
+rl.on('line', (line) => {
+    //console.log(`Line from file: ${line}`);
+    inputlines.push(line);
+});
+
 var fs = require('fs');
 
 var parser = new BASICParser();
@@ -46,10 +53,16 @@ runtime.print = (s:string) => {
 }
 runtime.input = async (prompt:string) => {
     return new Promise( (resolve, reject) => {
-        fs.writeSync(1, "\n");
-        rl.question(prompt, (answer) => {
+        function answered(answer) {
             var vals = answer.toUpperCase().split(',');
+            console.log(">>>",vals);
             resolve(vals);
+        }
+        fs.writeSync(1, prompt+"?");
+        if (inputlines.length) {
+            answered(inputlines.shift());
+        } else rl.question(prompt, (answer) => {
+            answered(answer);
         });
     });
 }
@@ -59,7 +72,8 @@ runtime.resume = function() {
             if (runtime.step()) {
                 if (runtime.running) runtime.resume();
             } else if (runtime.exited) {
-                rl.close();
+                console.log("*** PROGRAM EXITED ***");
+                process.exit(0);
             }
         } catch (e) {
             console.log(`### ${e.message} (line ${runtime.getCurrentSourceLocation().label})`);
diff --git a/src/common/basic/runtime.ts b/src/common/basic/runtime.ts
index 883d49f6..c2e7cae7 100644
--- a/src/common/basic/runtime.ts
+++ b/src/common/basic/runtime.ts
@@ -16,15 +16,21 @@ function isUnOp(arg: basic.Expr): arg is basic.UnOp {
     return (arg as any).op != null && (arg as any).expr != null;
 }
 
+// expr2js() options
 class ExprOptions {
-    isconst?: boolean;
-    locals?: string[];
+    isconst?: boolean;      // only allow constant operations
+    novalid?: boolean;      // check for valid values when fetching
+    locals?: string[];      // pass local variable names when defining functions
 }
 
 interface CompiledStatement {
     $run?: () => void;
 }
 
+function isArray(obj) {
+    return obj != null && (Array.isArray(obj) || obj.BYTES_PER_ELEMENT);
+}
+
 export class BASICRuntime {
 
     program : basic.BASICProgram;
@@ -158,7 +164,6 @@ export class BASICRuntime {
         // skip to next statment
         this.curpc++;
         // compile (unless cached) and execute statement
-        this.compileStatement(stmt);
         this.executeStatement(stmt);
         return this.running;
     }
@@ -178,6 +183,8 @@ export class BASICRuntime {
         }
     }
     executeStatement(stmt: basic.Statement & CompiledStatement) {
+        // compile (unless cached)
+        this.compileStatement(stmt);
         // run compiled statement
         stmt.$run();
     }
@@ -215,6 +222,8 @@ export class BASICRuntime {
     }
 
     gosubLabel(label) {
+        if (this.returnStack.length > 65535)
+            this.runtimeError(`I did too many GOSUBs without a RETURN.`)
         this.returnStack.push(this.curpc);
         this.gotoLabel(label);
     }
@@ -291,9 +300,22 @@ export class BASICRuntime {
             if (!expr.args && opts.locals && opts.locals.indexOf(expr.name) >= 0) {
                 return expr.name; // local arg in DEF
             } else {
-                if (opts.isconst) this.runtimeError(`I expected a constant value here.`);
-                var s = this.assign2js(expr, opts);
-                return `this.checkValue(${s}, ${JSON.stringify(expr.name)})`;
+                var s = '';
+                var qname = JSON.stringify(expr.name);
+                let jsargs = expr.args && expr.args.map((arg) => this.expr2js(arg, opts)).join(', ');
+                if (expr.name.startsWith("FN")) { // is it a user-defined function?
+                    // TODO: check argument count?
+                    s += `this.getDef(${qname})(${jsargs})`;
+                    // TODO: detect recursion?
+                } else if (this.builtins[expr.name]) { // is it a built-in function?
+                    this.checkFuncArgs(expr, this.builtins[expr.name]);
+                    s += `this.builtins.${expr.name}(${jsargs})`;
+                } else if (expr.args) {
+                    s += `this.arrayGet(${qname}, ${jsargs})`;
+                } else { // just a variable
+                    s += `this.vars.${expr.name}`;
+                }
+                return opts.novalid ? s : `this.checkValue(${s}, ${qname})`;
             }
         } else if (isBinOp(expr)) {
             var left = this.expr2js(expr.left, opts);
@@ -309,18 +331,12 @@ export class BASICRuntime {
         if (!opts) opts = {};
         var s = '';
         var qname = JSON.stringify(expr.name);
-        if (expr.name.startsWith("FN")) { // is it a user-defined function?
-            // TODO: check argument count?
-            let jsargs = expr.args && expr.args.map((arg) => this.expr2js(arg, opts)).join(', ');
-            s += `this.getDef(${qname})(${jsargs})`;
-            // TODO: detect recursion?
-        } else if (this.builtins[expr.name]) { // is it a built-in function?
-            this.checkFuncArgs(expr, this.builtins[expr.name]);
-            let jsargs = expr.args && expr.args.map((arg) => this.expr2js(arg, opts)).join(', ');
-            s += `this.builtins.${expr.name}(${jsargs})`;
-        } else if (expr.args) { // is it a subscript?
-            // TODO: check array bounds?
-            s += `this.getArray(${qname}, ${expr.args.length})`;
+         // is it a function? not allowed
+        if (expr.name.startsWith("FN") || this.builtins[expr.name]) this.runtimeError(`I can't call a function here.`);
+        // is it a subscript?
+        if (expr.args) {
+            s += this.expr2js(expr, {novalid:true}); // check array bounds
+            s += `;this.getArray(${qname}, ${expr.args.length})`;
             s += expr.args.map((arg) => '[this.ROUND('+this.expr2js(arg, opts)+')]').join('');
         } else { // just a variable
             s = `this.vars.${expr.name}`;
@@ -388,7 +404,7 @@ export class BASICRuntime {
 
     convert(name: string, right: number|string) : number|string {
         if (name.endsWith("$")) {
-            return right+"";
+            return right == null ? "" : right.toString();
         } else if (typeof right === 'number') {
             return right;
         } else {
@@ -418,8 +434,9 @@ export class BASICRuntime {
             this.arrays[name] = new arrcons(dims[0]+1);
         } else if (dims.length == 2) {
             this.arrays[name] = new Array(dims[0]+1);
-            for (var i=0; i<dims[0]+1; i++)
+            for (var i=0; i<dims[0]+1; i++) {
                 this.arrays[name][i] = new arrcons(dims[1]+1);
+            }
         } else {
             this.runtimeError(`I only support arrays of one or two dimensions.`)
         }
@@ -437,6 +454,25 @@ export class BASICRuntime {
         return this.arrays[name];
     }
 
+    arrayGet(name: string, ...indices: number[]) : basic.Value {
+        var arr = this.getArray(name, indices.length);
+        indices = indices.map(Math.round);
+        var v = arr;
+        for (var i=0; i<indices.length; i++) {
+            var idx = indices[i];
+            if (!isArray(v))
+                this.runtimeError(`I tried to lookup ${name}(${indices}) but used too many dimensions.`);
+            if (idx < this.opts.defaultArrayBase)
+                this.runtimeError(`I tried to lookup ${name}(${indices}) but an index was less than ${this.opts.defaultArrayBase}.`);
+            if (idx >= v.length)
+                this.runtimeError(`I tried to lookup ${name}(${indices}) but it exceeded the dimensions of the array.`);
+            v = v[indices[i]];
+        }
+        if (isArray(v)) // i.e. is an array?
+            this.runtimeError(`I tried to lookup ${name}(${indices}) but used too few dimensions.`);
+        return (v as any) as basic.Value;
+    }
+
     onGotoLabel(value: number, ...labels: string[]) {
         value = this.ROUND(value);
         if (value < 1 || value > labels.length)
@@ -473,7 +509,11 @@ export class BASICRuntime {
         var setvals = '';
         stmt.args.forEach((arg, index) => {
             var lexpr = this.assign2js(arg);
-            setvals += `valid &= this.isValid(${lexpr} = this.convert(${JSON.stringify(arg.name)}, vals[${index}]));`
+            setvals += `
+            var value = this.convert(${JSON.stringify(arg.name)}, vals[${index}]);
+            valid &= this.isValid(value);
+            ${lexpr} = value;
+            `
         });
         return `this.running=false;
                 this.input(${prompt}, ${stmt.args.length}).then((vals) => {
@@ -619,12 +659,12 @@ export class BASICRuntime {
     // TODO: memory quota
     // TODO: useless loop (! 4th edition)
     // TODO: other 4th edition errors
-
+    // TODO: ecma55 all-or-none input checking?
 
     // FUNCTIONS
 
     isValid(obj:number|string) : boolean {
-        if (typeof obj === 'number' && !isNaN(obj))
+        if (typeof obj === 'number' && !isNaN(obj) && isFinite(obj))
             return true;
         else if (typeof obj === 'string')
             return true;
diff --git a/src/common/teletype.ts b/src/common/teletype.ts
index eda435f5..0512b305 100644
--- a/src/common/teletype.ts
+++ b/src/common/teletype.ts
@@ -138,11 +138,18 @@ export class TeleType {
         if (show) ph.show(); else ph.hide();
         */
     }
+    resize(columns: number) {
+        // set font size proportional to window width
+        var charwidth = $(this.page).width() * 1.6 / columns;
+        $(this.page).css('font-size', charwidth + 'px');
+        this.scrollToBottom();
+    }
 }
 
 export class TeleTypeWithKeyboard extends TeleType {
     input : HTMLInputElement;
     keepinput : boolean = true;
+    msecPerLine : number = 100; // IBM 1403
 
     focused : boolean = true;
     scrolling : number = 0;
@@ -236,7 +243,7 @@ export class TeleTypeWithKeyboard extends TeleType {
         if (this.scrolldiv) {
             this.scrolling++;
             var top = $(this.page).height() + $(this.input).height();
-            $(this.scrolldiv).stop().animate({scrollTop: top}, 200, 'swing', () => {
+            $(this.scrolldiv).stop().animate({scrollTop: top}, this.msecPerLine, 'swing', () => {
                 this.scrolling = 0;
                 this.ncharsout = 0;
             });
diff --git a/src/platform/basic.ts b/src/platform/basic.ts
index 20363b2c..b9c5955f 100644
--- a/src/platform/basic.ts
+++ b/src/platform/basic.ts
@@ -63,10 +63,7 @@ class BASICPlatform implements Platform {
         }
         this.timer = new AnimationTimer(60, this.animate.bind(this));
         this.resize = () => {
-            // set font size proportional to window width
-            var charwidth = $(gameport).width() * 1.6 / 80;
-            $(windowport).css('font-size', charwidth + 'px');
-            this.tty.scrollToBottom();
+            this.tty.resize(80);
         }
         this.resize();
         this.runtime.print = (s:string) => {
@@ -174,12 +171,12 @@ class BASICPlatform implements Platform {
     }
     getDebugTree() {
         return {
+            CurrentLine: this.runtime.getCurrentLabel(),
             Variables: this.runtime.vars,
             Arrays: this.runtime.arrays,
             Functions: this.runtime.defs,
             ForLoops: this.runtime.forLoops,
             ReturnStack: this.runtime.returnStack,
-            CurrentLine: this.runtime.getCurrentLabel(),
             NextDatum: this.runtime.datums[this.runtime.dataptr],
             Dialect: this.runtime.opts,
             Internals: this.runtime,