Subject: Re: bug#10520: [platform-testers] Guile 2.0.4 release imminent

From: [email-masked] (Ludovic Courtès)
Date: Sat, 21 Jan 2012 13:10:29 +0100

Hi Bruno,

Bruno Haible <[email-masked]> skribis:

> 8) On Linux/hppa and Linux/SPARC32 and Linux/IA-64, build failure in the
> libguile directory:
>
>   SNARF  socket.doc
>   SNARF  regex-posix.doc
>   GEN    guile-procedures.texi
> /bin/sh: line 1:  1022 Broken pipe             cat alist.doc arbiters.doc array-handle.doc array-map.doc arrays.doc async.doc backtrace.doc boolean.doc bitvectors.doc bytevectors.doc chars.doc control.doc continuations.doc debug.doc deprecated.doc deprecation.doc dynl.doc dynwind.doc eq.doc error.doc eval.doc evalext.doc expand.doc extensions.doc feature.doc filesys.doc fluids.doc foreign.doc fports.doc gc-malloc.doc gc.doc gettext.doc generalized-arrays.doc generalized-vectors.doc goops.doc gsubr.doc guardians.doc hash.doc hashtab.doc hooks.doc i18n.doc init.doc ioext.doc keywords.doc list.doc load.doc macros.doc mallocs.doc memoize.doc modules.doc numbers.doc objprop.doc options.doc pairs.doc ports.doc print.doc procprop.doc procs.doc promises.doc r6rs-ports.doc random.doc rdelim.doc read.doc root.doc rw.doc scmsigs.doc script.doc simpos.doc smob.doc sort.doc srcprop.doc srfi-1.doc srfi-4.doc srfi-13.doc srfi-14.doc srfi-60.doc stackchk.doc stacks.doc stime.doc strings.doc strorder.doc strports.doc struct.doc symbols.doc threads.doc throw.doc trees.doc uniform.doc values.doc variable.doc vectors.doc version.doc vports.doc weaks.doc dynl.doc posix.doc net_db.doc socket.doc regex-posix.doc
>       1023 Segmentation fault      | GUILE_AUTO_COMPILE=0 ../meta/uninstalled-env guild snarf-check-and-output-texi > guile-procedures.texi
> make[3]: *** [guile-procedures.texi] Error 1

I bisected this and found this harmless-looking patch to be the culprit:

  http://git.savannah.gnu.org/cgit/guile.git/commit/?id=3e54fdfc217969abb50a46ec9c9c5c02a2c7d369

The segfault would occur in vm-i-system.c:1281, in the ‘CACHE_PROGRAM’
macro, while trying to access ‘SCM_PROGRAM_DATA’, because of a bogus
value for ‘program’, stemming from a bogus ‘fp’.

After a long and tiring period of investigation, I found that reverting
just the definitions SCM_FRAME_DYNAMIC_LINK and
SCM_FRAME_SET_DYNAMIC_LINK would solve the problem.

Eventually, I found that, keeping frames.h unchanged, adding a compiler
barrier after each ‘fp’ assignment would solve the problem:

diff --git a/libguile/vm-i-system.c b/libguile/vm-i-system.c
index 474fe78..a47ac38 100644
--- a/libguile/vm-i-system.c
+++ b/libguile/vm-i-system.c
@@ -55,6 +55,7 @@ VM_DEFINE_INSTRUCTION (1, halt, "halt", 0, 0, 0)
        stack */
     ip = SCM_FRAME_RETURN_ADDRESS (fp);
     fp = SCM_FRAME_DYNAMIC_LINK (fp);
+    asm volatile ("");
     NULLSTACK (old_sp - sp);
   }
   
@@ -1267,6 +1268,7 @@ VM_DEFINE_INSTRUCTION (67, return, "return", 0, 1, 1)
     sp = SCM_FRAME_LOWER_ADDRESS (fp);
     ip = SCM_FRAME_RETURN_ADDRESS (fp);
     fp = SCM_FRAME_DYNAMIC_LINK (fp);
+    asm volatile ("");
 
 #ifdef VM_ENABLE_STACK_NULLING
     NULLSTACK (old_sp - sp);
@@ -1302,6 +1304,7 @@ VM_DEFINE_INSTRUCTION (68, return_values, "return/values", 1, -1, -1)
       sp = SCM_FRAME_LOWER_ADDRESS (fp) - 1;
       ip = SCM_FRAME_MV_RETURN_ADDRESS (fp);
       fp = SCM_FRAME_DYNAMIC_LINK (fp);
+      asm volatile ("");
         
       /* Push return values, and the number of values */
       for (i = 0; i < nvalues; i++)
@@ -1322,6 +1325,7 @@ VM_DEFINE_INSTRUCTION (68, return_values, "return/values", 1, -1, -1)
       sp = SCM_FRAME_LOWER_ADDRESS (fp) - 1;
       ip = SCM_FRAME_RETURN_ADDRESS (fp);
       fp = SCM_FRAME_DYNAMIC_LINK (fp);
+      asm volatile ("");
         
       /* Push first value */
       *++sp = vals[1];

This is not entirely satisfactory either, because I don’t see why this
barrier would be needed.  It could be a GCC bug, as this file is a real
stress test.  Ideas?

Unless someone comes up with a better idea, I’ll commit a portable
version of the above patch.

(Tested on sparc64-unknown-linux-gnu and hppa2.0-unknown-linux-gnu.)

Thanks,
Ludo’.