-
Notifications
You must be signed in to change notification settings - Fork 140
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
## Summary of changes This PR addresses the issue #6045 ## Reason for change When using the `dlsym` function, the compiler adds in the import symbols table that we need the `dlsym` symbol. Before being a universal binary (same binary used for glibc-based linux and musl-libc-based linux) and the compiler added in a `DT_NEEDED` section the library `libdl.so` (the library containing `dlsym`). When the wrapper is loaded, it will look through all the `DT_NEEDED` sections to find a library that contains the `dlsym` symbol. Since being a universal binary, the `DT_NEEDED` sections are removed (part of being universal) and we have to resolve by hand needed symbols (`dlsym`, `pthread_once` ..). If we use `dlsym` (or other symbol), we will hit this issue. ## Implementation details - use `__dd_dlsym` instead ## Test coverage Added a snapshot test using `nm` that verifies that the undefined symbols in the universal binary haven't changed. It's equivalent to running ```bash nm -D Datadog.Linux.ApiWrapper.x64.so | grep ' U ' | awk '{print $2}' | sed 's/@.*//' | sort ``` but done using Nuke instead. It would probably make sense for this to be a "normal" test in the native tests, but given it has a dependency on `nm`, which is _definitely_ available in the universal build dockerfile it was quicker and easier to get this up and running directly. When it fails, it prints the diff and throws an exception, e.g. ```bash System.Exception: Found differences in undefined symbols (dlsym) in the Native Wrapper library. Verify that these changes are expected, and will not cause problems. Removing symbols is generally a safe operation, but adding them could cause crashes. If the new symbols are safe to add, update the snapshot file at C:\repos\dd-trace-dotnet\tracer\test\snapshots\native-wrapper-symbols-x64.verified.txt with the new values ``` ## Other details This will be hotfixed onto 3.3.1 and 2.59.1 --------- Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
- Loading branch information
1 parent
848c2b1
commit 4fbd8a5
Showing
8 changed files
with
152 additions
and
35 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14 changes: 14 additions & 0 deletions
14
tracer/test/snapshots/native-wrapper-symbols-arm64.verified.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
__errno_location | ||
access | ||
asprintf | ||
free | ||
getauxval | ||
getenv | ||
malloc | ||
strcasecmp | ||
strcmp | ||
strcpy | ||
strlen | ||
strncmp | ||
strncpy | ||
strrchr |
14 changes: 14 additions & 0 deletions
14
tracer/test/snapshots/native-wrapper-symbols-x64.verified.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
__errno_location | ||
__tls_get_addr | ||
access | ||
asprintf | ||
free | ||
getenv | ||
malloc | ||
strcasecmp | ||
strcmp | ||
strcpy | ||
strlen | ||
strncmp | ||
strncpy | ||
strrchr |