Project

General

Profile

Actions

Feature #20005

closed

Add C API to return symbols of native extensions resolved from features

Added by tagomoris (Satoshi Tagomori) 12 months ago. Updated 11 months ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:115388]

Description

I want an API to resolve symbols of other native extensions by a feature name and a symbol name (just like dlsym).

(rb_dln_resolve_symbol is an example function name of this feature in the example below)

// "a_client_open" is a function defined in "/..../a_client.so", that should be loaded in Ruby beforehand.

// in the extension to be "a.so"
VALUE (*a_client_open)(VALUE);

void Init_a(void)
{
  a_client_open = (VALUE(*)(VALUE))rb_dln_resolve_symbol("a_client", "a_client_open");
  // the return value may be NULL if the symbols is not found
}

This API is to replace direct reference of external symbols.

Currently, native extensions have to just call the function of other extensions directly on the assumption of:

  • The dependency native extension is already loaded
  • The symbol is defined correctly in the dependency (the version of dependency is correct&expected)
    Otherwise, it crashes.

This API provides a way for developers to raise exceptions if the resolved symbol is NULL, and these exceptions can display meaningful messages and instructions to the users of extensions.

Updated by nobu (Nobuyoshi Nakada) 12 months ago

a_client_open is a function just defined in a_client.so?
I.e., do you intend that all non-static functions can be retrieved from other extension libraries without any action in the defining library?

Updated by tagomoris (Satoshi Tagomori) 12 months ago

@nobu (Nobuyoshi Nakada) No, I intended that the API retrieves extern-ed (or EXPORTS-ed in *.def) symbols. I missed to mention it.

Updated by kjtsanaktsidis (KJ Tsanaktsidis) 12 months ago

Maybe I’m just not very clever, but I don’t understand why dlsym doesn’t already do this? Why doesn’t it work for your use case? I don’t think I understand.

Updated by nobu (Nobuyoshi Nakada) 12 months ago

@kjtsanaktsidis (KJ Tsanaktsidis) dlsym can do, but

  • not all platforms use dlopen and dlsym.
  • dlsym needs the handle of the loaded library, and it is not trivial to retrieve the path from the feature name.

Updated by kjtsanaktsidis (KJ Tsanaktsidis) 12 months ago

True, dlopen/dlsym have some portability issues, but:

dlsym needs the handle of the loaded library, and it is not trivial to retrieve the path from the feature name.

Can’t you just call dlopen(NULL) to get this? From the Linux man page:

If filename is NULL, then the returned handle is for the main
program. When given to dlsym(3), this handle causes a search for
a symbol in the main program, followed by all shared objects
loaded at program startup, and then all shared objects loaded by
dlopen() with the flag RTLD_GLOBAL.

Ruby uses RTLD_GLOBAL to load extensions I think, so any symbols exported by extensions should be accessible

Updated by nobu (Nobuyoshi Nakada) 12 months ago

kjtsanaktsidis (KJ Tsanaktsidis) wrote in #note-5:

True, dlopen/dlsym have some portability issues, but:

dlsym needs the handle of the loaded library, and it is not trivial to retrieve the path from the feature name.

Can’t you just call dlopen(NULL) to get this? From the Linux man page:

It returns RTLD_DEFAULT, no dlopen call is needed.
This special handle searches all loaded libraries in the loaded order.
That means it can't find a shadowed symbol.
Additionally, macOS man page says "This can be a costly search and should be avoided."

And another portability concern is exported symbols may be prefixed with "_".

Anyway it doesn't look a nice idea to make extensions libraries write #ifdef _WIN32 unnecessarily here and there.

I consider the wrapper API would be reasonable for these reasons.

Updated by kjtsanaktsidis (KJ Tsanaktsidis) 12 months ago

Ah that makes sense - if you want to get potentially overridden symbols there isn’t a good way to do that at the moment. Plus I grant you the portability story around symbol exporting is messy, so wrapping that up would be handy.

Updated by tagomoris (Satoshi Tagomori) 11 months ago

I noticed that dln_sym raises LoadError when the specified symbol is not found. But I expect this API to return NULL when the symbol is not found.
Which one should be better?

My expectation: libraries should raise LoadError under their own responsibility when this API returns NULL

Updated by nobu (Nobuyoshi Nakada) 11 months ago

tagomoris (Satoshi Tagomori) wrote in #note-8:

I noticed that dln_sym raises LoadError when the specified symbol is not found. But I expect this API to return NULL when the symbol is not found.
Which one should be better?

dln_sym has been used only to resolve and call immediately Init_xxx functions, and it would be reasonable to split error handling.

My expectation: libraries should raise LoadError under their own responsibility when this API returns NULL

What do you mean by "libraries" here?

