Skip to content

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

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

adrian-prantl
Copy link

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)

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)
@adrian-prantl adrian-prantl merged commit 0cac2ba into swiftlang:next Oct 6, 2021
@dreampiggy
Copy link

dreampiggy commented Apr 8, 2022

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 po/expr command to view the variable in Xcode version 13.3 (13E113) using built-in toolchain. Showing:

po self
Couldn't realize type of self

After some investigation by compiling LLDB.framework to replace the built-in one, I found that the issue is related to this change. 😱

Log

I compared with Swift 5.5.2(Xcode 13.2) and Swift 5.6(Xcode 13.3) toolchain, using the log to record lldb tasks.

  • Swift 5.6

I found that the SwiftASTContextForExpressions's Framework search paths and Import search paths is always 0. This cause that the debugging module (LarkChat) failed to parseAST and finally fallback on the ClangModuleLoader with dwarf. Which is not designed behavior.

There are also hint in log output:

SwiftASTContextForExpressions::GetModule("LarkChat")
SwiftASTContextForExpressions::LoadOneModule() -- "Imported" module LarkChat via SwiftDWARFImporterDelegate (no Swift AST or Clang module found)

See: lark-5.6-default-expressions.txt

  • Swift 5.5.2

However, when on Swift 5.5.2, the Framework search paths and Import search paths is not empty and has exactly where my built product's swiftmodule in. So my debug module (LarkChat) can succfully load the swiftmodule using MemoryBufferSerializedModuleLoader to parseAST.

See: lark-5.5.2-default-expressions.txt

Code Changes

When in Swift 5.5.2, the code here in left, will copy the SwiftASTContextForModule("MainExecutable").GetCompilerInvocation()'s search paths, into the new created SwiftASTContextForExpressions.GetCompilerInvocation().

    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 DeserializeAllCompilerFlags logic, I think it's not equal to previous 5.5.2's behavior. In 5.5.2, the MainExecutable will get call with lldb::SwiftASTContext::RegisterSectionModules to grab each section of machO and loop to use MemoryBufferSerializedModuleLoader to parseAST.

Note: Even in 5.6's SwiftASTContextForModule("MainExecutable"), it can successfully generate the full compiler search paths.

1

However, the new DeserializeAllCompilerFlags will fail to generate the full compiler search paths, which result 0 for our testing Lark app.

截屏2022-04-08 下午6 25 42

Solution

I 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. 🙏

@adrian-prantl
Copy link
Author

Thanks for your detailed write-up. I some time to read it more thoroughly. Does #4083 address this issue (at least partially)?

@adrian-prantl
Copy link
Author

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 ImportSearchPaths.

@adrian-prantl
Copy link
Author

Does my testcase in #4203 capture the entire behavior you are describing?

@dreampiggy
Copy link

dreampiggy commented Apr 9, 2022

Thanks for your detailed write-up. I some time to read it more thoroughly. Does #4083 address this issue (at least partially)?

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

@dreampiggy
Copy link

dreampiggy commented Apr 9, 2022

After pick that #4083, I found the po self work now.

The extra logic there to read SymbolFile to retrive the TypeSystem and Framework Search Paths seems work for my case.

See screenshot, the 185 Framework Search Paths is all here :

截屏2022-04-09 下午5 44 06

The final po self output show the designed result I want.

截屏2022-04-09 下午5 45 00

However, the Import Search Paths seems still not been kept. Which does not effect my use case, but may be useful in more general cases ?

1649497464.750006914 SwiftASTContextForExpressions::LogConfiguration() --   Runtime library paths        : (2 items)
1649497464.750032902 SwiftASTContextForExpressions::LogConfiguration() --   Runtime library import paths : (2 items)
1649497464.750058889 SwiftASTContextForExpressions::LogConfiguration() --   Framework search paths       : (185 items)
1649497464.751884937 SwiftASTContextForExpressions::LogConfiguration() --   Import search paths          : (0 items)
1649497464.751894951 SwiftASTContextForExpressions::LogConfiguration() --   Extra clang arguments        : (461 items)

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 Import Search Paths in grabbed from compiler in some cases, but I need to debug more step in.

