unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: Noah Lavine <noah.b.lavine@gmail.com>
To: guile-devel <guile-devel@gnu.org>
Subject: Patches for wip-rtl
Date: Sat, 20 Apr 2013 19:30:55 -0400	[thread overview]
Message-ID: <CA+U71=P1RuyZ=LqB73FQzhAan=m8z7dg+7bESYE+zNBW3z3Z5w@mail.gmail.com> (raw)


[-- Attachment #1.1: Type: text/plain, Size: 1664 bytes --]

Hello,

I've attached three patches for wip-rtl. The first is somewhat different
than the other two: it fixes an error that occurred when moving the linker
to its own file. (system vm rtl) and (system vm linker) both contain a
function called link-string-table, and (system vm rtl) was calling the
wrong one, which made rtl.test fail. I solved this with an @-reference.

The second two provide better VM errors when box-ref and box-set! are
called on something that is not a variable. In particular, they throw an
exception instead of aborting. This could occur in user code, if the user
wants to hand-write RTL code, but it's also very useful in trying a new
compiler implementation. The first patch handles box-ref, and the second
handles box-set!. Also note that I had to add a new type of exception to
(test-suite lib) to catch these errors.

Lastly, I'd like to ask for ideas on how to test for errors in VM
instructions that don't cause immediate problems, but do put the VM in an
inconsistent state. I know of a collection of these errors, and while I
could fix them, I'd rather fix them and add a test for them. I can think of
two possibilities off the top of my head: 1) add Scheme accessors for VM
state (I don't think mutators are necessary for now) or 2) write the tests
in C instead of Scheme.

I'd prefer option 1, mostly because I think that making the VM state more
explicit is a good direction to go in anyway - eventually it would be great
if the debugger could print the contents of registers, and that would
require VM introspection anyway, so I think starting now is good.

What do other people think of the patches and the ideas?

Best,
Noah

[-- Attachment #1.2: Type: text/html, Size: 1828 bytes --]

[-- Attachment #2: 0001-Bugfix-in-rtl.scm.patch --]
[-- Type: application/octet-stream, Size: 1012 bytes --]

From eddecdb057724016379b69c5e377d6ff13450594 Mon Sep 17 00:00:00 2001
From: Noah Lavine <noah.b.lavine@gmail.com>
Date: Sat, 20 Apr 2013 18:27:48 -0400
Subject: [PATCH 1/3] Bugfix in rtl.scm

* module/system/vm/rtl.scm: fix test errors stemming from two functions
  with the same name.
---
 module/system/vm/rtl.scm |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/module/system/vm/rtl.scm b/module/system/vm/rtl.scm
index 6126e0d..77039aa 100644
--- a/module/system/vm/rtl.scm
+++ b/module/system/vm/rtl.scm
@@ -777,7 +777,10 @@
 (define (link-string-table asm)
   (intern-string! asm ".shstrtab")
   (make-object asm '.shstrtab
-               (link-string-table (asm-string-table asm))
+               ;; delegate to a function with the same name but a
+               ;; different implementation
+               ((@ (system vm linker) link-string-table)
+                (asm-string-table asm))
                '() '()
                #:type SHT_STRTAB #:flags 0))
 
-- 
1.7.10.4


[-- Attachment #3: 0002-Better-Exception-in-RTL-VM.patch --]
[-- Type: application/octet-stream, Size: 2579 bytes --]

From ce441123bbd3c32a0b1a14618b7cdc9405df730e Mon Sep 17 00:00:00 2001
From: Noah Lavine <noah.b.lavine@gmail.com>
Date: Sat, 20 Apr 2013 18:57:01 -0400
Subject: [PATCH 2/3] Better Exception in RTL VM

 * libguile/vm-engine.c: in box-ref, throw an exception instead of
   aborting when the register does not contain a variable.
 * test-suite/tests/rtl.test: test this behavior.
 * test-suite/test-suite/lib.scm: add a new exception to enable this
   test.
---
 libguile/vm-engine.c          |    3 ++-
 test-suite/test-suite/lib.scm |    3 +++
 test-suite/tests/rtl.test     |    9 +++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/libguile/vm-engine.c b/libguile/vm-engine.c
index 44687e2..f5f1617 100644
--- a/libguile/vm-engine.c
+++ b/libguile/vm-engine.c
@@ -1865,7 +1865,8 @@ RTL_VM_NAME (SCM vm, SCM program, SCM *argv, size_t nargs_)
       SCM var;
       SCM_UNPACK_RTL_12_12 (op, dst, src);
       var = LOCAL_REF (src);
-      VM_ASSERT (SCM_VARIABLEP (var), abort ());
+      VM_ASSERT (SCM_VARIABLEP (var),
+		 vm_error_not_a_variable ("box-ref (VM instruction)", var));
       if (SCM_UNLIKELY (!VARIABLE_BOUNDP (var)))
         {
           SCM var_name;
diff --git a/test-suite/test-suite/lib.scm b/test-suite/test-suite/lib.scm
index 7517b4e..6b289ca 100644
--- a/test-suite/test-suite/lib.scm
+++ b/test-suite/test-suite/lib.scm
@@ -31,6 +31,7 @@
  exception:out-of-range exception:unbound-var
  exception:used-before-defined
  exception:wrong-num-args exception:wrong-type-arg
+ exception:not-a-variable
  exception:numerical-overflow
  exception:struct-set!-denied
  exception:system-error
@@ -277,6 +278,8 @@
   (cons 'wrong-number-of-args "^Wrong number of arguments"))
 (define exception:wrong-type-arg
   (cons 'wrong-type-arg "^Wrong type"))
+(define exception:not-a-variable
+  (cons 'wrong-type-arg "^Not a variable"))
 (define exception:numerical-overflow
   (cons 'numerical-overflow "^Numerical overflow"))
 (define exception:struct-set!-denied
diff --git a/test-suite/tests/rtl.test b/test-suite/tests/rtl.test
index 219407c..633425c 100644
--- a/test-suite/tests/rtl.test
+++ b/test-suite/tests/rtl.test
@@ -247,3 +247,12 @@
                             (end-program)))))
                     ((make-top-incrementor))
                     *top-val*))))
+
+(with-test-prefix "box-ref"
+  (pass-if-exception "bad variable" exception:not-a-variable
+    ((assemble-program
+      '((begin-program foo)
+        (assert-nargs-ee/locals 0 2)
+        (box-ref 0 1)
+        (return 0)
+        (end-program))))))
-- 
1.7.10.4


[-- Attachment #4: 0003-Better-Exception-in-RTL-VM.patch --]
[-- Type: application/octet-stream, Size: 1571 bytes --]

From 2828f6f3ec9fa76e7bb38f03d8dbd54c40369898 Mon Sep 17 00:00:00 2001
From: Noah Lavine <noah.b.lavine@gmail.com>
Date: Sat, 20 Apr 2013 19:04:32 -0400
Subject: [PATCH 3/3] Better Exception in RTL VM

 * libguile/vm-engine.c: in box-set!, throw an exception instead of
   aborting if the register does not contain a variable.
 * test-suite/tests/rtl.test: test this behavior.
---
 libguile/vm-engine.c      |    3 ++-
 test-suite/tests/rtl.test |    9 +++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/libguile/vm-engine.c b/libguile/vm-engine.c
index f5f1617..fa8c8fc 100644
--- a/libguile/vm-engine.c
+++ b/libguile/vm-engine.c
@@ -1889,7 +1889,8 @@ RTL_VM_NAME (SCM vm, SCM program, SCM *argv, size_t nargs_)
       SCM var;
       SCM_UNPACK_RTL_12_12 (op, dst, src);
       var = LOCAL_REF (dst);
-      VM_ASSERT (SCM_VARIABLEP (var), abort ());
+      VM_ASSERT (SCM_VARIABLEP (var),
+		 vm_error_not_a_variable ("box-ref (VM instruction)", var));
       VARIABLE_SET (var, LOCAL_REF (src));
       NEXT (1);
     }
diff --git a/test-suite/tests/rtl.test b/test-suite/tests/rtl.test
index 633425c..e97a801 100644
--- a/test-suite/tests/rtl.test
+++ b/test-suite/tests/rtl.test
@@ -256,3 +256,12 @@
         (box-ref 0 1)
         (return 0)
         (end-program))))))
+
+(with-test-prefix "box-set!"
+  (pass-if-exception "bad variable" exception:not-a-variable
+    ((assemble-program
+      '((begin-program foo)
+        (assert-nargs-ee/locals 0 2)
+        (box-ref 0 1)
+        (return 0)
+        (end-program))))))
-- 
1.7.10.4


             reply	other threads:[~2013-04-20 23:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-20 23:30 Noah Lavine [this message]
2013-04-21 15:23 ` Patches for wip-rtl Noah Lavine
2013-04-21 15:50   ` Noah Lavine
2013-04-22 20:27     ` Andy Wingo
2013-04-23  1:34       ` Noah Lavine
2013-04-22 20:39 ` Andy Wingo
2013-04-23  2:38   ` Noah Lavine
2013-04-23 10:13     ` Andy Wingo
2013-05-05  1:29       ` Noah Lavine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+U71=P1RuyZ=LqB73FQzhAan=m8z7dg+7bESYE+zNBW3z3Z5w@mail.gmail.com' \
    --to=noah.b.lavine@gmail.com \
    --cc=guile-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).