[PATCH] test for smob mark segv

[PATCH] test for smob mark segv

[Mike Gran]

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

Hello-

What would you think about applying the following patch?

It is a standalone test that, in effect, checks to see if
BDW-GC is running marking in its own non-Guile thread.  If
BDW does have parallel marking enabled, this test will SEGV.
If it doesn't have parallel marking enabled, this test will pass.

If you run it with GC_MARKERS=1 in the env it will always
pass even if BDW-GC was compiled with parallel marking enabled.

It could be a precursor to another patch that forces GC_MARKERS=1
either in libguile or in guile.c.

Thanks,

Mike Gran

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-standalone-test-for-smob-marking.patch --]
[-- Type: text/x-patch; name="0001-Add-standalone-test-for-smob-marking.patch", Size: 4190 bytes --]

From f3adf180edbf6ebe642cf4c1ad8f029ce0876d0d Mon Sep 17 00:00:00 2001
From: Mike Gran <spk121@yahoo.com>
Date: Tue, 26 Feb 2013 08:27:22 -0800
Subject: [PATCH] Add standalone test for smob marking

* test-suite/standalone/Makefile.am: add test-smob-mark
* test-suite/standalone/test-smob-mark.c: new test
* test-suite/standalone/.gitignore: ignore test-smob-mark
---
 test-suite/standalone/.gitignore       |   2 +
 test-suite/standalone/Makefile.am      |   6 ++
 test-suite/standalone/test-smob-mark.c | 118 +++++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+)
 create mode 100644 test-suite/standalone/test-smob-mark.c

diff --git a/test-suite/standalone/.gitignore b/test-suite/standalone/.gitignore
index 794146e..db0794b 100644
--- a/test-suite/standalone/.gitignore
+++ b/test-suite/standalone/.gitignore
@@ -13,3 +13,5 @@
 /test-scm-take-u8vector
 /test-loose-ends
 /test-srfi-1
+/test-smob-mark
+
diff --git a/test-suite/standalone/Makefile.am b/test-suite/standalone/Makefile.am
index be5d913..8190e59 100644
--- a/test-suite/standalone/Makefile.am
+++ b/test-suite/standalone/Makefile.am
@@ -251,4 +251,10 @@ EXTRA_DIST += test-with-guile-module.c test-scm-with-guile.c
 
 endif
 
+test_smob_mark_SOURCES = test-smob-mark.c
+test_smob_mark_CFLAGS = ${test_cflags}
+test_smob_mark_LDADD = $(LIBGUILE_LDADD)
+check_PROGRAMS += test-smob-mark
+TESTS += test-smob-mark
+
 EXTRA_DIST += ${check_SCRIPTS}
diff --git a/test-suite/standalone/test-smob-mark.c b/test-suite/standalone/test-smob-mark.c
new file mode 100644
index 0000000..d97ef25
--- /dev/null
+++ b/test-suite/standalone/test-smob-mark.c
@@ -0,0 +1,118 @@
+/* Copyright (C) 2013 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 library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#if HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <assert.h>
+#include <libguile.h>
+
+typedef struct x_tag
+{
+  SCM scm_value;
+  int c_value;
+} x_t;
+
+static scm_t_bits x_tag;
+SCM make_x (void);
+SCM mark_x (SCM x);
+int print_x (SCM x, SCM port, scm_print_state * pstate);
+size_t free_x (SCM x);
+void init_smob_type (void);
+void test_scm_smob_mark (void);
+
+SCM
+make_x ()
+{
+  static int i = 0;
+  SCM s_x;
+  x_t *c_x;
+
+  i++;
+  c_x = (x_t *) scm_gc_malloc (sizeof (x_t), "x");
+  c_x->scm_value = scm_from_int (i);
+  c_x->c_value = i;
+  SCM_NEWSMOB (s_x, x_tag, c_x);
+  return s_x;
+}
+
+SCM
+mark_x (SCM x)
+{
+  x_t *c_x;
+  c_x = (x_t *) SCM_SMOB_DATA (x);
+  scm_gc_mark (c_x->scm_value);
+  return SCM_BOOL_F;
+}
+
+size_t
+free_x (SCM x)
+{
+  x_t *c_x;
+  c_x = (x_t *) SCM_SMOB_DATA (x);
+  scm_gc_free (c_x, sizeof (x_t), "x");
+  c_x = NULL;
+  return 0;
+}
+
+int
+print_x (SCM x, SCM port, scm_print_state * pstate SCM_UNUSED)
+{
+  x_t *c_x = (x_t *) SCM_SMOB_DATA (x);
+  scm_puts ("#<x ", port);
+  if (c_x == (x_t *) NULL)
+    scm_puts ("(freed)", port);
+  else
+    scm_write (c_x->scm_value, port);
+  scm_puts (">", port);
+
+  return 1;
+}
+
+void
+test_scm_smob_mark ()
+{
+  int i;
+  for (i = 0; i < 10000; i++)
+    make_x ();
+  scm_gc ();
+}
+
+void
+init_smob_type ()
+{
+  x_tag = scm_make_smob_type ("x", sizeof (x_t));
+  scm_set_smob_free (x_tag, free_x);
+  scm_set_smob_print (x_tag, print_x);
+  scm_set_smob_mark (x_tag, mark_x);
+}
+
+static void
+tests (void *data, int argc, char **argv)
+{
+  init_smob_type ();
+  test_scm_smob_mark ();
+}
+
+int
+main (int argc, char *argv[])
+{
+  scm_boot_guile (argc, argv, tests, NULL);
+  return 0;
+}
-- 
1.7.11.7

Re: [PATCH] test for smob mark segv

[Mike Gran]

> From: Mike Gran <spk121@yahoo.com>

> What would you think about applying the following patch?
> 
> It is a standalone test that, in effect, checks to see if
> BDW-GC is running marking in its own non-Guile thread.  If
> BDW does have parallel marking enabled, this test will SEGV.
> If it doesn't have parallel marking enabled, this test will pass.

Hearing no objection, and since it is test only, I guess
I'll push it.

OK?

-Mike

Re: [PATCH] test for smob mark segv

[Ludovic Courtès]

Hi Mike,

Mike Gran <spk121@yahoo.com> skribis:

> It is a standalone test that, in effect, checks to see if
> BDW-GC is running marking in its own non-Guile thread.  If
> BDW does have parallel marking enabled, this test will SEGV.
> If it doesn't have parallel marking enabled, this test will pass.

Good.

FWIW, I’ve been meaning to fix the problem for good.  I hope to post a
patch within the next few days.

> From f3adf180edbf6ebe642cf4c1ad8f029ce0876d0d Mon Sep 17 00:00:00 2001
> From: Mike Gran <spk121@yahoo.com>
> Date: Tue, 26 Feb 2013 08:27:22 -0800
> Subject: [PATCH] Add standalone test for smob marking
>
> * test-suite/standalone/Makefile.am: add test-smob-mark

Specify the makefile variable name in parentheses.

> * test-suite/standalone/test-smob-mark.c: new test
> * test-suite/standalone/.gitignore: ignore test-smob-mark

No need for .gitignore here.

> +typedef struct x_tag
> +{
> +  SCM scm_value;
> +  int c_value;
> +} x_t;

Separate typedef and struct.

> +static scm_t_bits x_tag;
> +SCM make_x (void);
> +SCM mark_x (SCM x);
> +int print_x (SCM x, SCM port, scm_print_state * pstate);
> +size_t free_x (SCM x);
> +void init_smob_type (void);
> +void test_scm_smob_mark (void);

Make them all static.

> +SCM
> +mark_x (SCM x)
> +{
> +  x_t *c_x;
> +  c_x = (x_t *) SCM_SMOB_DATA (x);
> +  scm_gc_mark (c_x->scm_value);
> +  return SCM_BOOL_F;
> +}

I think it’d be good to increment a global counter to make sure the mark
procedure is called at all.

Thanks!

Ludo’.