Page MenuHomePhabricator

[lldb] Fix a crash in PlatformAppleSimulator::GetCoreSimulatorPath when Xcode developer directory can't be found

Authored by teemperor on Jun 2 2020, 6:16 AM.



PlatformAppleSimulator::GetCoreSimulatorPath currently checks if m_core_simulator_framework_path
wasn't set yet and then tries to calculate its actual value. However, if GetXcodeDeveloperDirectory
returns an invalid FileSpec, m_core_simulator_framework_path is never assigned a value which causes
that the return m_core_simulator_framework_path.getValue(); at the end of the function will
trigger an assert.

This patch just assigns an invalid FileSpec to m_core_simulator_framework_path which seems what
the calling code in PlatformAppleSimulator::LoadCoreSimulator expects as an error value.

I assume this can be reproduces on machines that don't have an Xcode installation, but this patch is
mostly based on this backtrace I received from someone else that tried to run the test suite:

Assertion failed: (hasVal), function getValue, file llvm/include/llvm/ADT/Optional.h, line 73.
3   libsystem_c.dylib             	0x00007fff682a1ac6 __assert_rtn + 314
4   liblldb.11.0.0git.dylib       	0x000000010b835931 PlatformAppleSimulator::GetCoreSimulatorPath() (.cold.1) + 33
5   liblldb.11.0.0git.dylib       	0x0000000107e92f11 PlatformAppleSimulator::GetCoreSimulatorPath() + 369
6   liblldb.11.0.0git.dylib       	0x0000000107e9383e void std::__1::__call_once_proxy<std::__1::tuple<PlatformAppleSimulator::LoadCoreSimulator()::$_1&&> >(void*) + 30
7   libc++.1.dylib                	0x00007fff654d5bea std::__1::__call_once(unsigned long volatile&, void*, void (*)(void*)) + 139
8   liblldb.11.0.0git.dylib       	0x0000000107e92019 PlatformAppleSimulator::LaunchProcess(lldb_private::ProcessLaunchInfo&) + 89
9   liblldb.11.0.0git.dylib       	0x0000000107e92be5 PlatformAppleSimulator::DebugProcess(lldb_private::ProcessLaunchInfo&, lldb_private::Debugger&, lldb_private::Target*, lldb_private::Status&) + 101
10  liblldb.11.0.0git.dylib       	0x0000000107cb044d lldb_private::Target::Launch(lldb_private::ProcessLaunchInfo&, lldb_private::Stream*) + 669
11  liblldb.11.0.0git.dylib       	0x000000010792c9c5 lldb::SBTarget::Launch(lldb::SBLaunchInfo&, lldb::SBError&) + 1109
12  liblldb.11.0.0git.dylib       	0x0000000107a92acd _wrap_SBTarget_Launch(_object*, _object*) + 477
13  org.python.python             	0x000000010681076f PyCFunction_Call + 321
14  org.python.python             	0x000000010689ee12 _PyEval_EvalFrameDefault + 7738

Diff Detail

Event Timeline

teemperor created this revision.Jun 2 2020, 6:16 AM
JDevlieghere added inline comments.Jun 2 2020, 11:46 AM

Should we make m_core_simulator_framework_path a plain FileSpec instead of an optional instead?

teemperor marked an inline comment as done.Jun 8 2020, 7:27 AM
teemperor added inline comments.

We still need to keep track of whether we already tried do calculate the path before, so with a plain FileSpec we would also need some other member to track if we actually tried to calculate the directory before. FWIW, all this should anyway only be temporary until we get C++17 and we can make our own nice "LazyMember" template (see D51557 ).

JDevlieghere accepted this revision.Jun 8 2020, 2:15 PM

Ok, LGTM then.

This revision is now accepted and ready to land.Jun 8 2020, 2:15 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2020, 1:15 AM