unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* Patches for wip-rtl
@ 2013-04-20 23:30 Noah Lavine
  2013-04-21 15:23 ` Noah Lavine
  2013-04-22 20:39 ` Andy Wingo
  0 siblings, 2 replies; 9+ messages in thread
From: Noah Lavine @ 2013-04-20 23:30 UTC (permalink / raw)
  To: guile-devel


[-- 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: Patches for wip-rtl
  2013-04-20 23:30 Patches for wip-rtl Noah Lavine
@ 2013-04-21 15:23 ` Noah Lavine
  2013-04-21 15:50   ` Noah Lavine
  2013-04-22 20:39 ` Andy Wingo
  1 sibling, 1 reply; 9+ messages in thread
From: Noah Lavine @ 2013-04-21 15:23 UTC (permalink / raw)
  To: guile-devel

[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]

Hello,

Please don't worry about the last part of that message. I realized I can
test the nargs value by (ab)using the assert-nargs-ee instruction.

On a related note, there's a certain piece of code in vm.c that is not
robust in the face of a corrupt nargs. Specifically, if you call
vm_error_wrong_num_args (vm.c:518 in wip-rtl) with nargs=-1, guile will
segfault. We could fix this, but I'm inclined to leave it as it is, because
this can only happen if nargs is corrupted by some earlier call, and I
think the right approach is to test the VM enough that nargs doesn't get
corrupted. Does that sound reasonable? If not, it should be simple to
insert some check there the nargs is big enough not to corrupt anything.

Noah


On Sat, Apr 20, 2013 at 7:30 PM, Noah Lavine <noah.b.lavine@gmail.com>wrote:

> 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 #2: Type: text/html, Size: 3023 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Patches for wip-rtl
  2013-04-21 15:23 ` Noah Lavine
@ 2013-04-21 15:50   ` Noah Lavine
  2013-04-22 20:27     ` Andy Wingo
  0 siblings, 1 reply; 9+ messages in thread
From: Noah Lavine @ 2013-04-21 15:50 UTC (permalink / raw)
  To: guile-devel


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

And here's another patch that fixes an off-by-one error in the bind-rest
instruction. This solves the problems I was having in my earlier email
about an abort in the VM.

Best,
Noah


On Sun, Apr 21, 2013 at 11:23 AM, Noah Lavine <noah.b.lavine@gmail.com>wrote:

> Hello,
>
> Please don't worry about the last part of that message. I realized I can
> test the nargs value by (ab)using the assert-nargs-ee instruction.
>
> On a related note, there's a certain piece of code in vm.c that is not
> robust in the face of a corrupt nargs. Specifically, if you call
> vm_error_wrong_num_args (vm.c:518 in wip-rtl) with nargs=-1, guile will
> segfault. We could fix this, but I'm inclined to leave it as it is, because
> this can only happen if nargs is corrupted by some earlier call, and I
> think the right approach is to test the VM enough that nargs doesn't get
> corrupted. Does that sound reasonable? If not, it should be simple to
> insert some check there the nargs is big enough not to corrupt anything.
>
> Noah
>
>
> On Sat, Apr 20, 2013 at 7:30 PM, Noah Lavine <noah.b.lavine@gmail.com>wrote:
>
>> 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: 3802 bytes --]

[-- Attachment #2: 0001-Bugfix-in-vm-engine.c.patch --]
[-- Type: application/octet-stream, Size: 2231 bytes --]

From 2cf7700ea500ec1fa51307201f32e850235c8e10 Mon Sep 17 00:00:00 2001
From: Noah Lavine <noah.b.lavine@gmail.com>
Date: Sun, 21 Apr 2013 11:25:49 -0400
Subject: [PATCH] Bugfix in vm-engine.c

 * libguile/vm-engine.c: fix off-by-one error in bind-rest instruction.
 * test-suite/tests/rtl.test: test the fix.
---
 libguile/vm-engine.c      |    8 ++++++--
 test-suite/tests/rtl.test |   26 ++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/libguile/vm-engine.c b/libguile/vm-engine.c
index fa8c8fc..a8fede6 100644
--- a/libguile/vm-engine.c
+++ b/libguile/vm-engine.c
@@ -1585,12 +1585,16 @@ RTL_VM_NAME (SCM vm, SCM program, SCM *argv, size_t nargs_)
 
       SCM_UNPACK_RTL_24 (op, dst);
 
-      while (nargs-- > dst)
+      for (; nargs > dst; nargs--)
         {
-          rest = scm_cons (LOCAL_REF (nargs), rest);
+          rest = scm_cons (LOCAL_REF (nargs - 1), rest);
           LOCAL_SET (nargs, SCM_UNDEFINED);
         }
 
+      /* we need nargs = dst + 1 so that a call to reserve-locals after
+	 this won't overwrite the list we just consed up. */
+      nargs = dst + 1;
+
       LOCAL_SET (dst, rest);
 
       RESET_FRAME (dst + 1);
diff --git a/test-suite/tests/rtl.test b/test-suite/tests/rtl.test
index e97a801..04324fa 100644
--- a/test-suite/tests/rtl.test
+++ b/test-suite/tests/rtl.test
@@ -265,3 +265,29 @@
         (box-ref 0 1)
         (return 0)
         (end-program))))))
