~andrewrk/ziglang

2

[0.11.0-dev] cImport AnalysisFail with ruby-3.3-rc

Details
Message ID
<CACEKW=LbzM8H9TqZ+PPMUM-CA3BmMiD3cSBi=dQ1ucVCBrNJFA@mail.gmail.com>
DKIM signature
pass
Download raw message
Playing with a very simple ruby zig extension[1].  It builds successfully
with ruby 2.5, 2.6, 2.7, 3.0, 3.1, and 3.2[2].  But, fails with 3.3-rc[3]:

+ zig version
0.11.0-dev.3220+447a30299
+ ruby -v
ruby 3.3.0preview1 (2023-05-12 master a1b01e7701) [x86_64-linux]

zig build-lib zigrb ReleaseSafe native: error: the following command
failed with 2 compilation errors:
/usr/local/bin/zig build-lib
/usr/local/bundle/gems/zigrb-0.0.1/ext/src/main.zig
-I/usr/local/include/ruby-3.3.0+0/x86_64-linux
-I/usr/local/include/ruby-3.3.0+0 -L/usr/local/lib -lruby -lm
-lpthread -lc -OReleaseSafe --cache-dir
/usr/local/bundle/gems/zigrb-0.0.1/ext/zig-cache --global-cache-dir
/root/.cache/zig --name zigrb -dynamic --listen=-
Build Summary: 0/6 steps succeeded; 2 failed (disable with -fno-summary)
install transitive failure
+- install zigrb transitive failure
   +- zig build-lib zigrb ReleaseSafe native 2 errors
/usr/local/bundle/gems/zigrb-0.0.1/ext/zig-cache/o/6da10025b96595a1763d1b367377584b/cimport.zig:9339:65:
error: use of undeclared identifier 'struct_s'
src/main.zig:2:14: error: C import failed: AnalysisFail
zig failed, exit code 2

[1] https://gitlab.com/fjc/gem-ext-zig_builder/-/blob/master/test/zigrb/ext/src/main.zig
[2] https://gitlab.com/fjc/gem-ext-zig_builder/-/jobs/4320132757
[3] https://gitlab.com/fjc/gem-ext-zig_builder/-/jobs/4320132758
Details
Message ID
<CACEKW=L3M2DdyVuqOAj2L2cW3gafnz79SbekgObJOwAbNC-yag@mail.gmail.com>
In-Reply-To
<CACEKW=LbzM8H9TqZ+PPMUM-CA3BmMiD3cSBi=dQ1ucVCBrNJFA@mail.gmail.com> (view parent)
DKIM signature
pass
Download raw message
On 5/20/23, Frank J. Cameron <fjc@fastmail.net> wrote:
> Playing with a very simple ruby zig extension[1].  It builds successfully
> with ruby 2.5, 2.6, 2.7, 3.0, 3.1, and 3.2[2].  But, fails with 3.3-rc[3]:
>
> error: use of undeclared identifier 'struct_s'
> error: C import failed: AnalysisFail

From git bisect, this is the line where the analysis fails:

	#define REF_EDGE(s, p) (offsetof(struct s, p))

