unofficial mirror of bug-guile@gnu.org 
 help / color / mirror / Atom feed
* bug#51264: Calling ‘texi-fragment->stexi’ in parallel leads to crashes
@ 2021-10-18 12:54 Ludovic Courtès
  2021-10-22 11:56 ` Ludovic Courtès
  2021-11-01 17:52 ` bug#39601: srfi library naming in r7rs lloda
  0 siblings, 2 replies; 6+ messages in thread
From: Ludovic Courtès @ 2021-10-18 12:54 UTC (permalink / raw)
  To: 51264

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

Hello!

I just stumbled upon this bug (here I use Guix to feed Texinfo strings
but I suppose we could reduce the test case to be Guix-less.)

Test case:


[-- Attachment #2: the test case --]
[-- Type: text/plain, Size: 588 bytes --]

(use-modules (guix) (gnu) (texinfo)
             (ice-9 threads))

(define sequential? (getenv "SEQUENTIAL"))

(define (for-each/maybe-parallel proc lst)
  (if (pk 'sequential? sequential?)
      (for-each proc lst)
      (n-par-for-each 10 proc lst)))

(for-each/maybe-parallel
 (lambda (package)
   (and=> (package-description package)
          (lambda (str)
            (catch 'parser-error
              (lambda ()
                (texi-fragment->stexi str))
              (lambda args
                (pk 'bah! args)
                (error "failed"))))))
 (fold-packages cons '()))

[-- Attachment #3: Type: text/plain, Size: 1342 bytes --]


This code crashes when using ‘n-par-for-each’ but passes when run
sequentially:

--8<---------------cut here---------------start------------->8---
$ guix time-machine --commit=9cda21cf20a5c9bdf97e3a6d6c8f901fc3e4307d -- repl -- bug-texi-parser-parallel.scm

;;; (sequential? #f)

;;; (bah! (parser-error #f "Unknown command" codeand))
In thread:
failed

;;; (bah! (parser-error #f "Unknown command" endm))
In thread:
failed

;;; (bah! (parser-error #f "Unknown command" cod))
In thread:
failed

;;; (bah! (parser-error #f "Unknown command" comm))
In thread:
failed

;;; (bah! (parser-error #f "Unknown command" enum))
In thread:
failed

;;; (bah! (parser-error #f "Unknown command" cod))
In thread:
failed

;;; (bah! (parser-error #f "Unknown command" enem))
In thread:
failed


;;; (bah! (parser-error #f "Unknown command" endmand))
In thread:
failed
;;;;; (bah! (parser-error #f "Unknown command" eomm))
In thread:
failed
$ SEQUENTIAL=y guix time-machine --commit=9cda21cf20a5c9bdf97e3a6d6c8f901fc3e4307d -- repl -- bug-texi-parser-parallel.scm

;;; (sequential? "y")
$ guix repl -- <(echo '(pk (version))')

;;; ("3.0.7")
--8<---------------cut here---------------end--------------->8---

The bits shown in the ‘parser-error’ arguments suggests memory
corruption.

Ludo’.

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

* bug#51264: Calling ‘texi-fragment->stexi’ in parallel leads to crashes
  2021-10-18 12:54 bug#51264: Calling ‘texi-fragment->stexi’ in parallel leads to crashes Ludovic Courtès
@ 2021-10-22 11:56 ` Ludovic Courtès
  2021-11-01 17:52 ` bug#39601: srfi library naming in r7rs lloda
  1 sibling, 0 replies; 6+ messages in thread
From: Ludovic Courtès @ 2021-10-22 11:56 UTC (permalink / raw)
  To: 51264-done

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

Ludovic Courtès <ludo@gnu.org> skribis:

> I just stumbled upon this bug (here I use Guix to feed Texinfo strings
> but I suppose we could reduce the test case to be Guix-less.)

Here’s a standalone reproducer:


[-- Attachment #2: the reproducer --]
[-- Type: text/plain, Size: 563 bytes --]

(use-modules (texinfo)
             (sxml simple)
             (ice-9 threads))

(define sequential? (getenv "SEQUENTIAL"))

(define (for-each/maybe-parallel proc lst)
  (if (pk 'sequential? sequential?)
      (for-each proc lst)
      (n-par-for-each 6 proc lst)))

(setvbuf (current-output-port) 'none)
(for-each/maybe-parallel
 (lambda (str)
   (catch 'parser-error
     (lambda ()
       (texi-fragment->stexi str))
     (lambda args
       (pk 'bah! args '<<>> str)
       (error "failed"))))
 (make-list 5000 "Hello @code{world}, this is @emph{Texinfo}."))

[-- Attachment #3: Type: text/plain, Size: 245 bytes --]


It turned out that (sxml ssax input-parse) would reuse the same global
buffer for each call to ‘next-token’ and ‘next-token-of’ (the Texinfo
parser uses the latter).

Fixed in 3b42b1eb526a85e4fac772e1837046e56e3b9bdc.

Ludo’.

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

* bug#39601: srfi library naming in r7rs
  2021-10-18 12:54 bug#51264: Calling ‘texi-fragment->stexi’ in parallel leads to crashes Ludovic Courtès
  2021-10-22 11:56 ` Ludovic Courtès
@ 2021-11-01 17:52 ` lloda
  2021-11-01 18:42   ` Taylan Kammer
  1 sibling, 1 reply; 6+ messages in thread
From: lloda @ 2021-11-01 17:52 UTC (permalink / raw)
  To: Taylan Kammer; +Cc: 39601, pclouds


Hi Taylan,

Your patch leaks a bunch of identifiers, could you fix that?

thanks

	Daniel






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

* bug#39601: srfi library naming in r7rs
  2021-11-01 17:52 ` bug#39601: srfi library naming in r7rs lloda
@ 2021-11-01 18:42   ` Taylan Kammer
  2021-11-02 17:53     ` lloda
  2021-11-03  7:37     ` Linus Björnstam
  0 siblings, 2 replies; 6+ messages in thread
From: Taylan Kammer @ 2021-11-01 18:42 UTC (permalink / raw)
  To: lloda; +Cc: 39601, pclouds

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

On 01.11.2021 18:52, lloda wrote:
> 
> Hi Taylan,
> 
> Your patch leaks a bunch of identifiers, could you fix that?
> 
> thanks
> 
> 	Daniel
> 

Apparently anything defined in boot-9 is implicitly made public in
the (guile) module, I wasn't aware of that.

Is there a work-around that allows one to define helpers that can
be used by multiple definitions?

Attached is a naive fix that duplicates a bunch of helpers which is
not very nice.

-- 
Taylan

[-- Attachment #2: 0001-Improve-support-for-R6-R7-SRFI-module-name-formats.patch --]
[-- Type: text/plain, Size: 7324 bytes --]

From 1c655b291cfeb7f89bcc95c9c23c6013708e103a Mon Sep 17 00:00:00 2001
From: Taylan Kammer <taylan.kammer@gmail.com>
Date: Mon, 10 May 2021 18:12:34 +0200
Subject: [PATCH] Improve support for R6/R7 SRFI module name formats.

Fixes <https://bugs.gnu.org/39601>.

Partly fixes <https://bugs.gnu.org/40371>.

It was already possible to import an SRFI module by referencing it
as (srfi :n) which is automatically translated to (srfi srfi-n), but
this conversion was only done during import.  After this change, it's
also possible to define a library as (srfi :n) which is automatically
translated to (srfi srfi-n) during definition.

It was not possible at all to define or import SRFI module names in the
R7RS format, (srfi n), where n is a non-negative exact integer.  It is
now possible both to define and import them as such, realized through
the same kind of conversion to a canonical (srfi srfi-n) name.

* module/ice-9/r6rs-libraries.scm: Numerous changes.
---
 module/ice-9/r6rs-libraries.scm | 130 ++++++++++++++++++++++++++------
 1 file changed, 108 insertions(+), 22 deletions(-)

diff --git a/module/ice-9/r6rs-libraries.scm b/module/ice-9/r6rs-libraries.scm
index c6ba6a496..f27b07841 100644
--- a/module/ice-9/r6rs-libraries.scm
+++ b/module/ice-9/r6rs-libraries.scm
@@ -20,7 +20,53 @@
 ;; This file is included from boot-9.scm and assumes the existence of (and 
 ;; expands into) procedures and syntactic forms defined therein.
 
+;; Note that we can't use top-level define for helpers here as it will
+;; pollute the (guile) module.
+
 (define (resolve-r6rs-interface import-spec)
+  (define (sym? stx)
+    (symbol? (syntax->datum stx)))
+
+  (define (n? stx)
+    (let ((n (syntax->datum stx)))
+      (and (exact-integer? n)
+           (not (negative? n)))))
+
+  (define (colon-n? x)
+    (let ((sym (syntax->datum x)))
+      (and (symbol? sym)
+           (let ((str (symbol->string sym)))
+             (and (string-prefix? ":" str)
+                  (let ((num (string->number (substring str 1))))
+                    (and (exact-integer? num)
+                         (not (negative? num)))))))))
+
+  (define (srfi-name? stx)
+    (syntax-case stx (srfi)
+      ((srfi n rest ...)
+       (and (and-map sym? #'(rest ...))
+            (or (n? #'n)
+                (colon-n? #'n))))
+      (_ #f)))
+
+  (define (module-name? stx)
+    (or (srfi-name? stx)
+        (syntax-case stx ()
+          ((name name* ...)
+           (and-map sym? #'(name name* ...)))
+          (_ #f))))
+
+  (define (make-srfi-n context n)
+    (datum->syntax
+     context
+     (string->symbol
+      (string-append
+       "srfi-"
+       (let ((n (syntax->datum n)))
+         (if (symbol? n)
+             (substring (symbol->string n) 1)
+             (number->string n)))))))
+
   (define (make-custom-interface mod)
     (let ((iface (make-module)))
       (set-module-kind! iface 'custom-interface)
@@ -37,27 +83,13 @@
     (for-each (lambda (mod)
                 (module-for-each f mod))
               (module-and-uses mod)))
-  (define (sym? x) (symbol? (syntax->datum x)))
 
   (syntax-case import-spec (library only except prefix rename srfi)
     ;; (srfi :n ...) -> (srfi srfi-n ...)
     ;; (srfi n ...) -> (srfi srfi-n ...)
     ((library (srfi n rest ... (version ...)))
-     (and (and-map sym? #'(srfi rest ...))
-          (or (and
-               (symbol? (syntax->datum #'n))
-               (let ((str (symbol->string (syntax->datum #'n))))
-                 (and (string-prefix? ":" str)
-                      (and=> (string->number (substring str 1))
-                             exact-integer?))))
-              (exact-integer? (syntax->datum #'n))))
-     (let ((srfi-n (string->symbol
-                    (string-append
-                     "srfi-"
-                     (let ((n (syntax->datum #'n)))
-                       (if (symbol? n)
-                           (substring (symbol->string n) 1)
-                           (number->string n)))))))
+     (srfi-name? #'(srfi n rest ...))
+     (let ((srfi-n (make-srfi-n #'srfi #'n)))
        (resolve-r6rs-interface
         (syntax-case #'(rest ...) ()
           (()
@@ -152,15 +184,58 @@
              (lp (cdr in) (cons (vector to replace? var) out))))))))
     
     ((name name* ... (version ...))
-     (and-map sym? #'(name name* ...))
+     (module-name? #'(name name* ...))
      (resolve-r6rs-interface #'(library (name name* ... (version ...)))))
 
-    ((name name* ...) 
-     (and-map sym? #'(name name* ...))
+    ((name name* ...)
+     (module-name? #'(name name* ...))
      (resolve-r6rs-interface #'(library (name name* ... ()))))))
 
 (define-syntax library
   (lambda (stx)
+    (define (sym? stx)
+      (symbol? (syntax->datum stx)))
+
+    (define (n? stx)
+      (let ((n (syntax->datum stx)))
+        (and (exact-integer? n)
+             (not (negative? n)))))
+
+    (define (colon-n? x)
+      (let ((sym (syntax->datum x)))
+        (and (symbol? sym)
+             (let ((str (symbol->string sym)))
+               (and (string-prefix? ":" str)
+                    (let ((num (string->number (substring str 1))))
+                      (and (exact-integer? num)
+                           (not (negative? num)))))))))
+
+    (define (srfi-name? stx)
+      (syntax-case stx (srfi)
+        ((srfi n rest ...)
+         (and (and-map sym? #'(rest ...))
+              (or (n? #'n)
+                  (colon-n? #'n))))
+        (_ #f)))
+
+    (define (module-name? stx)
+      (or (srfi-name? stx)
+          (syntax-case stx ()
+            ((name name* ...)
+             (and-map sym? #'(name name* ...)))
+            (_ #f))))
+
+    (define (make-srfi-n context n)
+      (datum->syntax
+       context
+       (string->symbol
+        (string-append
+         "srfi-"
+         (let ((n (syntax->datum n)))
+           (if (symbol? n)
+               (substring (symbol->string n) 1)
+               (number->string n)))))))
+
     (define (compute-exports ifaces specs)
       (define (re-export? sym)
         (or-map (lambda (iface) (module-variable iface sym)) ifaces))
@@ -195,23 +270,34 @@
               (else
                (lp #'rest (cons #'id e) r x))))))))
 
-    (syntax-case stx (export import)
+    (syntax-case stx (export import srfi)
       ((_ (name name* ...)
           (export espec ...)
           (import ispec ...)
           body ...)
-       (and-map identifier? #'(name name* ...))
+       (module-name? #'(name name* ...))
        ;; Add () as the version.
        #'(library (name name* ... ())
            (export espec ...)
            (import ispec ...)
            body ...))
 
+      ((_ (srfi n rest ... (version ...))
+          (export espec ...)
+          (import ispec ...)
+          body ...)
+       (srfi-name? #'(srfi n rest ...))
+       (let ((srfi-n (make-srfi-n #'srfi #'n)))
+         #`(library (srfi #,srfi-n rest ... (version ...))
+             (export espec ...)
+             (import ispec ...)
+             body ...)))
+
       ((_ (name name* ... (version ...))
           (export espec ...)
           (import ispec ...)
 	  body ...)
-       (and-map identifier? #'(name name* ...))
+       (module-name? #'(name name* ...))
        (call-with-values
            (lambda ()
              (compute-exports 
-- 
2.30.2


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

* bug#39601: srfi library naming in r7rs
  2021-11-01 18:42   ` Taylan Kammer
@ 2021-11-02 17:53     ` lloda
  2021-11-03  7:37     ` Linus Björnstam
  1 sibling, 0 replies; 6+ messages in thread
From: lloda @ 2021-11-02 17:53 UTC (permalink / raw)
  To: Taylan Kammer; +Cc: 39601-done, pclouds


Applied in a960d7869bc82bb56d5446ece01b8efff9b15fef. Thank you!






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

* bug#39601: srfi library naming in r7rs
  2021-11-01 18:42   ` Taylan Kammer
  2021-11-02 17:53     ` lloda
@ 2021-11-03  7:37     ` Linus Björnstam
  1 sibling, 0 replies; 6+ messages in thread
From: Linus Björnstam @ 2021-11-03  7:37 UTC (permalink / raw)
  To: Taylan Kammer, lloda; +Cc: 39601, pclouds

Well, as someone who has written a lot of macros https://srfi.schemers.org/srfi-206/ is a fantastic utility SRFI. It allows several libraries to define the same aux syntax (say like srfi-26's <>) without having collisions.

That is not strictly what you were looking for if I understand the patch correctly, though.

-- 
  Linus Björnstam

On Mon, 1 Nov 2021, at 19:42, Taylan Kammer wrote:
> On 01.11.2021 18:52, lloda wrote:
>> 
>> Hi Taylan,
>> 
>> Your patch leaks a bunch of identifiers, could you fix that?
>> 
>> thanks
>> 
>> 	Daniel
>> 
>
> Apparently anything defined in boot-9 is implicitly made public in
> the (guile) module, I wasn't aware of that.
>
> Is there a work-around that allows one to define helpers that can
> be used by multiple definitions?
>
> Attached is a naive fix that duplicates a bunch of helpers which is
> not very nice.
>
> -- 
> Taylan
> Attachments:
> * 0001-Improve-support-for-R6-R7-SRFI-module-name-formats.patch





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

end of thread, other threads:[~2021-11-03  7:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-18 12:54 bug#51264: Calling ‘texi-fragment->stexi’ in parallel leads to crashes Ludovic Courtès
2021-10-22 11:56 ` Ludovic Courtès
2021-11-01 17:52 ` bug#39601: srfi library naming in r7rs lloda
2021-11-01 18:42   ` Taylan Kammer
2021-11-02 17:53     ` lloda
2021-11-03  7:37     ` Linus Björnstam

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