From: "Ludovic Courtès" <ludo@gnu.org>
To: Mark H Weaver <mhw@netris.org>
Cc: Pierre Neidhardt <mail@ambrevar.xyz>, 33848@debbugs.gnu.org
Subject: bug#33848: Store references in SBCL-compiled code are "invisible"
Date: Thu, 08 Apr 2021 12:13:37 +0200 [thread overview]
Message-ID: <87ft01axta.fsf@gnu.org> (raw)
In-Reply-To: <87h7kj7j7x.fsf@netris.org> (Mark H. Weaver's message of "Tue, 06 Apr 2021 07:19:51 -0400")
Hi Mark,
Mark H Weaver <mhw@netris.org> skribis:
> From 6eec36e66d20d82fe02c6de793422875477b90d8 Mon Sep 17 00:00:00 2001
> From: Mark H Weaver <mhw@netris.org>
> Date: Fri, 2 Apr 2021 18:36:23 -0400
> Subject: [PATCH] DRAFT: grafts: Support rewriting UTF-16 and UTF-32 store
> references.
>
> * guix/build/graft.scm (replace-store-references): Add support for
> finding and rewriting UTF-16 and UTF-32 store references.
> * tests/grafts.scm: Add tests.
Please add a “Fixes” line in the commit log.
I’m not reviewing the code in depth and I trust your judgment.
The risks of bugs I can think of are: missed ASCII references (a
regression, whereby some ASCII references would not get rewritten), and
unrelated UTF-{16,32}-base32-looking references getting rewritten.
I guess the latter is very unlikely because only sequences found in the
replacement table may be rewritten, right?
The former should be caught by ‘tests/grafts.scm’ but we could also
check the closure of real-world systems, for instance, to make sure it
doesn’t refer to ungrafted things.
Do you know how this affects performance?
Some superficial comments:
> +(define (possible-utf16-hash? buffer i w)
> + (and (<= (* 2 hash-length) (- i w))
> + (let loop ((j (+ 1 (- i (* 2 hash-length)))))
> + (or (>= j i)
> + (and (zero? (bytevector-u8-ref buffer j))
> + (loop (+ j 2)))))))
> +
> +(define (possible-utf32-hash? buffer i w)
> + (and (<= (* 4 hash-length) (- i w))
> + (let loop ((j (+ 1 (- i (* 4 hash-length)))))
> + (or (>= j i)
> + (and (zero? (bytevector-u8-ref buffer j))
> + (zero? (bytevector-u8-ref buffer (+ j 1)))
> + (zero? (bytevector-u8-ref buffer (+ j 2)))
> + (loop (+ j 4)))))))
> +
> +(define (insert-nuls char-size bv)
Perhaps add short docstrings for clarity.
> +(for-each
> + (lambda (char-size1)
> + (for-each
> + (lambda (char-size2)
> + (for-each
> + (lambda (gap)
> + (for-each
> + (lambda (offset)
> + (test-equal (format #f "replace-store-references, char-sizes ~a ~a, gap ~s, offset ~a"
> + char-size1 char-size2 gap offset)
> + (string-append (make-string offset #\=)
> + (nul-expand (string-append "/gnu/store/"
> + (make-string 32 #\6)
> + "-BlahBlaH")
> + char-size1)
> + gap
> + (nul-expand (string-append "/gnu/store/"
> + (make-string 32 #\8)
> + "-SoMeTHiNG")
> + char-size2)
> + (list->string (map integer->char (iota 77 33))))
> +
> + ;; Create input data where the right-hand-size of the dash ("-something"
> + ;; here) goes beyond the end of the internal buffer of
> + ;; 'replace-store-references'.
> + (let* ((content (string-append (make-string offset #\=)
> + (nul-expand (string-append "/gnu/store/"
> + (make-string 32 #\5)
> + "-blahblah")
> + char-size1)
> + gap
> + (nul-expand (string-append "/gnu/store/"
> + (make-string 32 #\7)
> + "-something")
> + char-size2)
> + (list->string
> + (map integer->char (iota 77 33)))))
> + (replacement (alist->vhash
> + `((,(make-string 32 #\5)
> + . ,(string->utf8 (string-append
> + (make-string 32 #\6)
> + "-BlahBlaH")))
> + (,(make-string 32 #\7)
> + . ,(string->utf8 (string-append
> + (make-string 32 #\8)
> + "-SoMeTHiNG")))))))
> + (call-with-output-string
> + (lambda (output)
> + ((@@ (guix build graft) replace-store-references)
> + (open-input-string content) output
> + replacement
> + "/gnu/store"))))))
> + ;; offsets to test
> + (map (lambda (i) (- buffer-size (* 40 char-size1) i))
> + (iota 30))))
> + ;; gaps
> + '("" "-" " " "a")))
> + ;; char-size2 values to test
> + '(1 2)))
> + ;; char-size1 values to test
> + '(1 2 4))
For clarity, perhaps you can define a top-level procedure for the test
and call it from ‘for-each’.
Modulo these very minor issues, it looks like it’s ready to go!
Thank you,
Ludo’.
next prev parent reply other threads:[~2021-04-08 10:16 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-23 14:19 bug#33848: Store references in SBCL-compiled code are "invisible" Ludovic Courtès
2018-12-23 15:05 ` Pierre Neidhardt
2018-12-24 14:57 ` Ludovic Courtès
2018-12-23 16:45 ` Mark H Weaver
2018-12-23 17:32 ` Ludovic Courtès
2018-12-23 22:01 ` Pierre Neidhardt
2018-12-24 15:06 ` Ludovic Courtès
2018-12-24 17:08 ` Pierre Neidhardt
2018-12-26 16:07 ` Ludovic Courtès
2018-12-24 18:12 ` Mark H Weaver
2018-12-24 23:58 ` Pierre Neidhardt
2018-12-26 16:14 ` Ludovic Courtès
2018-12-27 10:37 ` Pierre Neidhardt
2018-12-27 14:03 ` Mark H Weaver
2018-12-27 14:45 ` Ludovic Courtès
2018-12-27 15:02 ` Pierre Neidhardt
2018-12-27 16:15 ` Pierre Neidhardt
2018-12-27 17:03 ` Ludovic Courtès
2018-12-27 18:57 ` Pierre Neidhardt
2018-12-27 21:54 ` Ludovic Courtès
2018-12-27 22:05 ` Pierre Neidhardt
2018-12-27 22:59 ` Ludovic Courtès
2018-12-28 7:47 ` Pierre Neidhardt
2021-03-30 10:15 ` Pierre Neidhardt
2021-03-30 20:09 ` Ludovic Courtès
2021-03-31 7:10 ` Pierre Neidhardt
2021-03-31 16:12 ` Pierre Neidhardt
2021-03-31 20:42 ` Ludovic Courtès
2021-03-31 20:57 ` Pierre Neidhardt
2021-04-01 17:23 ` Mark H Weaver
2021-04-02 15:13 ` Ludovic Courtès
2021-04-01 6:03 ` Mark H Weaver
2021-04-01 7:13 ` Pierre Neidhardt
2021-04-01 7:57 ` Ludovic Courtès
2021-04-01 8:48 ` Pierre Neidhardt
2021-04-01 9:07 ` Guillaume Le Vaillant
2021-04-01 9:13 ` Pierre Neidhardt
2021-04-01 9:52 ` Guillaume Le Vaillant
2021-04-01 10:06 ` Pierre Neidhardt
2021-04-01 10:07 ` Pierre Neidhardt
2021-04-01 15:24 ` Ludovic Courtès
2021-04-01 17:33 ` Mark H Weaver
2021-04-02 22:46 ` Mark H Weaver
2021-04-03 6:51 ` Pierre Neidhardt
2021-04-03 20:10 ` Mark H Weaver
2021-04-05 19:45 ` Ludovic Courtès
2021-04-05 23:04 ` Mark H Weaver
2021-04-06 8:16 ` Ludovic Courtès
2021-04-06 8:23 ` Pierre Neidhardt
2021-04-30 20:03 ` Mark H Weaver
2021-05-01 9:22 ` Pierre Neidhardt
2021-05-11 8:46 ` Ludovic Courtès
2021-04-06 17:23 ` Leo Famulari
2021-04-06 23:21 ` Mark H Weaver
2021-04-06 11:19 ` Mark H Weaver
2021-04-08 10:13 ` Ludovic Courtès [this message]
2021-04-13 20:06 ` Mark H Weaver
2021-04-14 10:55 ` Ludovic Courtès
2021-04-14 22:37 ` Leo Famulari
2021-04-15 7:26 ` Mark H Weaver
2021-04-16 9:44 ` Pierre Neidhardt
2018-12-27 13:52 ` Danny Milosavljevic
2018-12-27 14:29 ` Mark H Weaver
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ft01axta.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=33848@debbugs.gnu.org \
--cc=mail@ambrevar.xyz \
--cc=mhw@netris.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.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.