Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: DataDog/dd-trace-dotnet
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v3.3.0
Choose a base ref
...
head repository: DataDog/dd-trace-dotnet
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v3.3.1
Choose a head ref
  • 2 commits
  • 47 files changed
  • 3 contributors

Commits on Sep 18, 2024

  1. [Version Bump] 3.3.1

    @github-actions @andrewlock
    github-actions[bot] authored and andrewlock committed Sep 18, 2024
    Configuration menu
    Copy the full SHA
    b8f6066 View commit details
    Browse the repository at this point in the history

  2. Fix dlsym issue (#6048 => hotfix) (#6049) 

    ## 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 is a hotfix for 
- #6048

Co-authored-by: Gregory LEOCADIE <gregory.leocadie@datadoghq.com>
    @andrewlock @gleocadie
    andrewlock and gleocadie committed Sep 18, 2024
    Configuration menu
    Copy the full SHA
    2405e4c View commit details
    Browse the repository at this point in the history
Loading