Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 36 additions & 3 deletions .github/workflows/linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,15 @@ concurrency:
group: ${{ github.head_ref || github.sha }}-${{ github.workflow }}
cancel-in-progress: true
jobs:
setup:
name: Setup
libgcroots:
name: libgcroots
strategy:
fail-fast: false
matrix:
libgcroots:
- no
- installed
- tiny-subst
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
Expand All @@ -25,9 +32,35 @@ jobs:
- name: Generate configure
run: |
./autogen.sh
- name: "BDWGC: Checkout"
if: matrix.libgcroots == 'installed'
uses: actions/checkout@v6
with:
path: bdwgc
repository: bdwgc/bdwgc

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workflow checks out and builds BDWGC from the repository default branch without pinning a tag/commit (repository: bdwgc/bdwgc with no ref). This can make CI non-reproducible (upstream changes can break the build or change the reported pkg-config version relative to the bdw-gc >= 8.3.0 requirement). Consider pinning to a known-good release tag/commit (>= 8.3.0) or installing a packaged BDWGC version to keep CI deterministic.

Suggested change
repository: bdwgc/bdwgc
repository: bdwgc/bdwgc
ref: v8.3.0

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no such tag (v8.3.0), use commit sha instead, e.g. 946696b9b16580fcece3bc4d640f35afc139426d

- name: "BDWGC: Configure"
if: matrix.libgcroots == 'installed'
run: |
cmake \
-S bdwgc \
-B bdwgc.build \
-G Ninja \
-DCMAKE_INSTALL_PREFIX=/usr
- name: "BDWGC: Build"
if: matrix.libgcroots == 'installed'
run: |
cmake --build bdwgc.build
- name: "BDWGC: Install"
if: matrix.libgcroots == 'installed'
run: |
sudo cmake --install bdwgc.build
- name: Configure
run: |
./configure --enable-maintainer-mode
./configure \
--enable-conf=uim \
--enable-maintainer-mode \
--prefix=/usr \
Comment on lines +60 to +62

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This job now configures with --enable-conf=uim, --prefix=/usr, and a --with-libgcroots matrix, whereas previously it only ran ./configure --enable-maintainer-mode. If the goal of this workflow is specifically to validate the GC backend selection, these extra flags change what configuration is being exercised and may reduce coverage of the default build. Consider either keeping the default configure flags and adding a separate job for the uim config, or documenting why the uim config is required here.

Suggested change
--enable-conf=uim \
--enable-maintainer-mode \
--prefix=/usr \
--enable-maintainer-mode \

Copilot uses AI. Check for mistakes.
--with-libgcroots=${{ matrix.libgcroots }}
- name: Build
run: |
make -j$(nproc)
Expand Down
26 changes: 17 additions & 9 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,10 @@ AM_CONDITIONAL(WITH_MASTER_PKG, [test "x$with_master_pkg" != xno])

AC_ARG_WITH(libgcroots,
AS_HELP_STRING([--with-libgcroots@<:@=WHICH@:>@],
[use alternative libgcroots instead of
package-bundled one.
WHICH=(installed|tiny-subst)
@<:@default=installed@:>@]))
[use system BDWGC or alternative libgcroots
instead of package-bundled libgcroots.
WHICH=(no|installed|tiny-subst)
@<:@default=no@:>@]))

case "${with_libgcroots:=no}" in
no)
Expand All @@ -287,11 +287,16 @@ case "${with_libgcroots:=no}" in
GCROOTS_REQ=gcroots
;;
yes|installed)
PKG_CHECK_MODULES(GCROOTS, [gcroots >= 0.2.3],
[],
[AC_MSG_ERROR([installed libgcroots is not found.])])
use_libgcroots="installed one"
GCROOTS_REQ=gcroots
PKG_CHECK_MODULES(
BDWGC, [bdw-gc >= 8.3.0],
Comment thread
ivmai marked this conversation as resolved.
[BDWGC_REQ=bdw-gc
AC_DEFINE(SCM_WITH_BDWGC, 1, [Define to 1 to use BDWGC.])
use_libgcroots="installed BDWGC"],
[PKG_CHECK_MODULES(
GCROOTS, [gcroots >= 0.2.3],
[GCROOTS_REQ=gcroots
use_libgcroots="installed one"],
[AC_MSG_ERROR([neither BDWGC nor libgcroots was found.])])])
Comment on lines 268 to +299

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description says the build should use system BDWGC when it is found, but configure currently only probes for BDWGC in the --with-libgcroots=yes|installed branch. With the current logic, the default (with_libgcroots=no) will always build/use the bundled libgcroots even if a suitable system BDWGC is available. If the intent is auto-preference for system BDWGC, consider probing BDWGC in the default path (and only falling back to bundled libgcroots if not found), or clarify the intended behavior in the option help/PR description.