+
+(with-test-prefix "bind-rest"
+  ;; check that bind-rest leaves nargs set correctly
+  (pass-if-equal "nargs = 0"
+    '()
+    ((assemble-program
+      '((begin-program foo)
+        (assert-nargs-ee/locals 0 1)
+        (bind-rest 0)
+        ;; nonintuitive, but the output of bind-rest has to count as an
+        ;; argument for reserve-locals to work. therefore, even if we
+        ;; started with 0 arguments, we end up with one.
+        (assert-nargs-ee 1)
+        (return 0)
+        (end-program)))))
+
+  (pass-if-equal "nargs = 3"
+    '(b c)
+    ((assemble-program
+      '((begin-program foo)
+        (assert-nargs-ee/locals 3 0)
+        (bind-rest 1)
+        (assert-nargs-ee 2)
+        (return 1)
+        (end-program)))
+     'a 'b 'c)))
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: Patches for wip-rtl
  2013-04-21 15:50   ` Noah Lavine
@ 2013-04-22 20:27     ` Andy Wingo
  2013-04-23  1:34       ` Noah Lavine
  0 siblings, 1 reply; 9+ messages in thread
From: Andy Wingo @ 2013-04-22 20:27 UTC (permalink / raw)
  To: Noah Lavine; +Cc: guile-devel

Hi Noah,

On Sun 21 Apr 2013 17:50, Noah Lavine <noah.b.lavine@gmail.com> writes:

> +    ((assemble-program
> +      '((begin-program foo)
> +        (assert-nargs-ee/locals 0 1)
> +        (bind-rest 0)
> +        ;; nonintuitive, but the output of bind-rest has to count as an
> +        ;; argument for reserve-locals to work. therefore, even if we
> +        ;; started with 0 arguments, we end up with one.
> +        (assert-nargs-ee 1)
> +        (return 0)
> +        (end-program)))))

Do I understand you correctly that you're just making a sequence of
non-idiomatic instructions because you know that doing the
assert-nargs-ee will check nargs?  I would think in a normal case you'd
want to do "assert-nargs-ge 0", then "bind-rest 0", then "reserve-locals
0" (or not; in this case it would not be necessary).

However the bug you have seen with bind-rest is probably also present
with keyword arguments.  We should probably just change the
reserve-locals instruction to also take "nargs" as a value, given that
we know it statically at compile-time.

Does that make sense to you?

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Patches for wip-rtl
  2013-04-20 23:30 Patches for wip-rtl Noah Lavine
  2013-04-21 15:23 ` Noah Lavine
@ 2013-04-22 20:39 ` Andy Wingo
  2013-04-23  2:38   ` Noah Lavine
  1 sibling, 1 reply; 9+ messages in thread
From: Andy Wingo @ 2013-04-22 20:39 UTC (permalink / raw)
  To: Noah Lavine; +Cc: guile-devel

Hi :)

Thanks for working on the RTL VM!

First of all, sorry about that linker error.  I'm trying to make RTL
programs more debuggable by hacking on the linker, so that it emits
proper debugging information.  I should have caught that typo though!

I think however that the strategy of making the VM somehow more
resilient is not going to work.  There are so many ways that invalid
instructions can make the VM fail -- and you have no idea what you can
rely on if those invariants fail.  So it doesn't seem to me that it's
possible to do that job well at all, and for that reason it seems to me
that we shouldn't try very hard.

So my answer for dealing with VM issues would be to try to narrow down
the problem, then use instruction-level traps to print instructions as
you go.  Once you find the problem you probably won't be able to
recover; oh well.  At least you would know how you got there.

