From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39658) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fIrqt-00031s-Kb for guix-patches@gnu.org; Wed, 16 May 2018 04:32:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fIrqs-0006bV-Jw for guix-patches@gnu.org; Wed, 16 May 2018 04:32:03 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:56675) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fIrqs-0006bL-GP for guix-patches@gnu.org; Wed, 16 May 2018 04:32:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fIrqs-0000cv-7F for guix-patches@gnu.org; Wed, 16 May 2018 04:32:02 -0400 Subject: [bug#31470] [PATCH 0/1] Insert record type ABI checks in constructors Resent-Message-ID: Received: from eggs.gnu.org ([2001:4830:134:3::10]:39563) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fIrqT-0002rl-1G for guix-patches@gnu.org; Wed, 16 May 2018 04:31:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fIrqN-0006UN-BH for guix-patches@gnu.org; Wed, 16 May 2018 04:31:37 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Date: Wed, 16 May 2018 10:31:12 +0200 Message-Id: <20180516083112.29093-1-ludo@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: 31470@debbugs.gnu.org Hello Guix, Record type constructors and accessors are inlined, which is nice for performance, but causes ABI breakage whenever we change a record type definition and forget to recompile the files that use it. In that case, we get an obscure error, sometimes mentioning ‘allocate-struct’ (coming from the record constructor), sometimes suggesting that the wrong field of the record is picked up. This patch introduces ABI checks everywhere a record type constructor is used. When an ABI mismatch is detected, a specific exception is raised: --8<---------------cut here---------------start------------->8--- $ ./pre-inst-env guix system vm gnu/system/examples/bare-bones.tmpl -n Backtrace: 12 (apply-smob/1 #) In ice-9/boot-9.scm: 705:2 11 (call-with-prompt _ _ #) In ice-9/eval.scm: 619:8 10 (_ #(#(#))) In guix/ui.scm: 1535:12 9 (run-guix-command _ . _) In ice-9/boot-9.scm: 829:9 8 (catch _ _ # _) 829:9 7 (catch _ _ # …) In guix/scripts/system.scm: 1218:8 6 (_) 1088:6 5 (process-action _ _ _) In guix/store.scm: 1443:24 4 (run-with-store _ _ #:guile-for-build _ #:system _ #:target _) In guix/scripts/system.scm: 1101:13 3 (_ _) 799:18 2 (perform-action vm #< kernel: # …) In gnu/system/vm.scm: 821:31 1 (system-qemu-image/shared-store-script #< kernel: # …) 715:2 0 (virtualized-operating-system _ _ _) gnu/system/vm.scm:715:2: In procedure virtualized-operating-system: ERROR: #>: record ABI mismatch; recompilation needed --8<---------------cut here---------------end--------------->8--- In this case that’s because I modified without recompiling gnu/system/vm.scm. Once I’ve rebuilt it, everything is alright. The shortcoming of the approach is that accessors do not perform these ABI checks, so we can still silently miss ABI mismatch issues, but I think it would be too costly to do that. The overhead of those checks in constructors is probably OK since we’re allocating anyway. This is what constructors now expand to: --8<---------------cut here---------------start------------->8--- scheme@(guix records)> (define-record-type* thing make-thing thing? (name thing-name (default 32)) (port thing-port (default (current-output-port)) (thunked))) scheme@(guix records)> ,optimize (thing) $40 = (begin (if (eq? #{% abi-cookie}# 1515259617) (if #f #f) (throw 'record-abi-mismatch-error )) (let ((s (allocate-struct 2))) (struct-set! s 0 32) (struct-set! s 1 (lambda () (current-output-port))) s)) --8<---------------cut here---------------end--------------->8--- Thoughts? Ludo’. Ludovic Courtès (1): records: Insert record type ABI checks in constructors. guix/records.scm | 54 ++++++++++++++++++++++++++++++++++++++++++++--- tests/records.scm | 30 +++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 4 deletions(-) -- 2.17.0