github-actions[bot] commented on code in PR #67701:
URL: https://github.com/apache/doris/pull/67701#discussion_r3967411172
##########
be/src/service/CMakeLists.txt:
##########
@@ -64,21 +64,18 @@ if (${MAKE_TEST} STREQUAL "OFF" AND ${BUILD_BENCHMARK}
STREQUAL "OFF")
endif ()
pch_reuse(doris_be)
- # This permits libraries loaded by dlopen to link to the symbols in the
program.
- set_target_properties(doris_be PROPERTIES ENABLE_EXPORTS 1)
-
- # ...but not the symbols of the RocksDB we link statically. Exporting
those makes this
- # executable the definition every later-loaded library binds to, and a JNI
library that
- # carries its own RocksDB then runs half on ours: the fluss scanner
bundles frocksdbjni,
- # whose librocksdbjni.so defines 2576 rocksdb symbols under names
identical to ours but
- # was built against the pre-C++11 libstdc++ string ABI. Objects laid out
by one and used
- # by the other yield a garbage length, an std::bad_alloc that escapes the
JNI frame, and
- # an aborted BE. Hiding this archive lets that library bind to its own
copy.
- #
- # Scoped to the archive rather than dropping ENABLE_EXPORTS: what needs
the exports is
- # native UDFs (runtime/user_function_cache.cpp dlopens them), and those
use the Doris UDF
- # ABI, which has nothing to do with RocksDB. Crash stacks do not need it
either -- they are
- # symbolized from debug info, which is why they name even
anonymous-namespace functions.
+ # Keep doris_be symbols private. Java UDF callbacks are registered
explicitly through JNI and
Review Comment:
Removing executable exports also removes the fallback that keeps Doris
function names in `.dynsym`, but the supported `STRIP_DEBUG_INFO=ON` package
cannot load its separate symbols in-process: this file installs
`lib/debug_info/doris_be.dbg`, while `SymbolIndex` only checks an adjacent
`doris_be.debug`, `/usr/lib/debug/...`, or a build-id path and does not follow
`.gnu_debuglink`. After `--strip-unneeded`, it therefore falls back to a
stripped executable with almost no Doris symbols, so exception/crash stacks and
the thread-stack endpoint render most frames as `?`. Please make `SymbolIndex`
locate the packaged `.dbg` file (or otherwise preserve the needed symbol
source) before dropping `ENABLE_EXPORTS`, and cover the stripped layout in a
test.
##########
be/CMakeLists.txt:
##########
@@ -855,7 +855,17 @@ if (GLIBC_COMPATIBILITY)
# Keep lance_c here instead of COMMON_THIRDPARTY: placing its required
libm there
# would resolve -lm symbol before Doris compatibility is scanned,
preventing
# the linker from selecting Doris' optimized implementations.
+ # lance_c can also introduce single-precision math references after the
+ # compatibility archive has already been scanned. As ClickHouse does for
its
+ # compatibility archive, force the GLIBC_2.27 math symbols to be undefined
up
+ # front so the linker extracts Doris' musl implementations instead of
binding
+ # late references to the toolchain's libm.
set(DORIS_LINK_LIBS ${DORIS_LINK_LIBS}
+ -Wl,-u,logf
Review Comment:
These `-u` roots force musl's public math definitions into every linked
executable, but the local domain/range helpers only raise floating-point
exceptions and never set `errno`. Code compiled against glibc therefore sees
regressions across this block: for example `expf(100)`/`exp2f(128)` and zero
logs leave `errno` unchanged instead of `ERANGE`, while negative logs or
`powf(-1, 0.5)` omit `EDOM`. Please preserve glibc's errno contract for all
five forced functions and add final-linked domain/range tests; fixing only the
numerical result or only `expf` would leave the other interposed symbols
incompatible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
