From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Noah Lavine Newsgroups: gmane.lisp.guile.devel Subject: Re: Patches for wip-rtl Date: Sun, 21 Apr 2013 11:50:49 -0400 Message-ID: References: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=bcaec544ef92ba130b04dae0eb7b X-Trace: ger.gmane.org 1366559474 28098 80.91.229.3 (21 Apr 2013 15:51:14 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 21 Apr 2013 15:51:14 +0000 (UTC) To: guile-devel Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Sun Apr 21 17:51:19 2013 Return-path: Envelope-to: guile-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1UTwXt-0005ji-ED for guile-devel@m.gmane.org; Sun, 21 Apr 2013 17:51:17 +0200 Original-Received: from localhost ([::1]:36483 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UTwXs-0000Vk-Kj for guile-devel@m.gmane.org; Sun, 21 Apr 2013 11:51:16 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:34207) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UTwXn-0000VP-Rq for guile-devel@gnu.org; Sun, 21 Apr 2013 11:51:14 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UTwXm-0004uQ-Jp for guile-devel@gnu.org; Sun, 21 Apr 2013 11:51:11 -0400 Original-Received: from mail-pd0-f171.google.com ([209.85.192.171]:41690) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UTwXm-0004uH-9t for guile-devel@gnu.org; Sun, 21 Apr 2013 11:51:10 -0400 Original-Received: by mail-pd0-f171.google.com with SMTP id t12so957150pdi.16 for ; Sun, 21 Apr 2013 08:51:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:mime-version:sender:in-reply-to:references:from:date :x-google-sender-auth:message-id:subject:to:content-type; bh=/Pxk9AbgNhCak/dQjBFUqBNu7/pIA23Xev3IKabHueI=; b=Gi3KLHzpwJvI/GWXt0RZA8s//HsynFFKoV2xBuiDCvL6ScePVP0TG8IcEiUlg34okP t4vF7T54DGl6mq1UAtb/QOt3vV7cWCmOkiKO/85pxlf/w+f6bGcc9iRMBmE55NfvMupX Ck2wBgn4VhQziuF4+6yQ5GmxwKpqGVquhfURRzvI7s4K0gf+6zsPZii3m2Nbo1rJthoE 5GleADJK2YEaUuxBKFXkouYokaKuAxH/lrzXAIIG4iD0mbu/gx3CnF/MJ9Ol4wpANH8I KwQ0cF7cebyFyn4oEmnnNlpdDeNo4I7MI7yFRn/QFFpLxL9fQvuS6d9x3jkN9eEOWvK0 lrRQ== X-Received: by 10.68.49.130 with SMTP id u2mr27956901pbn.124.1366559469210; Sun, 21 Apr 2013 08:51:09 -0700 (PDT) Original-Received: by 10.68.7.9 with HTTP; Sun, 21 Apr 2013 08:50:49 -0700 (PDT) In-Reply-To: X-Google-Sender-Auth: Ec54j5S2PtmRQe8v1q6UaqCKunE X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 209.85.192.171 X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:16284 Archived-At: --bcaec544ef92ba130b04dae0eb7b Content-Type: multipart/alternative; boundary=bcaec544ef92ba130804dae0eb79 --bcaec544ef92ba130804dae0eb79 Content-Type: text/plain; charset=ISO-8859-1 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 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 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 >> > > --bcaec544ef92ba130804dae0eb79 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
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&= gt; wrote:
Hello,
Please don't worry about the last part of that message. I reali= zed 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 th= at 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=3D-1, guile will s= egfault. We could fix this, but I'm inclined to leave it as it is, beca= use 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 ge= t corrupted. Does that sound reasonable? If not, it should be simple to ins= ert 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:=
He= llo,

I've attached three patches for wip-rtl. The first is somew= hat different than the other two: it fixes an error that occurred when movi= ng 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 callin= g the wrong one, which made rtl.test fail. I solved this with an @-referenc= e.

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 use= r wants to hand-write RTL code, but it's also very useful in trying a n= ew 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 i= n VM instructions that don't cause immediate problems, but do put the V= M in an inconsistent state. I know of a collection of these errors, and whi= le 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) writ= e the tests in C instead of Scheme.

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

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

<= /div>Best,
Noah


