Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to build library without inlining #3664

Open
2 of 7 tasks
andre2007 opened this issue Jun 3, 2020 · 13 comments
Open
2 of 7 tasks

How to build library without inlining #3664

andre2007 opened this issue Jun 3, 2020 · 13 comments
Labels
Component: Core Issues related to the core code Priority: Medium Issue with medium priority Type: Enhancement Describes a feature request or addition of new functionality Type: Question General question or code understanding

Comments

@andre2007
Copy link

Description

The compiled libraries (https://github.com/open62541/open62541/releases/download/v1.1-rc1/open62541-linux64.tar.gz) makes heavy use of inlining. Therefore a lot of functions are missing
while trying to call the SO from other programming languages than C ( Example the D Programming Language).

It is almost impossible to get the client example running due to this missing references:

  • UA_Variant_init
  • UA_NODEID_STRING
  • UA_Client_readValueAttribute
  • UA_Variant_hasScalarType
  • UA_Variant_clear

Could you add in the documentation a step, how to compile the library without inlining?
Also it would be interesting how a static library (*.a) could be generated instead of shared object (*.so).

Background Information / Reproduction Steps

Used CMake options:

cmake -DUA_NAMESPACE_ZERO=<YOUR_OPTION> <ANY_OTHER_OPTIONS> ..

Checklist

Please provide the following information:

  • open62541 Version (release number or git tag): v1.1-rc1
  • Other OPC UA SDKs used (client or server):
  • Operating system: Ubuntu 18.04
  • Logs (with UA_LOGLEVEL set as low as necessary) attached
  • Wireshark network dump attached
  • Self-contained code example attached
  • Critical issue
@jpfr
Copy link
Member

jpfr commented Jun 3, 2020

UA_Variant_init
UA_NODEID_STRING
UA_Client_readValueAttribute
UA_Variant_hasScalarType
UA_Variant_clear

Right. So you use a language that has an FFI to call C functions. But you cannot use static inline function definitions.

You raise a good point. Let's get rid of static inline functions.
But it might take a while / contributor effort.
Can you contribute to the cases that are immediate to what you are working on?

We generate an .a archive-library by default.
You can switch between .a and .so via the config option BUILD_SHARED_LIBS.

@andre2007
Copy link
Author

My C knowledge and especially building them on linux / windows is quite limited. I cannot help here much but I can give some thoughts

  • Maybe some of your user C base would complain if you remove inlining in general.
  • I do not understand it completely, but I thought UA_INLINE is a macro definition static UA_INLINE UA_StatusCode UA_Client_readValueAttribute(UA_Client *client, const UA_NodeId nodeId, UA_Variant *outValue). Therefore I thought there is maybe a build switch, similiar to BUILD_SHARED_LIBS to influence whether this macro should evaluate to INLINE or to ``.
  • In worst case functions like UA_Variant_init_wrap, UA_NODEID_STRING_WRAP, UA_Client_readValueAttribute_wrap could be added which calls the inlined functions and are itself not marked as INLINE?

@jpfr
Copy link
Member

jpfr commented Jun 4, 2020

The question is for which compilation unit the object code (machine code) is generated.

In C, every .c file is a compilation unit that leads to a .o object file. The .o are combined in the final executable.

With normal functions, the .c file that defines the function body gets to hold the object code for it.

But what if the inline function is only defined in a .h header and not in a .c file?
Such inline functions lead to custom object code that is generated in all places that call this function.
This probably confues your D compiler.

@andre2007
Copy link
Author

I am not sure whether I understand everything of your comment, as here I only have very few experience.
What I know is, the shared object (*.so) / static library (*.a) does not have the symbols for UA_Variant_init as their function bodies are inlined into the caller functions. As far as I can see also none of the object files (*.o).

I just checked, there was a similar problem in librdkafka. The solution was actually to remove the inline attribute (confluentinc/librdkafka@7207083)

@NoelGraf NoelGraf added Component: Core Issues related to the core code Priority: Medium Issue with medium priority Type: Enhancement Describes a feature request or addition of new functionality Type: Question General question or code understanding labels May 6, 2021
@MattesWhite
Copy link

MattesWhite commented Aug 31, 2022

This issue is still relevant. Creating bindings for a Rust project we faced the same problems. Lots of high level convenience functions are declared static UA_INLINE which especially Rust's bindgen crate will ignore/create no bindings.

The suggested solution to replace UA_INLINE with #define UA_INLINE, i.e. make it empty does not work out of the box. Because it will leave static UA_Statuscode UA_Client_readValueAttribute() and similar functions in header files which cmake/clang don't like.

As a workaround we use a script that replaces static UA_INLINE in important header files with UA_EXPORT and disable check_add_cc_flag("-Wmissing-prototypes") and check_add_cc_flag("-Wstrict-prototypes") in CMakeLists.txt to suppress build warnings/errors. It works but we are not proud of the solution. However, the workaround will not work on generated inline functions like static UA_INLINE UA_String_clear().

Was there any progress on this issue from your side?

@andre2007
Copy link
Author

@MattesWhite Your workaround seems to be the best solution.
There is no progress on my side but actually the DLang has added in the meantime a C99 compiler into the language (importC).

@jpfr
Copy link
Member

jpfr commented Sep 8, 2022

That‘s a tough issue.

On one hand it would be great to enable language bindings/generation.

On the other hand inline functions are great.
Better type checking than with macros and no overhead for unused functions.
The last point is important as we auto-generate thousands of small type-handling functions.

In an ideal world my preferred solution would be this:

We wrap static inline methods inside a macro.

UA_INLINE(declaration, implementation)

That macro can be controlled to generate either 1) a static inline method or 2) a UA_EXPORT method declaration or 3) the method implementation.

