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

JNI shared object pollutes ELF symbol namespace #102

Open
shoffmeister opened this issue Oct 26, 2024 · 0 comments
Open

JNI shared object pollutes ELF symbol namespace #102

shoffmeister opened this issue Oct 26, 2024 · 0 comments

Comments

@shoffmeister
Copy link

The shared object provisioned for JNI integration pollutes the ELF symbol namespace by exporting functions which are not part of the the DuckDB API.

To reproduce, unpack the JAR to get access to the libduckdb_java.so_linux_amd64 then run

llvm-readelf --dyn-syms --demangle --elf-output-style=JSON libduckdb_java.so_linux_amd64 | jq '[.[] | .DynamicSymbols[] | select(.Symbol.Binding.Name == "Global" and .Symbol.Type.Name == "Function" and .Symbol.Section.Name == ".text" and (.Symbol.Name.Name | startswith("non-virtual thunk to") | not) and (.Symbol.Name.Name | startswith("virtual thunk to") | not)) ] | sort_by(.Symbol.Name.Name) | [ .[] | .Symbol.Name.Name ] ' 

Note exports such as

  • "AdbcConnectionCommit",
  • "UCaseMap::UCaseMap(char const*, unsigned int, UErrorCode*)",
  • "icu_66::AbsoluteValueSubstitution::getDynamicClassID() const",
  • "_ZGTtNSt14overflow_errorC1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE", <-- not demangled
  • "compressAuto(Encoder*, unsigned long, unsigned long*, unsigned char**, unsigned long, unsigned char*, unsigned long*, unsigned char**, int)",
  • "std::range_error::range_error(char const*)",
  • "u_memrchr32",
  • "ztrans_setTime",

All these become available to the ELF loader as targets to resolve symbols to.

As long as DuckDB is the only JNI in this context, that can be OK, I suppose. But whenever an other shared object needs resolving after DuckDB, then this might resolve to a DuckDB exported symbol - unintentionally.

So what DuckDB is doing here to (potentially) other is the same as RocksDB is doing to DuckDB in #14

The DuckDB JNI build of the C++ library should only really export what must be exported, to encapsulate the DuckDB JNI better. Whether or not the DuckDB "owned" C++ exports are exported could be debatable, but exporting icu_66::, for instance, feels wrong.

The only items requiring exporting seem to be the various JNI methods, e.g.

 "JNI_OnLoad",
  "JNI_OnUnload",
  "Java_org_duckdb_DuckDBNative_duckdb_1jdbc_1appender_1append_1boolean",
  "Java_org_duckdb_DuckDBNative_duckdb_1jdbc_1appender_1append_1byte",
  "Java_org_duckdb_DuckDBNative_duckdb_1jdbc_1appender_1append_1decimal",
...
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

No branches or pull requests

1 participant