-
Notifications
You must be signed in to change notification settings - Fork 344
Deserialize search path options in ProcessModule (NFCi) #3353
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
Conversation
That patch is one step towards removing the need for a separate SwiftASTContext per lldb::Module. It moves the parsing of the headers of modules found in Swift AST sections directly into ProcessModule() since all that ProcessModule() needs is the search path options from the serialized swift::CompilerInvocation in the module. Right now this means that the compiler invocations are deserialized twice, but this should be a cheap operation. rdar://81717792 (cherry picked from commit 70d2ba0)
2216ee2
to
82c40d0
Compare
Hello. @adrian-prantl Sorry to borther you. I'm the developer in ByteDance. Recently we found that our heavily-Swift based iOS Application Lark can not use the
After some investigation by compiling LLDB.framework to replace the built-in one, I found that the issue is related to this change. 😱 LogI compared with Swift 5.5.2(Xcode 13.2) and Swift 5.6(Xcode 13.3) toolchain, using the log to record lldb tasks.
I found that the SwiftASTContextForExpressions's There are also hint in log output:
See: lark-5.6-default-expressions.txt
However, when on Swift 5.5.2, the See: lark-5.5.2-default-expressions.txt Code ChangesWhen in Swift 5.5.2, the code here in left, will copy the const auto &opts = ast_context->GetSearchPathOptions(); // this is from SwiftASTContextForModule("MainExecutable") and copy to new SwiftASTContextForExpressions
module_search_paths.insert(module_search_paths.end(),
opts.ImportSearchPaths.begin(),
opts.ImportSearchPaths.end()); However, the code after this change, will use the MainExecutable to deserialize again, and generate new Search Path options. if (DeserializeAllCompilerFlags(invocation, *module_sp, m_description, errs,
got_serialized_options,
found_swift_modules)) {}
const auto &opts = invocation.getSearchPathOptions(); // this invocation is a temp-created one, not equal to SwiftASTContextForModule("MainExecutable")
module_search_paths.insert(module_search_paths.end(),
opts.ImportSearchPaths.begin(),
opts.ImportSearchPaths.end()); Why?I read the Note: Even in 5.6's SwiftASTContextForModule("MainExecutable"), it can successfully generate the full compiler search paths. However, the new SolutionI think we'd better to roll back this logic. Or use the robust other way to parse the search path (which match previous behavior to user) Is this cause anything issue to copy the previous parsed MainExecutable's search paths to the new SwiftASTContextForExpressions? 🤔️ Since I'm not in Apple and I can not get the radar system to know what happens to that rdar://81717792. If you're interested in this regression, please feel free to contact me via email ([email protected] or [email protected]). Thank you in advance. 🙏 |
Thanks for your detailed write-up. I some time to read it more thoroughly. Does #4083 address this issue (at least partially)? |
To answer my own question: No, because it doesn't address |
Does my testcase in #4203 capture the entire behavior you are describing? |
Thanks for your reply. 🙏 I'll cherry-pick that commit and check for the behavior again. For my case, although both Framework Search Paths and Import Search Paths are 0, the only matter is the Framework Search Paths since my debugging module is in one iOS static framework built on my local machine. I assume this is something related to that debugging module I use, so I'll log the build options and provide for you as well, soon. -- UPDATE build log provided in that #4203 comment |
After pick that #4083, I found the The extra logic there to read See screenshot, the 185 Framework Search Paths is all here : The final However, the
If you need more information, I can provide here for more investigation. 😼 Since I'm new to debugger codebase and Swift compiler. I guess that |
@dreampiggy Could you help me confirm/disprove a theory? |
For my test app. There is only one
However, the actually missing module
The
Tested that even without symbol link and pass each framework with their own search path, the Xcode 13.3.0's lldb still not work. Have to use the patches from #4083 |
Hi.
I think the debug info information is stored. However, during debugging, I found one question about the order for I set a breakpoint at LLDB to see the differences, here are the result SwiftASTContextForModules
This order will ensure the Framework Search Path/Import Search Path get ready. SwiftASTContextForExpressions
This order since no main executable been parsed, the Framework Search Path/Import Search Path are empty so Question ?The |
With this patch effectively reverted, SwiftASTContextForExpressions inherits the framework search paths discovered during the SwiftASTContextForModules initialization again. |
That patch is one step towards removing the need for a separate
SwiftASTContext per lldb::Module. It moves the parsing of the headers
of modules found in Swift AST sections directly into ProcessModule()
since all that ProcessModule() needs is the search path options from
the serialized swift::CompilerInvocation in the module.
Right now this means that the compiler invocations are deserialized
twice, but this should be a cheap operation.
rdar://81717792
(cherry picked from commit a4c2379b90d4365ebde42066b186eeea0e85ffc0)