diff --git a/dln.c b/dln.c
index bbed3af78ce..7c67fc2014f 100644
--- a/dln.c
+++ b/dln.c
@@ -419,32 +419,34 @@ dln_open(const char *file)
 static void *
 dln_sym(void *handle, const char *symbol)
 {
-    void *func;
-    const char *error;
-
 #if defined(_WIN32)
-    char message[1024];
+    return GetProcAddress(handle, symbol);
+#elif defined(USE_DLN_DLOPEN)
+    return dlsym(handle, symbol);
+#endif
+}
+
+static void *
+dln_sym_func(void *handle, const char *symbol)
+{
+    void *func = dln_sym(handle, symbol);
 
-    func = GetProcAddress(handle, symbol);
     if (func == NULL) {
+        const char *error;
+#if defined(_WIN32)
+        char message[1024];
         error = dln_strerror();
-        goto failed;
-    }
-
 #elif defined(USE_DLN_DLOPEN)
-    func = dlsym(handle, symbol);
-    if (func == NULL) {
         const size_t errlen = strlen(error = dln_strerror()) + 1;
         error = memcpy(ALLOCA_N(char, errlen), error, errlen);
-        goto failed;
-    }
 #endif
-
+        dln_loaderror("%s - %s", error, symbol);
+    }
     return func;
-
-  failed:
-    dln_loaderror("%s - %s", error, symbol);
 }
+
+#define dln_sym_call(rettype, argtype, handle, symbol) \
+    (*(rettype (*)argtype)dln_sym_func(handle, symbol))
 #endif
 
 #if defined(RUBY_DLN_CHECK_ABI) && defined(USE_DLN_DLOPEN)
@@ -464,9 +466,8 @@ dln_load(const char *file)
 
 #ifdef RUBY_DLN_CHECK_ABI
     typedef unsigned long long abi_version_number;
-    typedef abi_version_number abi_version_func(void);
-    abi_version_func *abi_version_fct = (abi_version_func *)dln_sym(handle, EXTERNAL_PREFIX "ruby_abi_version");
-    abi_version_number binary_abi_version = (*abi_version_fct)();
+    abi_version_number binary_abi_version =
+        dln_sym_call(abi_version_number, (void), handle, EXTERNAL_PREFIX "ruby_abi_version")();
     if (binary_abi_version != ruby_abi_version() && abi_check_enabled_p()) {
         dln_loaderror("incompatible ABI version of binary - %s", file);
     }
@@ -474,10 +475,9 @@ dln_load(const char *file)
 
     char *init_fct_name;
     init_funcname(&init_fct_name, file);
-    void (*init_fct)(void) = (void(*)(void))dln_sym(handle, init_fct_name);
 
     /* Call the init code */
-    (*init_fct)();
+    dln_sym_call(void, (void), handle, init_fct_name)();
 
     return handle;
 

Updated by tagomoris (Satoshi Tagomori) 11 months ago

nobu (Nobuyoshi Nakada) wrote in #note-9:

dln_sym has been used only to resolve and call immediately Init_xxx functions, and it would be reasonable to split error handling.

That makes sense.

My expectation: libraries should raise LoadError under their own responsibility when this API returns NULL

What do you mean by "libraries" here?

I meant extensions, that call this API.

The diff you pasted is really helpful. Just one point I have a question:

+#define dln_sym_call(rettype, argtype, handle, symbol) \
+    (*(rettype (*)argtype)dln_sym_func(handle, symbol))

This actually does not call the symbol but returns a callable symbol. So I think it's better to either:
A. rename it to dln_sym_callable
B. add () at the end of the line, and remove () from the code about "ruby_abi_version" and init_fct_name.

@nobu (Nobuyoshi Nakada) What do you think?

Updated by nobu (Nobuyoshi Nakada) 11 months ago

tagomoris (Satoshi Tagomori) wrote in #note-10:

This actually does not call the symbol but returns a callable symbol. So I think it's better to either:
A. rename it to dln_sym_callable
B. add () at the end of the line, and remove () from the code about "ruby_abi_version" and init_fct_name.

For B, that macro also needs an argument like argtype:

    dln_sym_call(void, (void), handle, init_fct_name, ());

I'm for A a little, right now.

Updated by tagomoris (Satoshi Tagomori) 11 months ago

nobu (Nobuyoshi Nakada) wrote in #note-11:

I'm for A a little, right now.

I see. I'll change the name later.

Updated by matz (Yukihiro Matsumoto) 11 months ago

Accepted (with a new name).

Matz.

Actions #14

Updated by tagomoris (Satoshi Tagomori) 11 months ago

  • Status changed from Open to Closed

Applied in changeset git|e51f9e9f75cc1dde9234836fa92077d71b3c5141.


rb_ext_resolve_symbol: C API to resolve and return externed symbols [Feature #20005]

This is a C API for extensions to resolve and get function symbols of other extensions.
Extensions can check the expected symbol is correctly loaded and accessible, and
use it if it is available.
Otherwise, extensions can raise their own error to guide users to setup their
environments correctly and what's missing.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like1Like0Like0Like0Like0Like0Like0Like0Like0