dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
114 stars 40 forks source link

[C] "Inline" function available in compiled dylib, but no bindings generated by ffigen? #459

Open modulovalue opened 1 year ago

modulovalue commented 1 year ago

I'm currently trying to bind a bitset implementation found in CRoaring to Dart via ffigen.

As expected, ffigen doesn't seem to generate bindings for inline functions such as:

static inline size_t bitset_size_in_words(const bitset_t *bitset) {
    return bitset->arraysize;
}

As expected, this function is not visible in the compiled dylib when searched for via nm -gU libroaring.dylib

However, if the static is changed to extern, i.e.:

extern inline size_t bitset_size_in_words(const bitset_t *bitset) {
    return bitset->arraysize;
}

Then the function becomes available in the compiled dylib:

> nm -gU libroaring.dylib | grep bitset_size_in_words         
> 0000000000001b08 T _bitset_size_in_words

but ffigen still won't generate bindings for it despite it being available in the dylib.

I'm wondering what's the exact limitation here? If a function is available in the dylib, shouldn't ffigen be able to generate appropriate bindings for it (regardless of whether it is inline annotated or not?)

Cf. https://github.com/RoaringBitmap/CRoaring/issues/487

modulovalue commented 1 year ago

I can confirm that a .lookupSymbol in dart on the following function declaration in a header file returns true:

/* Set the ith bit. Attempts to resize the bitset if needed (may silently fail)
 */
extern inline void bitset_set(bitset_t *bitset, size_t i) {
    ...
}

Edit:

The following also works if there's an extern declaration for it in the source:

inline void bitset_set(bitset_t *bitset, size_t i) {
    ...
}
modulovalue commented 1 year ago

This issue seems related, but It looks like it is about something related, but not quite the same https://github.com/dart-lang/native/issues/321

mannprerak2 commented 1 year ago

Currently, we filter out any inline functions from being generated, perhaps we can add a config to allow any function regardless of it being inline?

functions:
  skip_inline: false # default: true

The default should probably be true, it would depend on how common it is for libraries to declare the function as inline, but still expose it. wdyt @dcharkes?

dcharkes commented 1 year ago

Currently, we filter out any inline functions from being generated, perhaps we can add a config to allow any function regardless of it being inline?

functions:
  skip_inline: false # default: true

The default should probably be true, it would depend on how common it is for libraries to declare the function as inline, but still expose it. wdyt @dcharkes?

Can't we see it's extern inline and avoid adding a config option?

mannprerak2 commented 1 year ago

We can see if a cursor is extern using clang_Cursor_getStorageClass

Is there no other way for a symbol to be exposed without specifying extern? Also is it possible that either one of the header file or C file uses extern? Perhaps in such cases a skip_inline config would be required.

dcharkes commented 1 year ago

Is there no other way for a symbol to be exposed without specifying extern? Also is it possible that either one of the header file or C file uses extern? Perhaps in such cases a skip_inline config would be required.

I prefer we try to mimic the C compiler behavior and be precise about it.

I do not believe I know how C compilers work 🙃 there are likely more cases. We'll just cover them as our users run into them.

modulovalue commented 1 year ago

@dcharkes @mannprerak2 I believe, this solution doesn't cover all cases.

It looks to me like the change in dart-lang/ffigen#594 only considers whether a declaration is annotated extern in a header file, but a declaration can be annotated extern in the .c file, not be extern in the header file, and still be available in a dylib. That is, I believe, it is not enough to only consider the header file to discover whether an inline function is available in a dylib.

I've experienced this with the CRoaring project.

Consider, for example, these declarations in the .c file and these implementations in the header. (apparently, this is being done this way for performance reasons.)

Unless I'm mistaken, ffigen would not generate bindings for these functions because they are not extern in the header file, but it looks like it should, because they are available in the dylib.

dcharkes commented 1 year ago

Another wild idea: what about actually passing a path to the dynamic library when running FFIgen? Then we can lookup in the dynamic library if the symbol is there.

modulovalue commented 1 year ago

Then we can lookup in the dynamic library if the symbol is there.

That seems like it would work. I was using nm -gU libroaring.dylib on macos to look up symbols in dylibs while debugging these issues.

I agree that this feels like a wild idea, I think it would be nice if ffigen didn't have to depend on any one specific dylib.

[...] We'll just cover them as our users run into them.

It would be fine with me if solving this issue would be deferred to a later time. The current workaround is to manually add extern annotations to a copy of the header where that copy is only meant to be consumed by ffigen. It looks like dart-lang/ffigen#594 made that workaround possible.

mannprerak2 commented 1 year ago

Another wild idea: what about actually passing a path to the dynamic library when running FFIgen?

That would be platform specific, it's possible there's no dynamic library for the current platform, so we won't be able to open it to check if a symbol exist (Unless there's a way to do this in dart?)

A skip_inline flag seems like the safest option to me.

Edit: alternatively we can make skip_inline and include/exclude config as well