[Guile-commits] branch main updated: call-with-values builtin doesn't leave garbage on the stack

[Andy Wingo]

This is an automated email from the git hooks/post-receive script.

wingo pushed a commit to branch main
in repository guile.

The following commit(s) were added to refs/heads/main by this push:
     new 6efc0b815 call-with-values builtin doesn't leave garbage on the stack
6efc0b815 is described below

commit 6efc0b8159f0fc74c0eafec988fe5434fb4d9f51
Author: Andy Wingo <wingo@pobox.com>
AuthorDate: Sun May 7 09:49:00 2023 +0200

    call-with-values builtin doesn't leave garbage on the stack
    
    Fixes https://debbugs.gnu.org/63279.  The issue was that if the producer
    thunk caused a backtrace, pretty-printing the call-with-values frame
    would segfault because there was an unininitialized slot on the stack.
    For functions produced by the compiler this wouldn't be a problem
    because there are stack maps, but primitives require that all slots on a
    pending stack frame be packed (no uninitialized values) and tagged (all
    SCM values, no unboxed values).
    
    * test-suite/tests/error-handling.test: New test.
    * test-suite/Makefile.am: Add new file.
    * libguile/vm.c (define_vm_builtins): Fix call-with-values to have a
    more compact stack.
---
 libguile/vm.c                        | 13 +++++++------
 test-suite/Makefile.am               | 11 ++++++-----
 test-suite/tests/error-handling.test | 37 ++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/libguile/vm.c b/libguile/vm.c
index b565db970..36138f0d5 100644
--- a/libguile/vm.c
+++ b/libguile/vm.c
@@ -1,4 +1,4 @@
-/* Copyright 2001,2009-2015,2017-2020,2022
+/* Copyright 2001,2009-2015,2017-2020,2022-2023
      Free Software Foundation, Inc.
 
    This file is part of Guile.
@@ -451,11 +451,12 @@ define_vm_builtins (void)
 
   const uint32_t call_with_values_code[] = {
     SCM_PACK_OP_24 (assert_nargs_ee, 3),
-    SCM_PACK_OP_24 (alloc_frame, 8),
-    SCM_PACK_OP_12_12 (mov, 0, 6),
-    SCM_PACK_OP_24 (call, 7), SCM_PACK_OP_ARG_8_24 (0, 1),
-    SCM_PACK_OP_24 (long_fmov, 0), SCM_PACK_OP_ARG_8_24 (0, 2),
-    SCM_PACK_OP_12_12 (shuffle_down, 7, 1),
+    SCM_PACK_OP_24 (alloc_frame, 6),
+    SCM_PACK_OP_12_12 (mov, 0, 4),
+    SCM_PACK_OP_12_12 (mov, 4, 3),
+    SCM_PACK_OP_24 (call, 5), SCM_PACK_OP_ARG_8_24 (0, 1),
+    SCM_PACK_OP_24 (long_fmov, 0), SCM_PACK_OP_ARG_8_24 (0, 1),
+    SCM_PACK_OP_12_12 (shuffle_down, 5, 1),
     SCM_PACK_OP_24 (tail_call, 0)
   };
 
diff --git a/test-suite/Makefile.am b/test-suite/Makefile.am
index 16fa2e952..81e63bce2 100644
--- a/test-suite/Makefile.am
+++ b/test-suite/Makefile.am
@@ -1,6 +1,6 @@
 ## Process this file with automake to produce Makefile.in.
 ##
-## Copyright 2001-2020 Software Foundation, Inc.
+## Copyright 2001-2020, 2023 Software Foundation, Inc.
 ##
 ## This file is part of GUILE.
 ##
@@ -43,14 +43,15 @@ SCM_TESTS = tests/00-initial-env.test               \
            tests/cross-compilation.test        \
            tests/curried-definitions.test      \
            tests/dwarf.test                    \
-           tests/encoding-escapes.test         \
-           tests/encoding-iso88591.test        \
-           tests/encoding-iso88597.test        \
-           tests/encoding-utf8.test            \
            tests/ecmascript.test               \
            tests/elisp.test                    \
            tests/elisp-compiler.test           \
            tests/elisp-reader.test             \
+           tests/encoding-escapes.test         \
+           tests/encoding-iso88591.test        \
+           tests/encoding-iso88597.test        \
+           tests/encoding-utf8.test            \
+           tests/error-handling.test           \
            tests/eval.test                     \
            tests/eval-string.test              \
            tests/exceptions.test               \
diff --git a/test-suite/tests/error-handling.test 
b/test-suite/tests/error-handling.test
new file mode 100644
index 000000000..54c8df6fd
--- /dev/null
+++ b/test-suite/tests/error-handling.test
@@ -0,0 +1,37 @@
+;;; Test for call-with-error-handling.  -*- scheme -*-
+;;; Copyright (C) 2023
+;;;   Free Software Foundation, Inc.
+;;;
+;;; This library is free software: you can redistribute it and/or modify
+;;; it under the terms of the GNU Lesser General Public License as
+;;; published by the Free Software Foundation, either version 3 of the
+;;; License, or (at your option) any later version.
+;;;
+;;; This library is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;;; Lesser General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU Lesser General Public
+;;; License along with this program.  If not, see
+;;; <http://www.gnu.org/licenses/>.
+
+(define-module (test-suite test-error-handling)
+  #:use-module (test-suite lib)
+  #:use-module (system base compile)
+  #:use-module (system repl error-handling))
+
+(with-test-prefix "call-with-values"
+  (pass-if "backtrace with builtin"
+    (let* ((thunk (compile '(lambda ()
+                              (symbol?
+                               (call-with-values (lambda () (error 'oh-no))
+                                 list)))))
+           (str (with-output-to-string
+                  (lambda ()
+                    (call-with-error-handling thunk #:on-error 'backtrace))))
+           (lines (string-split str #\newline)))
+      ;; For https://debbugs.gnu.org/63279, we just want to ensure that
+      ;; the backtrace completes.  The check is just that the backtrace
+      ;; has at least a couple frames.
+      (>= (length lines) 4))))