Copilot uses AI. Check for mistakes.
;;
tiny-subst)
# use src/gcroots/gcroots.[hc]
Expand All @@ -304,6 +309,9 @@ case "${with_libgcroots:=no}" in
;;
esac

AC_SUBST(BDWGC_REQ)
AC_SUBST(BDWGC_LIBS)
AC_SUBST(BDWGC_CFLAGS)
AC_SUBST(GCROOTS_REQ)
AC_SUBST(GCROOTS_LIBS)
AC_SUBST(GCROOTS_CFLAGS)
Expand Down
2 changes: 1 addition & 1 deletion sigscheme.pc.in
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,6 @@ sscm_debug=@use_debug@
Name: @PACKAGE_NAME@
Description: Scheme interpreter for embedded use
Version: @VERSION@
Requires: @GCROOTS_REQ@
Requires.private: @BDWGC_REQ@ @GCROOTS_REQ@
Libs: -L${libdir} -lsscm
Cflags: -I${includedir}
10 changes: 5 additions & 5 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -369,20 +369,20 @@ endif

if USE_LIBSSCM
libsscm_la_SOURCES = $(libsscm_sources)
libsscm_la_LIBADD = $(LIBOBJS) @GCROOTS_LIBS@
libsscm_la_LIBADD = $(LIBOBJS) @BDWGC_LIBS@ @GCROOTS_LIBS@
libsscm_la_LDFLAGS = -version-info $(libsscm_version)
# To take precedence over installed gcroots.h by the bundled
# libgcroots/include/gcroots.h, @GCROOTS_CFLAGS@ must be placed here.
libsscm_la_CPPFLAGS = -I$(top_builddir)/include @GCROOTS_CFLAGS@ \
-DSCM_COMPILING_LIBSSCM=1
libsscm_la_CPPFLAGS = -I$(top_builddir)/include @BDWGC_CFLAGS@ \
@GCROOTS_CFLAGS@ -DSCM_COMPILING_LIBSSCM=1
libsscm_la_CFLAGS =
endif

if USE_SHELL
bin_PROGRAMS = sscm
sscm_SOURCES = main.c
sscm_LDADD = libsscm.la @GCROOTS_LIBS@
sscm_CPPFLAGS = -I$(top_builddir)/include @GCROOTS_CFLAGS@
sscm_LDADD = libsscm.la @BDWGC_LIBS@ @GCROOTS_LIBS@
sscm_CPPFLAGS = -I$(top_builddir)/include @BDWGC_CFLAGS@ @GCROOTS_CFLAGS@
sscm_CFLAGS =
endif

Expand Down
5 changes: 0 additions & 5 deletions src/sigscheme.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@
#include <stdio.h>
#include <stdarg.h>

#include <gcroots.h>
#if (GCROOTS_API_REVISION < 1)
#error "libgcroots that has API revision >= 1 is required"
#endif

#include "scmint.h"
#include "global.h"
#if SCM_USE_MULTIBYTE_CHAR
Expand Down
153 changes: 151 additions & 2 deletions src/storage-gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@
#include "sigscheme.h"
#include "sigschemeinternal.h"

#ifdef SCM_WITH_BDWGC
#include <gc/gc.h>
#include <gc/gc_mark.h>
Comment thread
ivmai marked this conversation as resolved.
#else
#include <gcroots.h>
#endif

/*=======================================
File Local Macro Definitions
=======================================*/
Expand All @@ -83,6 +90,22 @@
=======================================*/
typedef ScmCell *ScmObjHeap;

