lldb-tools
Win32: Change incoming path slashes to backslashes in ResolvePath
This PR adds a small fix to improve handling of absolute Unix-style paths in Win32 versions of lldb-mi.
Some details about the issue and the suggested fix:
When using lldb-mi to remotely debug a binary on a Linux system, I noticed frequent long stalls (> 10 seconds) when pausing execution or hitting a breakpoint. Further investigation revealed CMICmnLLDBDebugSessionInfo::ResolvePath() as the affected function; the delays were caused by waiting for a call to access() to complete. The path being tested started with two backslashes, and was thus interpreted as an UNC path to a nonexistent network server.
The suggested fix is to switch forward slashes to backslashes in the path to resolve, so the rest of the function will be able to split the path correctly. The reason for the double backslash seems to have been that ResolvePath treats the whole Unix-style path as one element, prepends a backslash, and then switches the slashes (turning the first slash of the original path into the second backslash).
Please let me know in case this is not an appropriate fix for the issue at hand.
Would be great if we could replace all this logic with a call to SBFileSpec::ResolvePath.
Would be great if we could replace all this logic with a call to SBFileSpec::ResolvePath.
I'm finally able to come back to this issue.
Switching to SBFileSpec::ResolvePath would change the behavior of this function:
- SBFileSpec::ResolvePath's is specific to the host platform (it hits platform-specific functionality down in llvm::sys::fs). This may lead to problems in cross-platform remote debugging scenarios, when the path format of the debuggee's debug info differs from the debugger's native format.
- The current implementation probes for incrementally larger portions of the debugee's file spec relative to the current working directory, which SBFileSpec::ResolvePath doesn't do.
I don't know much about who might rely on the current behavior, so I don't feel comfortable to implement such a change myself.
Happy to discuss, but at the moment, I would prefer if we could stay with a small-impact fix.
I think this change has more benefits than drawbacks, so I am inclined to merge this PR.
BTW, can you fix the warnings in the build step of the Windows CI? For example:
D:\a\lldb-mi\lldb-mi\llvm-inst\include\lldb/API/SBError.h(99): warning C4251: 'lldb::SBError::m_opaque_up': class 'std::unique_ptr<lldb_private::Status,std::default_delete<lldb_private::Status>>' needs to have dll-interface to be used by clients of class 'lldb::SBError'
D:\a\lldb-mi\lldb-mi\llvm-inst\include\lldb/API/SBError.h(99): note: see declaration of 'std::unique_ptr<lldb_private::Status,std::default_delete<lldb_private::Status>>'
D:\a\lldb-mi\lldb-mi\llvm-inst\include\lldb/API/SBPlatform.h(184): warning C4251: 'lldb::SBPlatform::m_opaque_sp': class 'std::shared_ptr<lldb_private::Platform>' needs to have dll-interface to be used by clients of class 'lldb::SBPlatform'
D:\a\lldb-mi\lldb-mi\llvm-inst\include\lldb/lldb-forward.h(366): note: see declaration of 'std::shared_ptr<lldb_private::Platform>'
