This is an archive of the discontinued LLVM Phabricator instance.

[unittests] Preserve LD_LIBRARY_PATH in crash recovery test
ClosedPublic

Authored by miyuki on Sep 25 2020, 8:07 AM.

Details

Summary

We need to preserve the LD_LIBRARY_PATH environment variable when
spawning a child process (certain setups rely on non-standard paths
for e.g. libstdc++). In order to achieve this, set
LLVM_CRC_UNIXCRCRETURNCODE in the parent process instead of creating
the child's environment from scratch.

Diff Detail

Event Timeline

miyuki created this revision.Sep 25 2020, 8:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 25 2020, 8:07 AM
miyuki requested review of this revision.Sep 25 2020, 8:07 AM

Thanks for the fix @miyuki ! Just being curious, what happens without this patch? I'm also wondering why isn't it a problem in the ProgramTest.cpp, since it also spawn child processes? (I actually copied some of this code from there)

In our case, the child process fails to find the correct version of libstdc++ and exits with return code 1, ASSERT_EQ(-2, RetCode) fails.

MaskRay added inline comments.Sep 25 2020, 8:40 AM
llvm/unittests/Support/CrashRecoveryTest.cpp
178

Perhaps ExecuteAndWait should have a mode to append environment variables. In the absence of that feature,

llvm/utils/not/not.cpp

#ifdef _WIN32
    SetEnvironmentVariableA("...);
#else
    setenv(...);
#endif
aganea added inline comments.Sep 25 2020, 8:47 AM
llvm/unittests/Support/CrashRecoveryTest.cpp
178

Agreed. In that case @miyuki would this work?
Replacing:

std::vector<StringRef> EnvTable;
EnvTable.push_back("LLVM_CRC_UNIXCRCRETURNCODE=1");

by:

setenv("LLVM_CRC_UNIXCRCRETURNCODE", "1", 0);
miyuki updated this revision to Diff 294731.Sep 28 2020, 9:37 AM
miyuki edited the summary of this revision. (Show Details)

Use setenv in the parent process

miyuki marked an inline comment as done.Sep 28 2020, 9:37 AM
miyuki added inline comments.
llvm/unittests/Support/CrashRecoveryTest.cpp
178

Yes, this works in our environment.

aganea accepted this revision.Sep 28 2020, 9:43 AM

Great! Thanks for the fix!

This revision is now accepted and ready to land.Sep 28 2020, 9:43 AM
This revision was landed with ongoing or failed builds.Sep 28 2020, 9:46 AM
This revision was automatically updated to reflect the committed changes.
miyuki marked an inline comment as done.