And I don't know how you've been able to get as far as you have with the
deplorable state of the debugging infrastructure.  You can't even
disassemble a function you just made!  I'll be working on that this
week; we'll see what happens.

So in summary, dunno.  Surely there is some way we can do things better,
but I wanted to register my skepticism for this line of hacking.  What
do you think?

Cheers,

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Patches for wip-rtl
  2013-04-22 20:27     ` Andy Wingo
@ 2013-04-23  1:34       ` Noah Lavine
  0 siblings, 0 replies; 9+ messages in thread
From: Noah Lavine @ 2013-04-23  1:34 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

Hello,

On Mon, Apr 22, 2013 at 4:27 PM, Andy Wingo <wingo@pobox.com> wrote:

> Hi Noah,
>
> Do I understand you correctly that you're just making a sequence of
> non-idiomatic instructions because you know that doing the
> assert-nargs-ee will check nargs?  I would think in a normal case you'd
> want to do "assert-nargs-ge 0", then "bind-rest 0", then "reserve-locals
> 0" (or not; in this case it would not be necessary).
>

Yes, that's exactly why I'm doing it. I just wanted to find a way to test
that the bug was fixed and stayed fixed. In this case that's a bit weird,
because we probably don't want to guarantee that this pattern of
instructions will continue to work, but I don't have a great alternative. I
think any sort of test for this is going to be dependent on the internals
of the VM. (My earlier email about adding accessors for the VM was about
this same bug, but then I realized that nargs isn't part of the vm
structure so I couldn't solve it like that.)


> However the bug you have seen with bind-rest is probably also present
> with keyword arguments.  We should probably just change the
> reserve-locals instruction to also take "nargs" as a value, given that
> we know it statically at compile-time.
>
> Does that make sense to you?
>

I'm not sure. As long as nargs is going to be a local variable, I don't see
a reason not to use it. It might make more sense to either keep it the way
it is or change every instruction that uses nargs. But I haven't looked to
see how many there are, or how important it is that they know nargs.

Noah

[-- Attachment #2: Type: text/html, Size: 2259 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Patches for wip-rtl
  2013-04-22 20:39 ` Andy Wingo
@ 2013-04-23  2:38   ` Noah Lavine
  2013-04-23 10:13     ` Andy Wingo
  0 siblings, 1 reply; 9+ messages in thread
From: Noah Lavine @ 2013-04-23  2:38 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

[-- Attachment #1: Type: text/plain, Size: 2814 bytes --]

Hello,

On Mon, Apr 22, 2013 at 4:39 PM, Andy Wingo <wingo@pobox.com> wrote:

> Hi :)
>
> Thanks for working on the RTL VM!
>

Thanks for doing most of the work! I'm happy to help. :-)


> First of all, sorry about that linker error.  I'm trying to make RTL
> programs more debuggable by hacking on the linker, so that it emits
> proper debugging information.  I should have caught that typo though!
>

Debugging would be great. I have thought about stopping this and working on
debugging stuff some, but that actually seems harder than what I'm doing
now, and I would like to get the compiler working.


> I think however that the strategy of making the VM somehow more
> resilient is not going to work.  There are so many ways that invalid
> instructions can make the VM fail -- and you have no idea what you can
> rely on if those invariants fail.  So it doesn't seem to me that it's
> possible to do that job well at all, and for that reason it seems to me
> that we shouldn't try very hard.
>

I assume you're talking about the box-ref and box-set! stuff with checking
for non-variables, right? I agree in general, but those instructions could
plausibly be emitted for any code that uses module introspection to get its
own variable objects, right?. At that point the not-a-variable error would
be a user type error, and so I think it's better to throw wrong-type-arg.
(But of course, that's not my real motivation - I just happened to hit this
while debugging the compiler.)

And I don't know how you've been able to get as far as you have with the
> deplorable state of the debugging infrastructure.  You can't even
> disassemble a function you just made!  I'll be working on that this
> week; we'll see what happens.
>
> So in summary, dunno.  Surely there is some way we can do things better,
> but I wanted to register my skepticism for this line of hacking.  What
> do you think?
>

First of all, I agree that debugging infrastructure would be great. I'm
very glad you're working on it - that should help a lot. I'm not sure if I
have much to add there right now, but I will keep it in mind.

As for the line of hacking, as I said above, I think in the particular case
of box-ref and box-set! it's justified; I don't know in general. I
certainly agree that once the VM's internal state is corrupt, all bets are
off, and we probably won't be able to continue. One thing that would be
interesting - and I don't know if we do this now - is using different VMs
for different parts of the code. Specifically, if the REPL ran in a
different VM than user code, then the REPL wouldn't die in these cases.
That might also enable cool debugging things, like single-stepping the user
VM and examining its registers. I noticed that we already have support for
changing the active VM. What do you think?

Best,
Noah

[-- Attachment #2: Type: text/html, Size: 3806 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Patches for wip-rtl
  2013-04-23  2:38   ` Noah Lavine
@ 2013-04-23 10:13     ` Andy Wingo
  2013-05-05  1:29       ` Noah Lavine
  0 siblings, 1 reply; 9+ messages in thread
From: Andy Wingo @ 2013-04-23 10:13 UTC (permalink / raw)
  To: Noah Lavine; +Cc: guile-devel

Heya,

On Tue 23 Apr 2013 04:38, Noah Lavine <noah.b.lavine@gmail.com> writes:

> I assume you're talking about the box-ref and box-set! stuff with
> checking for non-variables, right? I agree in general, but those
> instructions could plausibly be emitted for any code that uses module
> introspection to get its own variable objects, right?

Ah I see what you mean.  There are two uses of variables in the VM: one
for calls to variable-ref or variable-set, and the other for internal
use.  For the internal uses, we know the variable should indeed be a
variable and so we should emit the checks.  For calls to variable-ref /
variable-set, I think it would be best if somehow the compiler could
replace those calls with (if (call variable? x) (primcall box-ref x)
(call error ...)).  Dunno.  WDYT?

> One thing that would be interesting - and I don't know if we do this
> now - is using different VMs for different parts of the
> code. Specifically, if the REPL ran in a different VM than user code,
> then the REPL wouldn't die in these cases.  That might also enable
> cool debugging things, like single-stepping the user VM and examining
> its registers. I noticed that we already have support for changing the
> active VM. What do you think?

We can do that already to a degree... probably the best "fallback" that
we have is the stack VM, actually.  What happens if you install traps on
the VM that call stack VM procedures and then you run an RTL function?
I would think that you should be able to do something useful there.

Just a thought!

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Patches for wip-rtl
  2013-04-23 10:13     ` Andy Wingo
@ 2013-05-05  1:29       ` Noah Lavine
  0 siblings, 0 replies; 9+ messages in thread
From: Noah Lavine @ 2013-05-05  1:29 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]

Hello,

On Tue, Apr 23, 2013 at 6:13 AM, Andy Wingo <wingo@pobox.com> wrote:

> Heya,
>
> On Tue 23 Apr 2013 04:38, Noah Lavine <noah.b.lavine@gmail.com> writes:
>
> Ah I see what you mean.  There are two uses of variables in the VM: one
> for calls to variable-ref or variable-set, and the other for internal
> use.  For the internal uses, we know the variable should indeed be a
> variable and so we should emit the checks.  For calls to variable-ref /
> variable-set, I think it would be best if somehow the compiler could
> replace those calls with (if (call variable? x) (primcall box-ref x)
> (call error ...)).  Dunno.  WDYT?
>

Do we usually omit checks for internal stuff? If so, I agree, we should do
that here too. But right now we do the check every time anyway - the only
difference my patch made is whether we failed with abort() or
vm_error_not_a_variable.

I agree that that doesn't make much sense as an error for internal stuff.
Maybe change it to vm_error_bad_instruction or something like that for now?
In general I agree that we should be able to do nice error checking only
for user-generated box-refs, but that seems more complex than we need right
now (unless you want to add it to the VM).


> > One thing that would be interesting - and I don't know if we do this
> > now - is using different VMs for different parts of the
> > code. Specifically, if the REPL ran in a different VM than user code,
> > then the REPL wouldn't die in these cases.  That might also enable
> > cool debugging things, like single-stepping the user VM and examining
> > its registers. I noticed that we already have support for changing the
> > active VM. What do you think?
>
> We can do that already to a degree... probably the best "fallback" that
> we have is the stack VM, actually.  What happens if you install traps on
> the VM that call stack VM procedures and then you run an RTL function?
> I would think that you should be able to do something useful there.
>

That does sound interesting, but I've never used traps. I've been meaning
to learn them some time so I can try to remember tail calls for debugging,
but that probably won't be for a little while.

Noah

[-- Attachment #2: Type: text/html, Size: 2933 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-05-05  1:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-20 23:30 Patches for wip-rtl Noah Lavine
2013-04-21 15:23 ` 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

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).