running bash -c (cd /gem/test/zigrb/ext && rm -rf zig-cache && zig build)
7142328a94c85cc5f23630396f248b32133f87ab is the first bad commit
commit 7142328a94c85cc5f23630396f248b32133f87ab
Author: Matt Valentine-House <matt@eightbitraptor.com>
Date:   Thu Mar 16 22:23:17 2023 +0000

    [Feature #19406] Allow declarative definition of references

    When using rb_data_type_struct to wrap a C struct, that C struct can
    contain VALUE references to other Ruby objects.

    If this is the case then one must also define dmark and optionally
    dcompact callbacks in order to allow these objects to be correctly
    handled by the GC. This is suboptimal as it requires GC related logic to
    be implemented by extension developers. This can be a cause of subtle
    bugs when references are not marked of updated correctly inside these
    callbacks.

    This commit provides an alternative approach, useful in the simple case
    where the C struct contains VALUE members (ie. there isn't any
    conditional logic, or data structure manipulation required to traverse
    these references).

    In this case references can be defined using a declarative syntax
    as a list of edges (or, pointers to references).

    A flag can be set on the rb_data_type_struct to notify the GC that
    declarative references are being used, and a list of those references
    can be assigned to the dmark pointer instead of a function callback, on
    the rb_data_type_struct.

    Macros are also provided for simple declaration of the reference list,
    and building edges.

    To avoid having to also find space in the struct to define a length for
    the references list, I've chosed to always terminate the references list
    with RUBY_REF_END - defined as UINTPTR_MAX. My assumption is that no
    single struct will ever be large enough that UINTPTR_MAX is actually a
    valid reference.

 gc.c                                    | 51 +++++++++++++++++++++++++++++----
 include/ruby/internal/core/rtypeddata.h |  9 +++++-
 include/ruby/internal/gc.h              |  6 ++++
 3 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/gc.c b/gc.c
index c269d7fce0..06a4bacfb8 100644
--- a/gc.c
+++ b/gc.c
@@ -7241,6 +7241,39 @@ gc_mark_imemo(rb_objspace_t *objspace, VALUE obj)
     }
 }

