-
Notifications
You must be signed in to change notification settings - Fork 344
Support debugging Swift dylibs that were loaded after SwiftASTContext #3311
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
Support debugging Swift dylibs that were loaded after SwiftASTContext #3311
Conversation
@swift-ci test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent
module_sp->GetFileSpec().GetPath().c_str()); | ||
} | ||
std::vector<std::string> module_names; | ||
RegisterSectionModules(*module_sp, module_names); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be done for all modules? Or would it be already done for most modules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fine. I am a little bothered about the side-effect-y nature of how you get the context rebuilt. But that's more formal, and if you don't think it's likely to be a problem, I defer to your judgement.
// We cannot reconfigure ClangImporter after its | ||
// creation. Instead poison the SwiftASTContext so it gets | ||
// recreated. | ||
m_fatal_errors.SetErrorStringWithFormat( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth adding an API on SwiftASTContext (ClearClangImporter or something) to do this? I worry that having a fatal error pending on the context could get you into trouble if some other subsystem checks for errors but isn't prepared to reconstitute the context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only way to clear clangimporter is to create a new Swift compiler instance (= a new SwiftASTContext). The recreation is transparently handled by Target's GetScratchContext function.
… was initialized. This patch extends the ModulesDidLoad callback to register .swiftast sections in the new dylib and posons the context if necessary to force a reinitialization. rdar://81135636
1856499
to
1348ae3
Compare
… was initialized.
This patch extends the ModulesDidLoad callback to register .swiftast
sections in the new dylib and posons the context if necessary to force
a reinitialization.
rdar://81135636