--bcaec544ef92ba130804dae0eb79-- --bcaec544ef92ba130b04dae0eb7b Content-Type: application/octet-stream; name="0001-Bugfix-in-vm-engine.c.patch" Content-Disposition: attachment; filename="0001-Bugfix-in-vm-engine.c.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_hfse7gxs0 RnJvbSAyY2Y3NzAwZWE1MDBlYzFmYTUxMzA3MjAxZjMyZTg1MDIzNWM4ZTEwIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBOb2FoIExhdmluZSA8bm9haC5iLmxhdmluZUBnbWFpbC5jb20+ CkRhdGU6IFN1biwgMjEgQXByIDIwMTMgMTE6MjU6NDkgLTA0MDAKU3ViamVjdDogW1BBVENIXSBC dWdmaXggaW4gdm0tZW5naW5lLmMKCiAqIGxpYmd1aWxlL3ZtLWVuZ2luZS5jOiBmaXggb2ZmLWJ5 LW9uZSBlcnJvciBpbiBiaW5kLXJlc3QgaW5zdHJ1Y3Rpb24uCiAqIHRlc3Qtc3VpdGUvdGVzdHMv cnRsLnRlc3Q6IHRlc3QgdGhlIGZpeC4KLS0tCiBsaWJndWlsZS92bS1lbmdpbmUuYyAgICAgIHwg ICAgOCArKysrKystLQogdGVzdC1zdWl0ZS90ZXN0cy9ydGwudGVzdCB8ICAgMjYgKysrKysrKysr KysrKysrKysrKysrKysrKysKIDIgZmlsZXMgY2hhbmdlZCwgMzIgaW5zZXJ0aW9ucygrKSwgMiBk ZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9saWJndWlsZS92bS1lbmdpbmUuYyBiL2xpYmd1aWxl L3ZtLWVuZ2luZS5jCmluZGV4IGZhOGM4ZmMuLmE4ZmVkZTYgMTAwNjQ0Ci0tLSBhL2xpYmd1aWxl L3ZtLWVuZ2luZS5jCisrKyBiL2xpYmd1aWxlL3ZtLWVuZ2luZS5jCkBAIC0xNTg1LDEyICsxNTg1 LDE2IEBAIFJUTF9WTV9OQU1FIChTQ00gdm0sIFNDTSBwcm9ncmFtLCBTQ00gKmFyZ3YsIHNpemVf dCBuYXJnc18pCiAKICAgICAgIFNDTV9VTlBBQ0tfUlRMXzI0IChvcCwgZHN0KTsKIAotICAgICAg d2hpbGUgKG5hcmdzLS0gPiBkc3QpCisgICAgICBmb3IgKDsgbmFyZ3MgPiBkc3Q7IG5hcmdzLS0p CiAgICAgICAgIHsKLSAgICAgICAgICByZXN0ID0gc2NtX2NvbnMgKExPQ0FMX1JFRiAobmFyZ3Mp LCByZXN0KTsKKyAgICAgICAgICByZXN0ID0gc2NtX2NvbnMgKExPQ0FMX1JFRiAobmFyZ3MgLSAx KSwgcmVzdCk7CiAgICAgICAgICAgTE9DQUxfU0VUIChuYXJncywgU0NNX1VOREVGSU5FRCk7CiAg ICAgICAgIH0KIAorICAgICAgLyogd2UgbmVlZCBuYXJncyA9IGRzdCArIDEgc28gdGhhdCBhIGNh bGwgdG8gcmVzZXJ2ZS1sb2NhbHMgYWZ0ZXIKKwkgdGhpcyB3b24ndCBvdmVyd3JpdGUgdGhlIGxp c3Qgd2UganVzdCBjb25zZWQgdXAuICovCisgICAgICBuYXJncyA9IGRzdCArIDE7CisKICAgICAg IExPQ0FMX1NFVCAoZHN0LCByZXN0KTsKIAogICAgICAgUkVTRVRfRlJBTUUgKGRzdCArIDEpOwpk aWZmIC0tZ2l0IGEvdGVzdC1zdWl0ZS90ZXN0cy9ydGwudGVzdCBiL3Rlc3Qtc3VpdGUvdGVzdHMv cnRsLnRlc3QKaW5kZXggZTk3YTgwMS4uMDQzMjRmYSAxMDA2NDQKLS0tIGEvdGVzdC1zdWl0ZS90 ZXN0cy9ydGwudGVzdAorKysgYi90ZXN0LXN1aXRlL3Rlc3RzL3J0bC50ZXN0CkBAIC0yNjUsMyAr MjY1LDI5IEBACiAgICAgICAgIChib3gtcmVmIDAgMSkKICAgICAgICAgKHJldHVybiAwKQogICAg ICAgICAoZW5kLXByb2dyYW0pKSkpKSkKKworKHdpdGgtdGVzdC1wcmVmaXggImJpbmQtcmVzdCIK KyAgOzsgY2hlY2sgdGhhdCBiaW5kLXJlc3QgbGVhdmVzIG5hcmdzIHNldCBjb3JyZWN0bHkKKyAg KHBhc3MtaWYtZXF1YWwgIm5hcmdzID0gMCIKKyAgICAnKCkKKyAgICAoKGFzc2VtYmxlLXByb2dy YW0KKyAgICAgICcoKGJlZ2luLXByb2dyYW0gZm9vKQorICAgICAgICAoYXNzZXJ0LW5hcmdzLWVl L2xvY2FscyAwIDEpCisgICAgICAgIChiaW5kLXJlc3QgMCkKKyAgICAgICAgOzsgbm9uaW50dWl0 aXZlLCBidXQgdGhlIG91dHB1dCBvZiBiaW5kLXJlc3QgaGFzIHRvIGNvdW50IGFzIGFuCisgICAg ICAgIDs7IGFyZ3VtZW50IGZvciByZXNlcnZlLWxvY2FscyB0byB3b3JrLiB0aGVyZWZvcmUsIGV2 ZW4gaWYgd2UKKyAgICAgICAgOzsgc3RhcnRlZCB3aXRoIDAgYXJndW1lbnRzLCB3ZSBlbmQgdXAg d2l0aCBvbmUuCisgICAgICAgIChhc3NlcnQtbmFyZ3MtZWUgMSkKKyAgICAgICAgKHJldHVybiAw KQorICAgICAgICAoZW5kLXByb2dyYW0pKSkpKQorCisgIChwYXNzLWlmLWVxdWFsICJuYXJncyA9 IDMiCisgICAgJyhiIGMpCisgICAgKChhc3NlbWJsZS1wcm9ncmFtCisgICAgICAnKChiZWdpbi1w cm9ncmFtIGZvbykKKyAgICAgICAgKGFzc2VydC1uYXJncy1lZS9sb2NhbHMgMyAwKQorICAgICAg ICAoYmluZC1yZXN0IDEpCisgICAgICAgIChhc3NlcnQtbmFyZ3MtZWUgMikKKyAgICAgICAgKHJl dHVybiAxKQorICAgICAgICAoZW5kLXByb2dyYW0pKSkKKyAgICAgJ2EgJ2IgJ2MpKSkKLS0gCjEu Ny4xMC40Cgo= --bcaec544ef92ba130b04dae0eb7b--