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

Function implementations in the tkrzw module interface #61

Open
SchaichAlonso opened this issue Nov 13, 2024 · 2 comments · May be fixed by #62
Open

Function implementations in the tkrzw module interface #61

SchaichAlonso opened this issue Nov 13, 2024 · 2 comments · May be fixed by #62

Comments

@SchaichAlonso
Copy link
Contributor

SchaichAlonso commented Nov 13, 2024

Hi

tkrzw has quite a bit of code that is implemented in header files that are supposed to be included by consuming projects. This causes some technical issues with weak symbols inside the consumer's translation units (or .obj files), that might fail to be deduplicated if the consuming project uses compiler settings different from those used by the tkrzw build system.

More importantly, however, parts of the code, especially non-virtual implementations, might end up being embedded into consuming applications by optimizers that eliminate the procedure calls, raising concerns considering licensing.

SchaichAlonso added a commit to SchaichAlonso/tkrzw that referenced this issue Nov 13, 2024
tkrzw has quite a bit of code that is implemented in header files
that are supposed to be included by consuming projects.

This causes some technical issues with [weak symbols](https://en.wikipedia.org/wiki/Weak_symbol) inside
the consumer's translation units (or `.obj` files), that might fail
to be deduplicated if the consuming project uses compiler settings
different from those used by the tkrzw build system.

More importantly, however, parts of the code, especially imlementations
of non-virtual functions, might end up being embedded into consumer
applications when an optimization phase eliminates the procedure
calls, raising many concerns considering licensing.

This commit moves all implementations from header files documented
for inclusion into third-party projects on https://dbmx.net/tkrzw/,
as well as any header that implicitly gets included by including a
tkrzw interface header into source files.
Further, `inline` declarations are removed as within tkrzw, the
inlining happens on the discredition of the linker independed of
the `inline` keyword, while cross-module inlining is what this
commit is supposed to interdict.

Nothing can be done about template code in tkrzw_lib_common.h ,
however, template code is whitelisted on GPLv3.
SchaichAlonso added a commit to SchaichAlonso/tkrzw that referenced this issue Nov 13, 2024
tkrzw has quite a bit of code that is implemented in header files
that are supposed to be included by consuming projects.

This causes some technical issues with [weak symbols](https://en.wikipedia.org/wiki/Weak_symbol) inside
the consumer's translation units (or `.obj` files), that might fail
to be deduplicated if the consuming project uses compiler settings
different from those used by the tkrzw build system.

More importantly, however, parts of the code, especially imlementations
of non-virtual functions, might end up being embedded into consumer
applications when an optimization phase eliminates the procedure
calls, raising many concerns considering licensing.

This commit moves all implementations from header files documented
for inclusion into third-party projects on https://dbmx.net/tkrzw/,
as well as any header that implicitly gets included by including a
tkrzw interface header into source files.
Further, `inline` declarations are removed as within tkrzw, the
inlining happens on the discredition of the linker independed of
the `inline` keyword, while cross-module inlining is what this
commit is supposed to interdict.

Nothing can be done about template code in tkrzw_lib_common.h ,
however, template code is whitelisted on GPLv3.
@SchaichAlonso SchaichAlonso linked a pull request Nov 13, 2024 that will close this issue
@SchaichAlonso SchaichAlonso changed the title DBM class implementations in the interface Function implementations in the tkrzw module interface Nov 13, 2024
@estraier
Copy link
Owner

Hi,

Thanks for making the PR. However, let me ask some questions because I wrote some codes in the header files intentionally.

With --enable-debug, CFLAGS has -fno-inline, which avoids embedding of inline functions. Thus, you can trace inline functions on the debugger.
What are actual problems caused by weak symbols?
Embedding inline functions is useful for optimization. Whether to optimize them or not is up to the optimizer's decision.

As for licensing, Tkrzw uses the Apache2 license, not GPL3.
I'm not sure why using header files causes licensing problems.

@SchaichAlonso
Copy link
Contributor Author

Hi,

Within itself (and its' tests/demos/examples), tkrzw forms a homogenous unit of CFLAGS and everything has the same license, too.
When another project attempts using tkrzw, by including its interface headers and linking against its' library, however, this is not neccessarily the case any more. "Random" projects have more or less "random" licenses, and their CFLAGS might be different from those used to build tkrzw, too.
If -fno-inline is used to the build of tkrzw, it will cause the tkrzw code not to be inlined within the tkrzw library. It does, however, not affect whether code of tkrzw is inlined into other projects that #include tkrzw code.
Independend of whether tkrzw has been built using -fno-inline, adding -fno-inline to the external projects' CFLAGS will stop tkrzw code from getting inlined into the external project, and getting mixed with foreign-licensed code there. However, the flag will also suppress inlining of any other function within the foreign code.

From a packaging/operations perspective, a goal would be the ability to replace tkrzw by replacing libtkrzw.so with an ABI-compatible version. If parts of tkrzw have ended up getting inlined into tkrzw-using binaries, then these binaries will need to be rebuilt and redeployed on top of tkrzw in order to swap tkrzw.

Note that in order to "inline" a function, an optimizer doesn't need the inline keyword, it only needs enough "knowledge"... i.e. non-virtual functions can almost always be inlined, unless they are decorated with compiler specific attributes, such as __attribute__(("noinline")), or a link unit wide policy like -fno-inline interdicts it.

SchaichAlonso added a commit to SchaichAlonso/tkrzw that referenced this issue Nov 14, 2024
tkrzw has quite a bit of code that is implemented in header files
that are supposed to be included by consuming projects.

This causes some technical issues with [weak symbols](https://en.wikipedia.org/wiki/Weak_symbol) inside
the consumer's translation units (or `.obj` files), that might fail
to be deduplicated if the consuming project uses compiler settings
different from those used by the tkrzw build system.

More importantly, however, parts of the code, especially imlementations
of non-virtual functions, might end up being embedded into consumer
applications when an optimization phase eliminates the procedure
calls, raising many concerns considering licensing.

This commit moves all implementations from header files documented
for inclusion into third-party projects on https://dbmx.net/tkrzw/,
as well as any header that implicitly gets included by including a
tkrzw interface header into source files.
Further, `inline` declarations are removed as within tkrzw, the
inlining happens on the discredition of the linker independed of
the `inline` keyword, while cross-module inlining is what this
commit is supposed to interdict.

Nothing can be done about template code in tkrzw_lib_common.h ,
however, template code is whitelisted on GPLv3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants