Skip to content

Conversation

@zhengyu123
Copy link
Contributor

@zhengyu123 zhengyu123 commented Jan 20, 2026

What does this PR do?:
Handle soft link paths in path comparison.

Motivation:
Java profiler patches library's pthread_create() @plt entry, but not itself. If java profiler's library is on a path with soft link, path comparison may fail that results it accidentally patches itself.

Additional Notes:

How to test the change?:

  • Regular tests
  • Drop libjavaprofiler.so to a path with soft link, run `java -agentpath:${PATH_WITH_SOFT_LINK}/libjavaProfiler.so=start,filter=1,wall=10ms,cpu=10ms,file=profile.jfr $JAVA_APPLICATION

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.
  • JIRA: PROF-13517

Unsure? Have a question? Request a review!

@zhengyu123 zhengyu123 marked this pull request as ready for review January 20, 2026 20:46
@dd-octo-sts
Copy link

dd-octo-sts bot commented Jan 20, 2026

Scan-Build Report

User:runner@runnervmmtnos
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 clean all
Clang Version:Ubuntu clang version 18.1.3 (1ubuntu1)
Date:Tue Jan 20 20:47:48 2026

Bug Summary

Bug TypeQuantityDisplay?
All Bugs10
Unused code
Dead assignment2
Dead initialization7
Dead nested assignment1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Unused codeDead assignmentlibraryPatcher_linux.cpppatch_library_unlocked931
Unused codeDead assignmentstackWalker.cppwalkVM5511
Unused codeDead initializationjavaApi.cppJava_com_datadoghq_profiler_JavaProfiler_getStatus01191
Unused codeDead initializationprofiler.cpprunInternal16441
Unused codeDead initializationflightRecorder.cppcleanupUnreferencedMethods6581
Unused codeDead initializationflightRecorder.cppcleanupUnreferencedMethods6911
Unused codeDead initializationflightRecorder.cppresolveMethod3511
Unused codeDead initializationjavaApi.cppJava_com_datadoghq_profiler_OTelContext_setProcessCtx04751
Unused codeDead initializationvmStructs_dd.cppinitOffsets561
Unused codeDead nested assignmentvmStructs_dd.cppcheckNativeBinding3881

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a bug where the Java profiler could accidentally patch itself when its library path contains symbolic links. The fix uses realpath() to resolve symbolic links before comparing library paths.

Changes:

  • Added realpath() call to resolve the profiler's own library path in initialize()
  • Added realpath() call to resolve each library's path before comparing with the profiler's path in patch_library_unlocked()
  • Handles special cases like virtual files (e.g., [vdso]) that cannot be resolved

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bool ret = dladdr(caller_address, &info);
assert(ret);
_profiler_name = strdup(info.dli_fname);
_profiler_name = realpath(info.dli_fname, nullptr);
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing error handling: If realpath() fails (returns nullptr), _profiler_name will be set to nullptr. This could cause crashes later when _profiler_name is used in strcmp() at line 96. The code should handle this error case, either by falling back to the original path from strdup() or by logging an error and handling the null case appropriately.

Suggested change
_profiler_name = realpath(info.dli_fname, nullptr);
char* resolved_path = realpath(info.dli_fname, nullptr);
if (resolved_path != nullptr) {
_profiler_name = resolved_path;
} else {
// Fall back to the original path if realpath fails
_profiler_name = strdup(info.dli_fname);
}

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI commented Jan 21, 2026

@jbachorik I've opened a new pull request, #335, to work on those changes. Once the pull request is ready, I'll request review from you.

@zhengyu123 zhengyu123 merged commit 43c671b into main Jan 21, 2026
102 checks passed
@zhengyu123 zhengyu123 deleted the zgu/lib-path branch January 21, 2026 15:59
@github-actions github-actions bot added this to the 1.37.0 milestone Jan 21, 2026
jbachorik pushed a commit that referenced this pull request Jan 21, 2026
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 this pull request may close these issues.

3 participants