Then we can have an optional single .c compilation unit that includes the headers with a macro configuration that generates normal code.

Do you have some bandwidth to start working in that direction?

@MattesWhite
Copy link

I'm willing to try but can't say when I have the time left to dig in.

Just for clarification your suggestion is to write the UA_INLINE that, depending on a #define either extends to a static inline function with implementation (of course) or to a UA_EXPORT declaration and in the C source file to the implementation? So roughly:

#define UA_INLINE(name, decl, impl) 
#ifndef UA_EXPORT_INLINES
  static UA_INLINE_DECL decl impl
#else
  #ifndef UA_INLINE##name
    decl
    #define UA_INLINE##name
  #else
    impl
  #endif
#endif

Disclaimer: I haven't written such macros in a while but the idea should be clear

@jpfr
Copy link
Member

jpfr commented Sep 9, 2022

Yeah, exactly!
Taking your proposal a bit further:

#define UA_INLINE(decl, impl) 
#if defined(UA_INLINE_EXPORT) and defined(UA_INLINE_IMPL)
   decl impl
#elif defined(UA_INLINE_EXPORT)
  UA_EXPORT decl ;
#else
  static inline decl impl
#endif

With UA_INLINE_EXPORT as a global setting and UA_INLINE_IMPL only in the "code-generation" .c.
Than we define for example UA_Variant_new as:

UA_INLINE(UA_Variant * UA_Variant_init(void),
          { return UA_new(&UA_TYPES[UA_TYPES_VARIANT]); })

@jpfr
Copy link
Member

jpfr commented Nov 14, 2022

This is being implemented in #5445.

@pieter-scholtz
Copy link
Contributor

I am having problems getting UA_Variant_init to not compile as static inline , I have UA_ENABLE_INLINABLE_EXPORT , BUILD_SHARED_LIBS and UA_ENABLE_AMALGAMATION set to ON , am I missing something ? I understand that UA_Variant_init is a auto-generated type-handling function . Is it possible that this has not been implemented for the auto-generated functions ? ( I am a bit clumsy when it comes to understanding C )

@jpfr
Copy link
Member

jpfr commented Jul 27, 2023

Yes, we need to add the inline-wrapper handline from #5445 to the auto-generated methods.
You could try your hand here:

def print_functions(self, datatype):

@pieter-scholtz
Copy link
Contributor

I made a pull request with 2 initial commits #5926 , please let me know if the style is acceptable or if I need to change anything , or do it differently for the rest of the changes : D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Issues related to the core code Priority: Medium Issue with medium priority Type: Enhancement Describes a feature request or addition of new functionality Type: Question General question or code understanding
Projects
None yet
Development

No branches or pull requests

5 participants