#ifdef SCM_WITH_BDWGC
typedef struct BDWGCContext_ {
struct GC_stack_base stack_base;
scm_bool is_protected;
} BDWGCContext;

typedef struct BDWGCReadyStackData_ {
ScmGCGateFunc func;
void *arg;
} BDWGCReadyStackData;

typedef struct BDWGCFindObjectData_ {
void *findee;
} BDWGCFindObjectData;
#endif

/*=======================================
Variable Definitions
=======================================*/
Expand All @@ -101,7 +124,11 @@ static ScmObj l_freelist;

static ScmObj **l_protected_vars;
static size_t l_protected_vars_size, l_n_empty_protected_vars;
#ifdef SCM_WITH_BDWGC
static BDWGCContext *l_bdwgc_ctx;
#else
static GCROOTS_context *l_gcroots_ctx;
#endif
#if SCM_DEBUG
static size_t l_gcing;
static scm_bool l_allocating;
Expand All @@ -120,7 +147,11 @@ SCM_GLOBAL_VARS_END(static_gc);
#define l_protected_vars_size SCM_GLOBAL_VAR(static_gc, l_protected_vars_size)
#define l_n_empty_protected_vars \
SCM_GLOBAL_VAR(static_gc, l_n_empty_protected_vars)
#ifdef SCM_WITH_BDWGC
#define l_bdwgc_ctx SCM_GLOBAL_VAR(static_gc, l_bdwgc_ctx)
#else
#define l_gcroots_ctx SCM_GLOBAL_VAR(static_gc, l_gcroots_ctx)
#endif
#if SCM_DEBUG
#define l_gcing SCM_GLOBAL_VAR(static_gc, l_gcing)
#define l_allocating SCM_GLOBAL_VAR(static_gc, l_allocating)
Expand Down Expand Up @@ -160,6 +191,17 @@ static void finalize_protected_var(void);
static void scm_ensure_proper_freelist(ScmObj flst); /* FIXME */
#endif

/* BDWGC Related Functions */
#ifdef SCM_WITH_BDWGC
static void *GC_CALLBACK bdwgc_ready_stack(struct GC_stack_base *base,
void *data);
static void GC_CALLBACK bdwgc_mark(void **start, void **end,
void *data, unsigned hint);
static void GC_CALLBACK bdwgc_find_object(void **start, void **end,
void *data, unsigned hint);
static int bdwgc_context_is_protected(BDWGCContext *ctx, void *obj);
#endif

/*=======================================
Function Definitions
=======================================*/
Expand Down Expand Up @@ -188,9 +230,14 @@ scm_init_gc(const ScmStorageConf *conf)
l_allocating = scm_false;
#endif

#ifdef SCM_WITH_BDWGC
l_bdwgc_ctx = scm_malloc(sizeof(BDWGCContext));
l_bdwgc_ctx->is_protected = scm_false;
#else
l_gcroots_ctx = GCROOTS_init(scm_malloc,
(GCROOTS_mark_proc)gc_mark_locations,
scm_false);
#endif

initialize_heap(conf);
}
Expand All @@ -201,8 +248,14 @@ scm_fin_gc(void)
finalize_heap();
finalize_protected_var();

#ifdef SCM_WITH_BDWGC
free(l_bdwgc_ctx);
l_bdwgc_ctx = NULL;
#else
GCROOTS_fin(l_gcroots_ctx);
free(l_gcroots_ctx);
l_gcroots_ctx = NULL;
#endif

SCM_GLOBAL_VARS_FIN(static_gc);
}
Expand Down Expand Up @@ -339,7 +392,13 @@ scm_gc_protectedp(ScmObj obj)
#else
SCM_CONSTANTP(obj)
#endif
|| GCROOTS_is_protected(l_gcroots_ctx, (void *)obj))
||
#ifdef SCM_WITH_BDWGC
bdwgc_context_is_protected(l_bdwgc_ctx, (void *)obj)
#else
GCROOTS_is_protected(l_gcroots_ctx, (void *)obj)
#endif
)
return scm_true;