@adrian-prantl
Copy link
Author

@dreampiggy Could you help me confirm/disprove a theory?
As a first step, we need to identify at least one Swift module that cannot be found because of the missing import search paths. Under normal circumstances, all Swift modules built by your application should be registered in the debug info. This happens when the binary or dylib is linked, using the -add_ast_path linker flag. I'm suspecting that there is no such flag being passed to the linker for the missing Swift module. One explanation for why this might happen is if the module is being built as part of a static archive (*.a) or framework. Because static archives are no linked binaries the -add_ast_path flag needs to be added to linker invocation of the application or dylib that links against the static archive.
If you can confirm that the missing Swift module comes from a static archive or framework, this can be fixed by adding -add-ast-path /path/to/the.swiftmodule to the linker flags or by adding the -I/-F search paths to the compiler flags.

@dreampiggy
Copy link

dreampiggy commented Apr 13, 2022

For my test app. There is only one add_ast_path for the main executable during linking.

-add_ast_path -Xlinker /Users/lizhuoli/Library/Developer/Xcode/DerivedData/Lark-ealzgdgullarlicdikunwgspcena/Build/Intermediates.noindex/Lark.build/Debug-iphonesimulator/LarkMessengerDemo.build/Objects-normal/x86_64/Messenger.swiftmodule

However, the actually missing module LarkChat does not get passed into the add_ast_path, but linked through:

-framework LarkChat
-F/Users/lizhuoli/Library/Developer/Xcode/DerivedData/Lark-ealzgdgullarlicdikunwgspcena/Build/Products/Debug-iphonesimulator/Pods-LarkMessengerDemo-SystemFramework

The Pods-LarkMessengerDemo-SystemFramework folder contains the symbol link to the built framework:

cd Pods-LarkMessengerDemo-SystemFramework
ls -l

LarkChat.framework -> ../LarkChat/LarkChat.framework

file LarkChat.framework/Modules/LarkChat.swiftmodule/x86_64-apple-ios-simulator.swiftmodule
LarkChat.framework/Modules/LarkChat.swiftmodule/x86_64-apple-ios-simulator.swiftmodule: data

Or does this symbol link cause the issue ?

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

@dreampiggy
Copy link

Update: I checked using new Xcode iOS App demo, for each dependent iOS static framework, it does not get passed extra -add_ast_path well. Seems a build system issue ?

image

@dreampiggy
Copy link

dreampiggy commented Apr 14, 2022

Hi.

Under normal circumstances, all Swift modules built by your application should be registered in the debug info

I think the debug info information is stored. However, during debugging, I found one question about the order for SwiftASTContextForExpressions to loadModule.

I set a breakpoint at LLDB to see the differences, here are the result

SwiftASTContextForModules

SwiftASTContext::CreateInstance(lldb::LanguageType language, Module &module, TypeSystemSwiftTypeRef *typeref_typesystem, Target *target, bool fallback)

  • RegisterSectionModules("Messenger") // main executable
  • LoadOneModule("Swift") // stdlib
  • LoadOneModule("Foobar1")
  • LoadOneModule("Foobar2")
    ...
  • LoadOneModule("LarkChat") // debugging module
    ...
  • LoadOneModule("Foobar100")

This order will ensure the Framework Search Path/Import Search Path get ready.

SwiftASTContextForExpressions

lldb::TypeSystemSP SwiftASTContext::CreateInstance(lldb::LanguageType language, Target &target, const char *extra_options)

  • LoadOneModule("Swift") // stdlib
  • PerformCompileUnitImports("LarkChat") // called because of po self
  • LoadOneModule("LarkChat") // debugging module

This order since no main executable been parsed, the Framework Search Path/Import Search Path are empty so LarkChat load failed. (unless you copies the SwiftASTContextForModule("Messenger") from main executable before that happends)

Question ?

The ForExpressions initialization use does not read the main executable section and parse the paths using Swift compiler. Is this by design or can this cause issue ?

@adrian-prantl
Copy link
Author

With this patch effectively reverted, SwiftASTContextForExpressions inherits the framework search paths discovered during the SwiftASTContextForModules initialization again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants