Skip to content

Fix for overwriting LD_PRELOAD if it contains libscalene.so and other values #925

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

elesiuta
Copy link

@elesiuta elesiuta commented Jun 9, 2025

It looks like in some cases setup_preload gets called more than once, so it was adding libscalene.so to LD_PRELOAD the first time then later replacing LD_PRELOAD with just libscalene.so, this fixes that case.

@emeryberger emeryberger requested a review from Copilot June 9, 2025 16:51
Copy link
Contributor

@Copilot 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 ensures that if setup_preload is called multiple times, existing LD_PRELOAD entries aren’t overwritten when they already include libscalene.so.

  • Adds an elif branch to preserve the full LD_PRELOAD when it already contains libscalene.so
  • Retains existing behavior of prepending libscalene.so only when it’s absent
Comments suppressed due to low confidence (1)

scalene/scalene_preload.py:74

  • Add unit tests for the branch where LD_PRELOAD already includes libscalene.so to verify that existing values are preserved and not accidentally overwritten.
elif "LD_PRELOAD" in os.environ and new_ld_preload in os.environ["LD_PRELOAD"]:

- split on : as per plasma-umass#925 (comment)
- simplify conditions
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.

1 participant