/* referred from static variables */
Expand Down Expand Up @@ -373,13 +432,32 @@ scm_gc_protectedp(ScmObj obj)
SCM_EXPORT void *
scm_call_with_gc_ready_stack(ScmGCGateFunc func, void *arg)
{
#ifdef SCM_WITH_BDWGC
assert(l_bdwgc_ctx);
assert(func);

if (l_bdwgc_ctx->is_protected) {
return func(arg);
} else {
BDWGCReadyStackData data;
data.func = func;
data.arg = arg;
return GC_call_with_stack_base(bdwgc_ready_stack, &data);
}
#else
return GCROOTS_call_with_gc_ready_stack(l_gcroots_ctx, func, arg);
#endif
}

SCM_EXPORT scm_bool
scm_gc_protected_contextp(void)
{
return GCROOTS_is_protected_context(l_gcroots_ctx);
#ifdef SCM_WITH_BDWGC
assert(l_bdwgc_ctx);
return l_bdwgc_ctx->is_protected;
#else
return GCROOTS_is_protected_context(l_gcroots_ctx);
#endif
}

/*===========================================================================
Expand Down Expand Up @@ -769,7 +847,17 @@ gc_mark(void)

/* Mark stack and all machine-dependent contexts such as registers,
* register windows (SPARC), register stack backing store (IA-64) etc. */
#ifdef SCM_WITH_BDWGC
assert(l_bdwgc_ctx);
if (l_bdwgc_ctx->is_protected) {
GC_custom_push_regs_and_stack(bdwgc_mark,
NULL,
&(l_bdwgc_ctx->stack_base),
NULL);
}
#else
GCROOTS_mark(l_gcroots_ctx);
#endif

gc_mark_global_vars();

Expand Down Expand Up @@ -924,3 +1012,64 @@ gc_sweep(void)

return sum_collected;
}

#ifdef SCM_WITH_BDWGC
static void *GC_CALLBACK
bdwgc_ready_stack(struct GC_stack_base *base,
void *data)
{
void *result;
BDWGCReadyStackData *ready_stack_data = (BDWGCReadyStackData *)data;

memcpy(&(l_bdwgc_ctx->stack_base), base, sizeof(*base));
l_bdwgc_ctx->is_protected = scm_true;
result = (*ready_stack_data->func)(ready_stack_data->arg);
l_bdwgc_ctx->is_protected = scm_false;
return result;
}

static void GC_CALLBACK
bdwgc_mark(void **start, void **end, void *data, unsigned hint)
{

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bdwgc_mark doesn’t use the data or hint parameters. With -Wall this can produce unused-parameter warnings; consider explicitly marking them unused (e.g., via (void)data; (void)hint;) or annotating the parameters if you want warning-clean builds.

Suggested change
{
{
(void)data;
(void)hint;

Copilot uses AI. Check for mistakes.
gc_mark_locations((ScmObj *)start, (ScmObj *)end, scm_false, scm_false);
}

static void GC_CALLBACK
bdwgc_find_object(void **start, void **end, void *data, unsigned hint)
{
BDWGCFindObjectData *find_object_data = (BDWGCFindObjectData *)data;
char *p = (char *)start;
Comment on lines +1037 to +1041

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bdwgc_find_object doesn’t use the hint parameter. To avoid unused-parameter warnings under -Wall, consider marking it unused explicitly (or using an attribute/macro consistent with the project).

Copilot uses AI. Check for mistakes.
char *lim = (char *)(end - 1);
void *findee = find_object_data->findee;

if (findee == NULL)
return; /* already found */

for (; p <= lim; p += ALIGNOF_VOID_P) {
if (*(void **)p == findee) {
find_object_data->findee = NULL; /* found */
break;
}
}
}

static int
bdwgc_context_is_protected(BDWGCContext *ctx, void *obj)
{
BDWGCFindObjectData data;

assert(ctx);
if (obj == NULL) /* not expected actually */
return scm_true;

if (!ctx->is_protected)
return scm_false;

data.findee = obj;
GC_custom_push_regs_and_stack(bdwgc_find_object,
&data,
&(ctx->stack_base),
NULL);
return data.findee == NULL;
}
#endif