[all-commits] [llvm/llvm-project] 835990: [lldb][Module] Document ModuleList::ForEach and as...

[Michael137 via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 83599000e1f4b30d93b8f4509011b9b68d722835
      https://github.com/llvm/llvm-project/commit/83599000e1f4b30d93b8f4509011b9b68d722835
  Author: Michael Buch <michaelbuch12 at gmail.com>
  Date:   2022-12-02 (Fri, 02 Dec 2022)

  Changed paths:
    M lldb/include/lldb/Core/ModuleList.h
    M lldb/source/Core/ModuleList.cpp

  Log Message:
  -----------
  [lldb][Module] Document ModuleList::ForEach and assert nullness

Currently all callsites already assume the pointer is non-null.
This patch just asserts this assumption.

This is practically enforced by `ModuleList::Append`
which won't add `nullptr`s to `m_modules`.

Differential Revision: https://reviews.llvm.org/D139082


  Commit: 5941858efdca72425c29bf043d3ec84e7fec6f62
      https://github.com/llvm/llvm-project/commit/5941858efdca72425c29bf043d3ec84e7fec6f62
  Author: Michael Buch <michaelbuch12 at gmail.com>
  Date:   2022-12-02 (Fri, 02 Dec 2022)

  Changed paths:
    M lldb/include/lldb/Core/ModuleList.h
    M lldb/source/Core/ModuleList.cpp

  Log Message:
  -----------
  [lldb][Module][NFC] Add ModuleList::AnyOf

Differential Revision: https://reviews.llvm.org/D139083


  Commit: 4df11394a10b3b15d2fb9bde8b831cf68785aa45
      https://github.com/llvm/llvm-project/commit/4df11394a10b3b15d2fb9bde8b831cf68785aa45
  Author: Michael Buch <michaelbuch12 at gmail.com>
  Date:   2022-12-02 (Fri, 02 Dec 2022)

  Changed paths:
    M lldb/source/Target/Target.cpp
    A lldb/test/API/functionalities/rerun_and_expr/Makefile
    A lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
    A lldb/test/API/functionalities/rerun_and_expr/main.cpp
    A lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp
    A lldb/test/API/functionalities/rerun_and_expr_dylib/Makefile
    A lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
    A lldb/test/API/functionalities/rerun_and_expr_dylib/lib.cpp
    A lldb/test/API/functionalities/rerun_and_expr_dylib/main.cpp
    A lldb/test/API/functionalities/rerun_and_expr_dylib/rebuild.cpp

  Log Message:
  -----------
  [lldb][Target] Flush the scratch TypeSystem when owning lldb_private::Module gets unloaded

**Summary**

This patch addresses #59128, where LLDB would crash when evaluating
importing a type that has been imported before into the same target.
The proposed solution is to clear the scratch AST (and associated
persistent variables, `ClangASTImporter`, etc.) whenever a module that
could've owned one of the stale `TypeSystem`s gets unloaded/destroyed.

Details:
1. The first time we evaluate the expression we import the decl for Foo into the Targets scratch AST
   context (lives in m_scratch_type_system_map). During this process we also create a ClangASTImporter
   that lives in the ClangPersistentVariables::m_ast_importer_sp. This importer has decl tracking
   structures which reference the source AST that the decl got imported from. This importer also gets
   re-used for all calls to DeportType (which we use to copy the final decl into the Targets scratch AST).
2. Rebuilding the executable triggers a tear-down of the Module that was backing the ASTContext that
   we originally got the Foo decl from (which lived in the Module::m_type_system_map). However, the Target’s scratch AST lives on.
3. Re-running the same expression will now create a new ASTImporterDelegate where the destination TranslationUnitDecl is
   the same as the one from step (1).
4. When importing the new Foo decl we first try to find it in the destination DeclContext, which happens to be
   the scratch destination TranslationUnitDecl. The `Foo` decl exists in this context since we copied it into
   the scratch AST in the first run. The ASTImporter then queries LLDB for the origin of that decl. Using the
   same persistent variable ClangASTImporter we claim the decl has an origin in the AST context that got torn
   down with the Module. This faulty origin leads to a use-after-free.

**Testing**

- Added API test

Differential Revision: https://reviews.llvm.org/D138724


Compare: https://github.com/llvm/llvm-project/compare/a8a376cbc996...4df11394a10b