OS (Memory.vm) bug

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

OS (Memory.vm) bug

cadet1620
Administrator
I've found an interesting bug while writing my Memory.jack.

As I was writing Memory.alloc I realized that the compiled code for the constructor of a class with no fields in it would call Memory.alloc(0) which would result in a block that did not have room for the free list link. I wrote my code to allocate a minimum of 2 words to allow for the required size and link fields.

This is my test code for this case, which works with my Memory.jack:

class Main {
    function void main() {
        var Zero z;
        let z = Zero.new();
        do z.dispose();
        do Output.printString("Hello world!");
        return;
    }
}

class Zero {
    constructor Zero new() {
        return this;
    }
    method void dispose() {
        do Memory.deAlloc(this);
        return;
    }
}
Compiled with JackCompiler and the reference OS .vm files, this program does not print "Hello world!"; it prints "ERR5", which is "Allocated memory size must be positive". The problem is with the generated code for Zero.new:
function Zero.new 0
push constant 0
call Memory.alloc 1
pop pointer 0
push pointer 0
return

I've been thinking about this a bit, and it seems to me that requiring Memory.alloc to handle 0 length requests is the cleanest way to handle this. Other choices might be:

  • require all classes with constructors to contain at least one field,
  • require the compiler to emit code that allocates at least one word for all objects,
The first option is the ugliest; it requires changing the Jack grammar and adds complexity to the parser.

The second option is easy since the object's storage requirement is known at compile time. I plan to make this change to my compiler so that I don't generate code that crashes using the reference OS .vm files.

Long term, I think it makes the most sense to keep this detail out of the compiler and handle it in the OS where the oddments traditionally reside. (Also, there is nowhere in the "Jack OS Specification" that says that Memory.alloc(0) is illegal.)

--Mark

 

[Thank you Noam and Shimon for an amazingly well done job. I stumbled across TECS looking for material for a student of mine, started evaluating it, got hooked, bought the book, and have thoroughly enjoyed working through it.]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OS (Memory.vm) bug

sgaweda
Reading this helped solve a problem I was having running my project 9 Jack program in the vme. As always, thank you Mark! Answering questions I haven't even asked yet.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: OS (Memory.vm) bug

cadet1620
Administrator
Here's a new version of Memory.vm that fixes Memory.alloc() to allow allocation of 0 sized blocks.

For those interested, the source code change is:
    function int alloc(int size) {
        var Array block;

--      if (size < 1) {
--          do Sys.error(5);
++      if (size < 0) {         // JackCompiler writes code that calls alloc(0)
++          do Sys.error(5);    // for classes with no fields.
        }
++      if (size = 0) {         // For 0 length blocks, set the size to 1 so
++          let size = 1;       // that there will be space for the free list
++      }                       // pointer.

        [remainder of alloc]

--Mark
Loading...