bug#60799: Bogus 'Error while printing exception' message when raising srfi-35 exception

[Maxim Cournoyer]

Hello Ricardo,

Ricardo Wurmus  writes:

> Hi Maxim,
>
>> When raising a srfi-35 defined exception type like in the following, a
>> generic (and unhelpful) "Error while printing exception" message is
>> shown, with not even the exception type mentioned:
>>
>> (use-modules (srfi srfi-35))
>>
>> (define-condition-type &platform-not-found-error &error
>>   platform-not-found-error?)
>>
>> (raise-exception &platform-not-found-error)
>>
>>
>> Produces:
>>
>> Backtrace:
>> In ice-9/boot-9.scm:
>>   1752:10  5 (with-exception-handler _ _ #:unwind? _ # _)
>> In unknown file:
>>4 (apply-smob/0 #)
>> In ice-9/boot-9.scm:
>> 724:2  3 (call-with-prompt _ _ #)
>> In ice-9/eval.scm:
>> 619:8  2 (_ #(#(#)))
>> In ice-9/boot-9.scm:
>>2836:4  1 (save-module-excursion _)
>>   4388:12  0 (_)
>>
>> ice-9/boot-9.scm:4388:12: Error while printing exception.
>>
>> This is probably not by design, right?
>
> Perhaps not, but conditions are expected to be raised with “raise”:
>
> (use-modules (srfi srfi-34) (srfi srfi-35))
>
> (define-condition-type &platform-not-found-error &error
>   platform-not-found-error?)
>
> (raise (condition (&platform-not-found-error)))
> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
> ERROR:
>   1. &platform-not-found-error

Thanks for pointing that.  The above with 'raise' doesn't produce the
same output for my Guile 3.0.8:

--8<---cut here---start->8---
(use-modules (srfi srfi-35))

(define-condition-type &platform-not-found-error &error
  platform-not-found-error?)

(raise (condition (&platform-not-found-error)))
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Wrong type (expecting exact integer): #<&platform-not-found-error>
--8<---cut here---end--->8---

Using 'raise-exception' instead of 'raise' fixes it for me:

--8<---cut here---start->8---
(raise-exception (condition (&platform-not-found-error)))
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
ERROR:
  1. &platform-not-found-error

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
--8<---cut here---end--->8---

So my initial issue was attempting to raise on a type rather than an
object (which works both ways in other languages such as Python).

If I also import srfi-34, then it works as expected:

--8<---cut here---start->8---
(use-modules (srfi srfi-34) (srfi srfi-35))

(define-condition-type &platform-not-found-error &error
  platform-not-found-error?)

(raise (condition (&platform-not-found-error)))
--8<---cut here---end--->8---

So my original confusing was that there exists a 'raise' procedure in
Guile, which has nothing to do with exceptions (it's used to send a
signal to the current process).

What I'll take from this is to use exclusively 'raise-exception', which
is not subject to the above srfi-34 vs builtin raise confusion.

Closing, thanks for helping me untangle things!

-- 
Thanks,
Maxim

bug#27782: patch for mmap and friends

[Maxime Devos]

On 14-01-2023 01:49, Matt Wette wrote:

Please consider this patch for adding mmap(), munmap() and msync()
  to libguile/filesys.c.  Included is update for posix.texi and test 
file mman.test.

Once included, feature 'mman should be #t.

Matt






+  if (SCM_UNBNDP (fd))
+c_fd = -1;
+  else
+c_fd = scm_to_int (fd);


Port objects should be accepted too, as previously asked on 
.
As implied by later comments, using a raw fd causes problems with 
'move->fdes'.  For the remaining response, I'll assume that the function 
accepts ports as well.


 (---)

After this code, the port 'fd' becomes unreferenced by this function.


+  if (SCM_UNBNDP (offset))
+c_offset = 0;
+  else
+c_offset = scm_to_off_t (offset);
+
+  if ((c_addr == NULL) && (c_flags & MAP_FIXED))
+scm_misc_error ("mmap", "cannot have NULL addr w/ MAP_FIXED", SCM_EOL);


Hence, if the GC is run here, its possible for fd to be automatically 
closed here.



+  SCM_SYSCALL (c_mem = mmap(c_addr, c_len, c_prot, c_flags, c_fd, c_offset));


As such, C 'mmap' could be called on a closed file descriptor even even 
if the argument to Scheme 'mmap' was an open port, which isn't going to 
work.


(While it is recommended for Scheme code to keep a reference to the port 
to manually close afterwards, to free resources faster than waiting for 
GC, it is not actually required.)


Worse, if the port is closed (by GC), in the mean time another thread 
may have opened a new port, whose file descriptor coincides with c_fd.


To avoid this problem, you can add

  scm_remember_upto_here_1 (fd);

after the SCM_SYSCALL.

Even then, a problem remains -- a concurrent thread might be using 
'move->fdes' to 'move' the file descriptor, hence invalidating c_fd.
Functions like 'scm_seek' handle this with 'scm_dynwind_acquire_port' 
(IIUC, it takes a mutex to delay concurrent 'move->fdes' until finished).


IIUC, the solution then looks like (ignoring wrapping) (the lack of 
scm_remember_upto_here_1 is intentional):


scm_dynwind_begin (0);
scm_dynwind_acquire_port (fd); // (accepts both ports and numbers, IIUC)
// needs to be after scm_dynwind_acquire_port
if (SCM_UNBNDP (fd))
  c_fd = -1;
else
  c_fd = scm_to_int (fd);

SCM_SYSCALL (c_mem = mmap(c_addr, c_len, c_prot, c_flags, c_fd, c_offset));
if (c_mem == MAP_FAILED)
scm_syserror ("mmap");
scm_dynwind_end ();

(I'm not really familiar with scm_dynwind_begin & friends, I'm mostly 
copy-pasting from libguile/ports.c here.)



+  if (c_mem == MAP_FAILED)
+scm_syserror ("mmap");  /* errno set */
+
+  pointer = scm_cell (scm_tc7_pointer, (scm_t_bits) c_mem);
+  bvec = scm_c_take_typed_bytevector((signed char *) c_mem + c_offset, c_len,
+SCM_ARRAY_ELEMENT_TYPE_VU8, pointer);



+  assert(sizeof(void*) <= sizeof(size_t));


IIRC there is a C trick involving fields, arrays and types to check this 
at compile-time instead.  Maybe:


struct whatever {
   /* if availability of zero-length arrays can be assumed */
   int foo[sizeof(size_t) - sizeof(void*)];
   /* alternatively, a weaker but portable check */
   int foo[sizeof(size_t) - sizeof(void*) + 1];
};

Greetings,
Maxime.


OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

bug#27782: patch for mmap and friends

[Matt Wette]

On 1/14/23 2:42 PM, Maxime Devos wrote:

    {
  /* Use the fd of the port under clobber protection from
 concurrency. As scm_dynwind_acquire_port assumes that
 FILE is a port, check that first. */
  SCM_VALIDATE_PORT (SCM_ARG5, file);
  scm_dynwind_acquire_port (file);
  c_fd = scm_fileno (file);
    }


Thanks.  I'll try this, modulo update to  scm_to_int (scm_fileno (file)).

Matt