Add preload of jemalloc library to spectre script #4550
Conversation
| @@ -33,6 +33,10 @@ configure_or_symlink_py_file( | |||
| ) | |||
| # Also link the main entry point to bin/ | |||
| set(PYTHON_SCRIPT_LOCATION "spectre") | |||
| set(JEMALLOC_PRELOAD "") | |||
| if(BUILD_PYTHON_BINDINGS AND "${JEMALLOC_LIB_TYPE}" STREQUAL SHARED) | |||
| set(JEMALLOC_PRELOAD "LD_PRELOAD=${JEMALLOC_LIBRARIES}") | |||
There was a problem hiding this comment.
Not for this PR, but my general education... @wthrowe would there be issues here if someone did LD_PRELOAD=... ./bin/specter ... where either our or the user's LD_PRELOAD is overwritten/ignored?
There was a problem hiding this comment.
This will overwrite any preexisting value, so if anyone is relying on that it could cause problems. Properly adding to these colon-separated values is annoying. I think the most correct thing would be LD_PRELOAD=${LD_PRELOAD}${LD_PRELOAD:+:}new_lib. That middle thing can be read as "Look at LD_PRELOAD, and, treating unset as empty (:), if it is not empty then substitute (+) the string :."
I'm not sure whether prepending or appending is correct here. The above appends, prepending can be done by reversing the order of the three sections.
There was a problem hiding this comment.
squashed a change to append if LD_PRELOAD is defined
| @@ -33,6 +33,14 @@ configure_or_symlink_py_file( | |||
| ) | |||
| # Also link the main entry point to bin/ | |||
| set(PYTHON_SCRIPT_LOCATION "spectre") | |||
| set(JEMALLOC_PRELOAD "") | |||
| if(BUILD_PYTHON_BINDINGS AND "${JEMALLOC_LIB_TYPE}" STREQUAL SHARED) | |||
| if(DEFINED ENV{LD_PRELOAD}) | |||
There was a problem hiding this comment.
I'd be OK with the old version that didn't respect LD_PRELOAD at all, but if you are going to respect it you need to use the value in the script's environment, not cmake's environmment.
There was a problem hiding this comment.
ah okay; squashed and pushed again...
Proposed changes
Upgrade instructions
Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixornew featureif appropriate.Further comments