+static bool
+gc_declarative_marking_p(const rb_data_type_t *type)
+{
+    return (type->flags & RUBY_TYPED_DECL_MARKING) != 0;
+}
+
+#define EDGE (VALUE *)((char *)data_struct + offset)
+
+static inline void
+gc_mark_from_offset(rb_objspace_t *objspace, VALUE obj)
+{
+    // we are overloading the dmark callback to contain a list of offsets
+    size_t *offset_list = (size_t
*)RANY(obj)->as.typeddata.type->function.dmark;
+    void *data_struct = RANY(obj)->as.typeddata.data;
+
+    for (size_t offset = *offset_list; *offset_list != RUBY_REF_END;
offset = *offset_list++) {
+        rb_gc_mark_movable(*EDGE);
+    }
+}
+
+static inline void
+gc_ref_update_from_offset(rb_objspace_t *objspace, VALUE obj)
+{
+    // we are overloading the dmark callback to contain a list of offsets
+    size_t *offset_list = (size_t
*)RANY(obj)->as.typeddata.type->function.dmark;
+    void *data_struct = RANY(obj)->as.typeddata.data;
+
+    for (size_t offset = *offset_list; *offset_list != RUBY_REF_END;
offset = *offset_list++) {
+        if (SPECIAL_CONST_P(*EDGE)) continue;
+        *EDGE = rb_gc_location(*EDGE);
+    }
+}
+
 static void
 gc_mark_children(rb_objspace_t *objspace, VALUE obj)
 {
@@ -7350,10 +7383,15 @@ gc_mark_children(rb_objspace_t *objspace, VALUE obj)
         {
             void *const ptr = DATA_PTR(obj);
             if (ptr) {
-                RUBY_DATA_FUNC mark_func = RTYPEDDATA_P(obj) ?
-                    any->as.typeddata.type->function.dmark :
-                    any->as.data.dmark;
-                if (mark_func) (*mark_func)(ptr);
+                if (RTYPEDDATA_P(obj) &&
gc_declarative_marking_p(any->as.typeddata.type)) {
+                    gc_mark_from_offset(objspace, obj);
+                }
+                else {
+                    RUBY_DATA_FUNC mark_func = RTYPEDDATA_P(obj) ?
+                        any->as.typeddata.type->function.dmark :
+                        any->as.data.dmark;
+                    if (mark_func) (*mark_func)(ptr);
+                }
             }
         }
         break;
@@ -10606,7 +10644,10 @@ gc_update_object_references(rb_objspace_t
*objspace, VALUE obj)
         {
             void *const ptr = DATA_PTR(obj);
             if (ptr) {
-                if (RTYPEDDATA_P(obj)) {
+                if (RTYPEDDATA_P(obj) &&
gc_declarative_marking_p(any->as.typeddata.type)) {
+                    gc_ref_update_from_offset(objspace, obj);
+                }
+                else if (RTYPEDDATA_P(obj)) {
                     RUBY_DATA_FUNC compact_func =
any->as.typeddata.type->function.dcompact;
                     if (compact_func) (*compact_func)(ptr);
                 }
diff --git a/include/ruby/internal/core/rtypeddata.h
b/include/ruby/internal/core/rtypeddata.h
index bbf208867d..9d91a3e3f1 100644
--- a/include/ruby/internal/core/rtypeddata.h
+++ b/include/ruby/internal/core/rtypeddata.h
@@ -176,7 +176,14 @@ rbimpl_typeddata_flags {
      * This flag  is mysterious.  It seems  nobody is currently using  it.  The
      * intention of this flag is also unclear.  We need further investigations.
      */
-    RUBY_TYPED_PROMOTED1        = RUBY_FL_PROMOTED1     /* THIS FLAG
DEPENDS ON Ruby version */
+    RUBY_TYPED_PROMOTED1        = RUBY_FL_PROMOTED1,     /* THIS FLAG
DEPENDS ON Ruby version */
+
+    /**
+     * This flag determines whether marking and compaction should be
carried out
+     * using the dmark/dcompact callback functions or whether we should mark
+     * declaratively using a list of references defined inside the
data struct we're wrapping
+     */
+    RUBY_TYPED_DECL_MARKING     = RUBY_FL_USER2
 };

 /**
diff --git a/include/ruby/internal/gc.h b/include/ruby/internal/gc.h
index fb14b34211..bc81883e0e 100644
--- a/include/ruby/internal/gc.h
+++ b/include/ruby/internal/gc.h
@@ -44,6 +44,12 @@

 RBIMPL_SYMBOL_EXPORT_BEGIN()

+#define REF_EDGE(s, p) (offsetof(struct s, p))
+#define REFS_LIST_PTR(l) ((RUBY_DATA_FUNC)l)
+#define RUBY_REF_END SIZE_MAX
+#define RUBY_REFERENCES_START(t) static size_t t[] = {
+#define RUBY_REFERENCES_END RUBY_REF_END, };
+
 /* gc.c */

 RBIMPL_ATTR_COLD()
Details
Message ID
<CACEKW=K3xjH67Z6WDU_3UHgrOpVrLrgq2SHtm4hVZo6E_G34VQ@mail.gmail.com>
In-Reply-To
<CACEKW=L3M2DdyVuqOAj2L2cW3gafnz79SbekgObJOwAbNC-yag@mail.gmail.com> (view parent)
DKIM signature
pass
Download raw message
On 5/21/23, Frank J. Cameron <fjc@fastmail.net> wrote:
> On 5/20/23, Frank J. Cameron <fjc@fastmail.net> wrote:
>> Playing with a very simple ruby zig extension[1].  It builds successfully
>> with ruby 2.5, 2.6, 2.7, 3.0, 3.1, and 3.2[2].  But, fails with
>> 3.3-rc[3]:
>>
>> error: use of undeclared identifier 'struct_s'
>> error: C import failed: AnalysisFail
>
> From git bisect, this is the line where the analysis fails:
>
> 	#define REF_EDGE(s, p) (offsetof(struct s, p))

$ zig translate-c -Iinclude -I.ext/include/x86_64-linux -I/usr/include
-I/usr/include/x86_64-linux-gnu include/ruby/ruby.h

Produces:

pub const offsetof = @compileError("unable to translate macro:
undefined identifier `__builtin_offsetof`");
pub inline fn REF_EDGE(s: anytype, p: anytype) @TypeOf(offsetof(struct_s, p)) {
    _ = @TypeOf(s);
    return offsetof(struct_s, p);
}

$ zig build-obj /tmp/rberr.zig
/tmp/rberr.zig:3:65: error: use of undeclared identifier 'struct_s'
pub inline fn REF_EDGE(s: anytype, p: anytype) @TypeOf(offsetof(struct_s, p)) {
                                                                ^~~~~~~~
Reply to thread Export thread (mbox)