[Lldb-commits] [PATCH] D152861: Clear non-addressable bits from fp/sp/lr/pc values in RegisterContextUnwind

2023-06-15 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

TIL what gcore is. One can save a corefile from within lldb too, I assume 
you've checked what that does (and with this change, it wouldn't matter either 
way).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152861/new/

https://reviews.llvm.org/D152861

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D132510: [RISCV][LLDB] Add initial SysV ABI support

2023-06-15 Thread baoyuxu via Phabricator via lldb-commits
baoyuxu added a comment.

@kasper81, I just tried this patch with lastest repo in github and can bulid 
patched lldb. But when I try to remote debug with riscv qemu, it still get all 
1s frame. Could you please see what I'm doing wrong?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132510/new/

https://reviews.llvm.org/D132510

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152516: [lldb][AArch64] Add thread local storage tpidr register

2023-06-15 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid accepted this revision.
omjavaid added a comment.
This revision is now accepted and ready to land.

This looks good to me, just a minor nit above. I have not run the test myself.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:286
 src = (uint8_t *)GetMTEControl() + offset;
+  } else if (IsTLS(reg)) {
+error = ReadTLSTPIDR();

Do we need to read this register on every stop ? similar to SVE vg? May be 
consider moving this else if before SVE which is still optionally available in 
most cases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152516/new/

https://reviews.llvm.org/D152516

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152516: [lldb][AArch64] Add thread local storage tpidr register

2023-06-15 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett planned changes to this revision.
DavidSpickett added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:286
 src = (uint8_t *)GetMTEControl() + offset;
+  } else if (IsTLS(reg)) {
+error = ReadTLSTPIDR();

omjavaid wrote:
> Do we need to read this register on every stop ? similar to SVE vg? May be 
> consider moving this else if before SVE which is still optionally available 
> in most cases.
What is the criteria for "need to"? I assume for vg we need it because that 
tells us the vector length, so we can handle SVE changing per stop.

In this case this register is always the same size and only the value of it can 
be changed. So I think the answer is no we don't need to read it on every stop. 
I will move it.

(later if we have SME there will be a second register, but if you have SME it's 
always there it doesn't come and go)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152516/new/

https://reviews.llvm.org/D152516

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 74113a4 - [lldb] Fix build error after 7bca6f45

2023-06-15 Thread Jan Svoboda via lldb-commits

Author: Jan Svoboda
Date: 2023-06-15T11:59:47+02:00
New Revision: 74113a4150d683c9478331ae91018ab9092a634d

URL: 
https://github.com/llvm/llvm-project/commit/74113a4150d683c9478331ae91018ab9092a634d
DIFF: 
https://github.com/llvm/llvm-project/commit/74113a4150d683c9478331ae91018ab9092a634d.diff

LOG: [lldb] Fix build error after 7bca6f45

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
index 0af5de4702df6..5ce0d35378230 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -329,8 +329,8 @@ bool ClangModulesDeclVendorImpl::AddModule(const 
SourceModule &module,
 
   bool is_system = true;
   bool is_framework = false;
-  auto dir =
-  HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
+  auto dir = HS.getFileMgr().getOptionalDirectoryRef(
+  module.search_path.GetStringRef());
   if (!dir)
 return error();
   auto file = HS.lookupModuleMapFile(*dir, is_framework);



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153043: [lldb] Fix handling of cfi_restore in the unwinder

2023-06-15 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin created this revision.
jarin added a reviewer: labath.
jarin added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jarin requested review of this revision.
Herald added a subscriber: lldb-commits.

Currently, lldb's unwinder ignores cfi_restore opcodes for registers
that are not set in the first row of the unwinding info. This prevents
unwinding of failed assertion in Chrome/v8 (https://github.com/v8/v8).
The attached test is an x64 copy of v8's function that failed to unwind
correctly.

This patch changes handling of cfi_restore to reset the location if
the first unwind table row does not map the restored register.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153043

Files:
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
  lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test


Index: lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
@@ -0,0 +1,21 @@
+# Test restoring of register values.
+
+# UNSUPPORTED: system-windows
+# REQUIRES: target-x86_64, native
+
+# RUN: %clang_host %p/Inputs/call-asm.c 
%p/Inputs/eh-frame-dwarf-unwind-abort.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+process launch
+# CHECK: stop reason = signal SIGTRAP
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`asm_main + 23
+# CHECK: frame #1: {{.*}}`main + {{.*}}
+
+target modules show-unwind -n asm_main
+# CHECK: eh_frame UnwindPlan:
+# CHECK: row[0]:0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK: row[1]:   14: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[2]:   17: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[3]:   22: CFA=rsp +8 => rip=[CFA-8]
Index: lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
@@ -0,0 +1,25 @@
+.text
+.globl  asm_main
+asm_main:
+.cfi_startproc
+cmpb $0x0, g_hard_abort(%rip)
+jne .L
+
+pushq   %rbp
+.cfi_def_cfa_offset 16
+.cfi_offset 6, -16
+movq%rsp, %rbp
+.cfi_def_cfa_register 6
+callq   abort
+.L:
+.cfi_def_cfa 7, 8
+.cfi_restore 6
+int3
+ud2
+.cfi_endproc
+
+   .data
+   .globl  g_hard_abort
+g_hard_abort:
+   .byte   1
+   .size   g_hard_abort, 1
\ No newline at end of file
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -674,6 +674,8 @@
   unwind_plan.GetRowAtIndex(0)->GetRegisterInfo(reg_num,
 reg_location))
 row->SetRegisterInfo(reg_num, reg_location);
+  else
+row->RemoveRegisterInfo(reg_num);
   break;
 }
 }


Index: lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
@@ -0,0 +1,21 @@
+# Test restoring of register values.
+
+# UNSUPPORTED: system-windows
+# REQUIRES: target-x86_64, native
+
+# RUN: %clang_host %p/Inputs/call-asm.c %p/Inputs/eh-frame-dwarf-unwind-abort.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+process launch
+# CHECK: stop reason = signal SIGTRAP
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`asm_main + 23
+# CHECK: frame #1: {{.*}}`main + {{.*}}
+
+target modules show-unwind -n asm_main
+# CHECK: eh_frame UnwindPlan:
+# CHECK: row[0]:0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK: row[1]:   14: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[2]:   17: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[3]:   22: CFA=rsp +8 => rip=[CFA-8]
Index: lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
@@ -0,0 +1,25 @@
+.text
+.globl  asm_main
+asm_main:
+.cfi_startproc
+cmpb $0x0, g_hard_abort(%rip)
+jne .L
+
+pushq   %rbp
+.cfi_def_cfa_offset 16
+.cfi_offset 6, -16
+movq%rsp, %rbp
+.cfi_def_cfa_register 6
+callq   abort
+.L:
+.cfi_def_cfa 7, 8
+.cfi_restore 6
+int3
+ud2
+.cfi_endproc
+
+	.data
+	.globl  g_hard_abort
+g_hard_abort:
+	.byte   1
+	.size   g_hard_abort, 1
\ No newline at end of file
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Sy

[Lldb-commits] [PATCH] D153054: [lldb][NFCI] TypeSystemClang::GetTypeForIdentifier should take a StringRef

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: aprantl, Michael137, augusto2112, JDevlieghere.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This method just takes its ConstString parameter and gets a StringRef
out of it. Let's just pass in a StringRef directly.

This also cleans up some logic in the callers to be a little easier to
read and to avoid unnecessary ConstString creation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153054

Files:
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -252,31 +252,29 @@
 
   template 
   CompilerType
-  GetTypeForIdentifier(ConstString type_name,
+  GetTypeForIdentifier(llvm::StringRef type_name,
clang::DeclContext *decl_context = nullptr) {
 CompilerType compiler_type;
-
-if (type_name.GetLength()) {
-  clang::ASTContext &ast = getASTContext();
-  if (!decl_context)
-decl_context = ast.getTranslationUnitDecl();
-
-  clang::IdentifierInfo &myIdent = ast.Idents.get(type_name.GetCString());
-  clang::DeclarationName myName =
-  ast.DeclarationNames.getIdentifier(&myIdent);
-
-  clang::DeclContext::lookup_result result = decl_context->lookup(myName);
-
-  if (!result.empty()) {
-clang::NamedDecl *named_decl = *result.begin();
-if (const RecordDeclType *record_decl =
-llvm::dyn_cast(named_decl))
-  compiler_type =
-  CompilerType(weak_from_this(),
-   clang::QualType(record_decl->getTypeForDecl(), 0)
-   .getAsOpaquePtr());
-  }
-}
+if (type_name.empty())
+  return compiler_type;
+
+clang::ASTContext &ast = getASTContext();
+if (!decl_context)
+  decl_context = ast.getTranslationUnitDecl();
+
+clang::IdentifierInfo &myIdent = ast.Idents.get(type_name);
+clang::DeclarationName myName =
+ast.DeclarationNames.getIdentifier(&myIdent);
+clang::DeclContext::lookup_result result = decl_context->lookup(myName);
+if (result.empty())
+  return compiler_type;
+
+clang::NamedDecl *named_decl = *result.begin();
+if (const RecordDeclType *record_decl =
+llvm::dyn_cast(named_decl))
+  compiler_type = CompilerType(
+  weak_from_this(),
+  clang::QualType(record_decl->getTypeForDecl(), 0).getAsOpaquePtr());
 
 return compiler_type;
   }
Index: lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -407,8 +407,8 @@
 // symbols in PDB for types with const or volatile modifiers, but we need
 // to create only one declaration for them all.
 Type::ResolveState type_resolve_state;
-CompilerType clang_type = m_ast.GetTypeForIdentifier(
-ConstString(name), decl_context);
+CompilerType clang_type =
+m_ast.GetTypeForIdentifier(name, decl_context);
 if (!clang_type.IsValid()) {
   auto access = GetAccessibilityForUdt(*udt);
 
@@ -479,8 +479,8 @@
 uint64_t bytes = enum_type->getLength();
 
 // Check if such an enum already exists in the current context
-CompilerType ast_enum = m_ast.GetTypeForIdentifier(
-ConstString(name), decl_context);
+CompilerType ast_enum =
+m_ast.GetTypeForIdentifier(name, decl_context);
 if (!ast_enum.IsValid()) {
   auto underlying_type_up = enum_type->getUnderlyingType();
   if (!underlying_type_up)
@@ -557,8 +557,7 @@
 
 // Check if such a typedef already exists in the current context
 CompilerType ast_typedef =
-m_ast.GetTypeForIdentifier(ConstString(name),
-   decl_ctx);
+m_ast.GetTypeForIdentifier(name, decl_ctx);
 if (!ast_typedef.IsValid()) {
   CompilerType target_ast_type = target_type->GetFullCompilerType();
 
Index: lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
===
--- lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
+++ lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
@@ -48,7 +48,7 @@
   // See if we have made this type before and can reuse it.
   CompilerType fields_type =
   type_system->GetTypeForIdentifier(
-  ConstString(register_type_nam

[Lldb-commits] [lldb] 35b0b24 - [lldb] Introduce DynamicRegisterInfo::CreateFromDict

2023-06-15 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-06-15T10:51:17-07:00
New Revision: 35b0b244401aad5ca3a16c9e4d97a5892ca7592b

URL: 
https://github.com/llvm/llvm-project/commit/35b0b244401aad5ca3a16c9e4d97a5892ca7592b
DIFF: 
https://github.com/llvm/llvm-project/commit/35b0b244401aad5ca3a16c9e4d97a5892ca7592b.diff

LOG: [lldb] Introduce DynamicRegisterInfo::CreateFromDict

I want to add some error handling to DynamicRegisterInfo because there
are many operations that can fail and many of these operations do not
give meaningful information back to the caller.

To begin that process, I want to add a static method that is responsible
for creating a DynamicRegisterInfo from a StructuredData::Dictionary
(and ArchSpec). This is meant to replace the equivalent constructor
because constructors are ill-equipped to perform error handling.

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

Added: 


Modified: 
lldb/include/lldb/Target/DynamicRegisterInfo.h
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
lldb/source/Target/DynamicRegisterInfo.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/DynamicRegisterInfo.h 
b/lldb/include/lldb/Target/DynamicRegisterInfo.h
index 1b1df848315ce..22ad6335fe438 100644
--- a/lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ b/lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -46,8 +46,8 @@ class DynamicRegisterInfo {
 
   DynamicRegisterInfo() = default;
 
-  DynamicRegisterInfo(const lldb_private::StructuredData::Dictionary &dict,
-  const lldb_private::ArchSpec &arch);
+  static std::unique_ptr
+  Create(const StructuredData::Dictionary &dict, const ArchSpec &arch);
 
   virtual ~DynamicRegisterInfo() = default;
 

diff  --git 
a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp 
b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
index c22003f24f760..9560ae108f3e3 100644
--- a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
+++ b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
@@ -128,8 +128,9 @@ DynamicRegisterInfo 
*OperatingSystemPython::GetDynamicRegisterInfo() {
 if (!dictionary)
   return nullptr;
 
-m_register_info_up = std::make_unique(
+m_register_info_up = DynamicRegisterInfo::Create(
 *dictionary, m_process->GetTarget().GetArchitecture());
+assert(m_register_info_up);
 assert(m_register_info_up->GetNumRegisters() > 0);
 assert(m_register_info_up->GetNumRegisterSets() > 0);
   }

diff  --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp 
b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index ac707ffb21711..fa2ee723e093b 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -341,7 +341,7 @@ std::shared_ptr 
ScriptedThread::GetDynamicRegisterInfo() {
   LLVM_PRETTY_FUNCTION, "Failed to get scripted thread registers 
info.",
   error, LLDBLog::Thread);
 
-m_register_info_sp = std::make_shared(
+m_register_info_sp = DynamicRegisterInfo::Create(
 *reg_info, m_scripted_process.GetTarget().GetArchitecture());
   }
 

diff  --git a/lldb/source/Target/DynamicRegisterInfo.cpp 
b/lldb/source/Target/DynamicRegisterInfo.cpp
index 700619959f5c8..d577e20b3740c 100644
--- a/lldb/source/Target/DynamicRegisterInfo.cpp
+++ b/lldb/source/Target/DynamicRegisterInfo.cpp
@@ -20,10 +20,17 @@
 using namespace lldb;
 using namespace lldb_private;
 
-DynamicRegisterInfo::DynamicRegisterInfo(
-const lldb_private::StructuredData::Dictionary &dict,
-const lldb_private::ArchSpec &arch) {
-  SetRegisterInfo(dict, arch);
+std::unique_ptr
+DynamicRegisterInfo::Create(const StructuredData::Dictionary &dict,
+const ArchSpec &arch) {
+  auto dyn_reg_info = std::make_unique();
+  if (!dyn_reg_info)
+return nullptr;
+
+  if (dyn_reg_info->SetRegisterInfo(dict, arch) == 0)
+return nullptr;
+
+  return dyn_reg_info;
 }
 
 DynamicRegisterInfo::DynamicRegisterInfo(DynamicRegisterInfo &&info) {



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152594: [lldb] Introduce DynamicRegisterInfo::CreateFromDict

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG35b0b244401a: [lldb] Introduce 
DynamicRegisterInfo::CreateFromDict (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152594/new/

https://reviews.llvm.org/D152594

Files:
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Target/DynamicRegisterInfo.cpp


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -20,10 +20,17 @@
 using namespace lldb;
 using namespace lldb_private;
 
-DynamicRegisterInfo::DynamicRegisterInfo(
-const lldb_private::StructuredData::Dictionary &dict,
-const lldb_private::ArchSpec &arch) {
-  SetRegisterInfo(dict, arch);
+std::unique_ptr
+DynamicRegisterInfo::Create(const StructuredData::Dictionary &dict,
+const ArchSpec &arch) {
+  auto dyn_reg_info = std::make_unique();
+  if (!dyn_reg_info)
+return nullptr;
+
+  if (dyn_reg_info->SetRegisterInfo(dict, arch) == 0)
+return nullptr;
+
+  return dyn_reg_info;
 }
 
 DynamicRegisterInfo::DynamicRegisterInfo(DynamicRegisterInfo &&info) {
Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -341,7 +341,7 @@
   LLVM_PRETTY_FUNCTION, "Failed to get scripted thread registers 
info.",
   error, LLDBLog::Thread);
 
-m_register_info_sp = std::make_shared(
+m_register_info_sp = DynamicRegisterInfo::Create(
 *reg_info, m_scripted_process.GetTarget().GetArchitecture());
   }
 
Index: lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
===
--- lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
+++ lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
@@ -128,8 +128,9 @@
 if (!dictionary)
   return nullptr;
 
-m_register_info_up = std::make_unique(
+m_register_info_up = DynamicRegisterInfo::Create(
 *dictionary, m_process->GetTarget().GetArchitecture());
+assert(m_register_info_up);
 assert(m_register_info_up->GetNumRegisters() > 0);
 assert(m_register_info_up->GetNumRegisterSets() > 0);
   }
Index: lldb/include/lldb/Target/DynamicRegisterInfo.h
===
--- lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -46,8 +46,8 @@
 
   DynamicRegisterInfo() = default;
 
-  DynamicRegisterInfo(const lldb_private::StructuredData::Dictionary &dict,
-  const lldb_private::ArchSpec &arch);
+  static std::unique_ptr
+  Create(const StructuredData::Dictionary &dict, const ArchSpec &arch);
 
   virtual ~DynamicRegisterInfo() = default;
 


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -20,10 +20,17 @@
 using namespace lldb;
 using namespace lldb_private;
 
-DynamicRegisterInfo::DynamicRegisterInfo(
-const lldb_private::StructuredData::Dictionary &dict,
-const lldb_private::ArchSpec &arch) {
-  SetRegisterInfo(dict, arch);
+std::unique_ptr
+DynamicRegisterInfo::Create(const StructuredData::Dictionary &dict,
+const ArchSpec &arch) {
+  auto dyn_reg_info = std::make_unique();
+  if (!dyn_reg_info)
+return nullptr;
+
+  if (dyn_reg_info->SetRegisterInfo(dict, arch) == 0)
+return nullptr;
+
+  return dyn_reg_info;
 }
 
 DynamicRegisterInfo::DynamicRegisterInfo(DynamicRegisterInfo &&info) {
Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -341,7 +341,7 @@
   LLVM_PRETTY_FUNCTION, "Failed to get scripted thread registers info.",
   error, LLDBLog::Thread);
 
-m_register_info_sp = std::make_shared(
+m_register_info_sp = DynamicRegisterInfo::Create(
 *reg_info, m_scripted_process.GetTarget().GetArchitecture());
   }
 
Index: lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
===
--- lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
+++ lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
@@ -128,8 +128,9 @@
 if 

[Lldb-commits] [PATCH] D153054: [lldb][NFCI] TypeSystemClang::GetTypeForIdentifier should take a StringRef

2023-06-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:76
+
+  compiler_type = 
scratch_ts_sp->GetTypeForIdentifier(g_lldb_autogen_nspair);
+

why not directly pass in `"__lldb_autogen_nspair"` here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153054/new/

https://reviews.llvm.org/D153054

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153054: [lldb][NFCI] TypeSystemClang::GetTypeForIdentifier should take a StringRef

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:76
+
+  compiler_type = 
scratch_ts_sp->GetTypeForIdentifier(g_lldb_autogen_nspair);
+

aprantl wrote:
> why not directly pass in `"__lldb_autogen_nspair"` here?
It's used below as well, I didn't trust myself to be able to type it correctly 
twice in a row. :)
It's also nice that we can `constexpr` the llvm::StringRef and avoid computing 
the length at runtime, but that's a minor thing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153054/new/

https://reviews.llvm.org/D153054

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153060: lldb: do more than 1 kilobyte at a time to vastly increase binary sync speed

2023-06-15 Thread Russell Greene via Phabricator via lldb-commits
russelltg created this revision.
Herald added a project: All.
russelltg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

https://github.com/llvm/llvm-project/issues/62750

I setup a simple test with a large .so (~100MiB) that was only present on the 
target machine
but not present on the local machine, and ran a lldb server on the target and 
connectd to it.

LLDB properly downloads the file from the remote, but it does so at a very slow 
speed, even over a hardwired 1Gbps connection!

Increasing the buffer size for downloading these helps quite a bit.

Test setup:

  bash
  $ cat gen.py
  print('const char* hugeglobal = ')
  
  for _ in range(1000*500):
  print('  "' + '1234'*50 + '"')
  
  print(';')
  print('const char* mystring() { return hugeglobal; }')
  $ gen.py > huge.c
  $ mkdir libdir
  $ gcc -fPIC huge.c -Wl,-soname,libhuge.so -o libdir/libhuge.so -shared
  $ gcc test.c -L libdir -l huge -Wl,-rpath='$$ORIGIN' -o test
  $ rsync libdir remote:~/
  $ ssh remote bash -c "cd ~/libdir /llvm/buildr/bin/lldb-server platform 
--server --listen '*:1234'"

in another terminal

  bash
  $ rm -rf ~/.lldb # clear cache
  $ cat connect.lldb
  platform select remote-linux
  platform connect connect://10.0.0.14:1234
  file test
  b main
  r
  image list
  c
  q
  $ time /llvm/buildr/bin/lldb --source connect.lldb

Times with various buffer sizes:

1kiB (current): ~22s
8kiB: ~8s
16kiB: ~4s
32kiB: ~3.5s
64kiB: ~2.8s
128kiB: ~2.6s
256kiB: ~2.1s
512kiB: ~2.1s
1MiB: ~2.1s
2MiB: ~2.1s

I choose 512kiB from this list as it seems to be the place where the returns 
start diminishing and still isn't that much memory


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153060

Files:
  lldb/source/Target/Platform.cpp


Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1630,7 +1630,7 @@
 return error;
   }
 
-  std::vector buffer(1024);
+  std::vector buffer(512 * 1024);
   auto offset = src_offset;
   uint64_t total_bytes_read = 0;
   while (total_bytes_read < src_size) {


Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1630,7 +1630,7 @@
 return error;
   }
 
-  std::vector buffer(1024);
+  std::vector buffer(512 * 1024);
   auto offset = src_offset;
   uint64_t total_bytes_read = 0;
   while (total_bytes_read < src_size) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153060: lldb: do more than 1 kilobyte at a time to vastly increase binary sync speed

2023-06-15 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

Yeah that's an unintended bottleneck.  If you're debugging to debugserver on a 
darwin system, it will tell lldb it uses a 128k max packet size, so larger read 
requests are still being broken up in to 128k chunks.  The 128k size was set 
when we found little perf benefit from larger packet sizes.  But using a 1k 
local buffer in this method to transfer binaries is unnecessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153060/new/

https://reviews.llvm.org/D153060

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152757: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:592
   if (data_sp->GetByteSize() < length)
-data_sp = MapFileData(file, -1, file_offset);
+data_sp = MapFileData(file, length, file_offset);
   if (data_sp)

A length of `-1` means "map the whole file", if my understanding is correct. 
Why do we want to change this to `length` instead of just -1? Or is this `file` 
the entire zip file?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152757/new/

https://reviews.llvm.org/D152757

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

Thanks for reworking it! You may want to wait a little bit before landing so 
others have some time to look over it, but I don't see anything that should 
prevent this from going in.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152855: [lldb][Android] Add PlatformAndroidTest

2023-06-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Very easy fix for this as suggested in code changes and this will be good to go




Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.h:73
 
+  typedef std::unique_ptr AdbClientSP;
+  virtual AdbClientSP GetAdbClient();

Using the "SP" suffix is for std::shared_ptr, "UP" is for unique pointers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152855/new/

https://reviews.llvm.org/D152855

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152855: [lldb][Android] Add PlatformAndroidTest

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:209
 
-  return adb.ShellToFile(cmd, minutes(1), destination);
+  return adb->ShellToFile(cmd, minutes(1), destination);
 }

`GetAdbClient()` calls `make_unique` which can fail. You'll want to add some 
checks when using all the `adb` objects in this file.



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.h:73
 
+  typedef std::unique_ptr AdbClientSP;
+  virtual AdbClientSP GetAdbClient();

`SP` -> `shared_ptr`. Was this supposed to be `std::shared_ptr`? If it's 
supposed to be a unique_ptr, you probably want `AdbClientUP`.



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.h:74
+  typedef std::unique_ptr AdbClientSP;
+  virtual AdbClientSP GetAdbClient();
+

I feel a little strange about this one because it's possible to create 2 
`AdbClient`s from the same PlatformAndroid ID, both are unique pointers and 
essentially do the same thing. Maybe `PlatformAndroid` can own an `AdbClient` 
that it vends through `GetAdbClient`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152855/new/

https://reviews.llvm.org/D152855

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added inline comments.



Comment at: lldb/include/lldb/Host/common/ZipFileResolver.h:30
+
+  static bool ResolveBionicPath(const FileSpec &file_spec, FileKind &file_kind,
+std::string &file_path,

This function name sounds like the the bionic linker is in the zip file which 
is not the case IIUC. I'm fine with adding a zip file resolver in lldb but I'd 
prefer if it had a generic name, otherwise, this should be a plugin.



Comment at: lldb/source/Utility/ZipFile.cpp:19-21
+// Zip headers.
+// https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
+

Did you just copy & past the this file from somewhere else of did you implement 
it yourself ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152933: [lldb][Android] Add platform.plugin.remote-android.run-as

2023-06-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

I am wondering if the current proposed setting name 
"platform.plugin.remote-android.run-as" should actually be 
"platform.plugin.remote-android. package-name" to be a bit more clear. Not sure 
if we can use the package name in other places?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152933/new/

https://reviews.llvm.org/D152933

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added inline comments.



Comment at: lldb/include/lldb/Host/common/ZipFileResolver.h:30
+
+  static bool ResolveBionicPath(const FileSpec &file_spec, FileKind &file_kind,
+std::string &file_path,

mib wrote:
> This function name sounds like the the bionic linker is in the zip file which 
> is not the case IIUC. I'm fine with adding a zip file resolver in lldb but 
> I'd prefer if it had a generic name, otherwise, this should be a plugin.
what about this instead ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153073: [LLDB] Add DWARF definitions for the new Mojo language

2023-06-15 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision.
Herald added a reviewer: deadalnix.
Herald added a subscriber: hiraditya.
Herald added a project: All.
wallace requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

The new language Mojo recently received a proper DWARF code, which can be seen 
in https://dwarfstd.org/languages.html, and this patch adds the basic 
definitions for using this language in DWARF.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153073

Files:
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Target/Language.cpp
  llvm/include/llvm-c/DebugInfo.h
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/BinaryFormat/Dwarf.h
  llvm/lib/IR/DIBuilder.cpp

Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -138,7 +138,7 @@
 DICompileUnit::DebugNameTableKind NameTableKind, bool RangesBaseAddress,
 StringRef SysRoot, StringRef SDK) {
 
-  assert(((Lang <= dwarf::DW_LANG_Ada2012 && Lang >= dwarf::DW_LANG_C89) ||
+  assert(((Lang <= dwarf::DW_LANG_Mojo && Lang >= dwarf::DW_LANG_C89) ||
   (Lang <= dwarf::DW_LANG_hi_user && Lang >= dwarf::DW_LANG_lo_user)) &&
  "Invalid Language tag");
 
Index: llvm/include/llvm/BinaryFormat/Dwarf.h
===
--- llvm/include/llvm/BinaryFormat/Dwarf.h
+++ llvm/include/llvm/BinaryFormat/Dwarf.h
@@ -263,6 +263,7 @@
   case DW_LANG_Fortran18:
   case DW_LANG_Ada2005:
   case DW_LANG_Ada2012:
+  case DW_LANG_Mojo:
 result = false;
 break;
   }
@@ -329,6 +330,7 @@
   case DW_LANG_C17:
   case DW_LANG_Ada2005:
   case DW_LANG_Ada2012:
+  case DW_LANG_Mojo:
 result = false;
 break;
   }
@@ -393,6 +395,7 @@
   case DW_LANG_Fortran18:
   case DW_LANG_Ada2005:
   case DW_LANG_Ada2012:
+  case DW_LANG_Mojo:
 return false;
   }
   llvm_unreachable("Unknown language kind.");
Index: llvm/include/llvm/BinaryFormat/Dwarf.def
===
--- llvm/include/llvm/BinaryFormat/Dwarf.def
+++ llvm/include/llvm/BinaryFormat/Dwarf.def
@@ -925,6 +925,7 @@
 HANDLE_DW_LANG(0x002d, Fortran18, 0, 0, DWARF)
 HANDLE_DW_LANG(0x002e, Ada2005, 0, 0, DWARF)
 HANDLE_DW_LANG(0x002f, Ada2012, 0, 0, DWARF)
+HANDLE_DW_LANG(0x0033, Mojo, 0, 0, DWARF)
 // Vendor extensions:
 HANDLE_DW_LANG(0x8001, Mips_Assembler, std::nullopt, 0, MIPS)
 HANDLE_DW_LANG(0x8e57, GOOGLE_RenderScript, 0, 0, GOOGLE)
Index: llvm/include/llvm-c/DebugInfo.h
===
--- llvm/include/llvm-c/DebugInfo.h
+++ llvm/include/llvm-c/DebugInfo.h
@@ -125,6 +125,7 @@
   LLVMDWARFSourceLanguageFortran18,
   LLVMDWARFSourceLanguageAda2005,
   LLVMDWARFSourceLanguageAda2012,
+  LLVMDWARFSourceLanguageMojo,
   // Vendor extensions:
   LLVMDWARFSourceLanguageMips_Assembler,
   LLVMDWARFSourceLanguageGOOGLE_RenderScript,
Index: lldb/source/Target/Language.cpp
===
--- lldb/source/Target/Language.cpp
+++ lldb/source/Target/Language.cpp
@@ -209,9 +209,9 @@
 {"fortran18", eLanguageTypeFortran18},
 {"ada2005", eLanguageTypeAda2005},
 {"ada2012", eLanguageTypeAda2012},
+{"mojo", eLanguageTypeMojo},
 // Vendor Extensions
 {"assembler", eLanguageTypeMipsAssembler},
-{"mojo", eLanguageTypeMojo},
 // Now synonyms, in arbitrary order
 {"objc", eLanguageTypeObjC},
 {"objc++", eLanguageTypeObjC_plus_plus},
@@ -457,12 +457,12 @@
   return std::pair();
 }
 
-bool Language::DemangledNameContainsPath(llvm::StringRef path, 
+bool Language::DemangledNameContainsPath(llvm::StringRef path,
  ConstString demangled) const {
   // The base implementation does a simple contains comparision:
   if (path.empty())
 return false;
-  return demangled.GetStringRef().contains(path); 
+  return demangled.GetStringRef().contains(path);
 }
 
 DumpValueObjectOptions::DeclPrintingHelper Language::GetDeclPrintingHelper() {
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -490,16 +490,16 @@
   eLanguageTypeFortran18 = 0x002d,
   eLanguageTypeAda2005 = 0x002e,
   eLanguageTypeAda2012 = 0x002f,
+  eLanguageTypeMojo = 0x0033,
 
   // Vendor Extensions
   // Note: Language::GetNameForLanguageType
   // assumes these can be used as indexes into array language_names, and
   // Language::SetLanguageFromCString and Language::AsCString assume these can
   // be used as indexes into array g_languages.
-  eLanguageTypeMipsAssembler,   ///< Mips_Assembler.
+  eLanguageTypeMipsAssembler, ///< Mips_Assembler.
   // Mojo will move to the common list of

[Lldb-commits] [PATCH] D152933: [lldb][Android] Add platform.plugin.remote-android.run-as

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

What happens if you do `run-as` with an empty package name?
I'm also not tied to the name `"platform.plugin.remote-android.run-as"` if 
others don't like it. `platform.plugin.remote-android.package-name` is fine too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152933/new/

https://reviews.llvm.org/D152933

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152757: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:592
   if (data_sp->GetByteSize() < length)
-data_sp = MapFileData(file, -1, file_offset);
+data_sp = MapFileData(file, length, file_offset);
   if (data_sp)

bulbazord wrote:
> A length of `-1` means "map the whole file", if my understanding is correct. 
> Why do we want to change this to `length` instead of just -1? Or is this 
> `file` the entire zip file?
I'm now updating this diff to add comments to clarify the intention.

For Android zip .so file (D152759),
the ModuleSpec `spec` will have these info with this diff.
- object offset = .so file offset inside the zip file
- object size = .so file size

PlatformAndroid requires this module spec to call DownloadModuleSlice in order 
to download the .so file from the zip file using adb shell dd.
- file spec = "zip_path!/lib_path"
- file offset = .so file offset inside the zip file == dd skip parameter
- file size = .so file size == dd count parameter

And ObjectFileELF will calculate CRC32 if the ELF does not have GNU BuildID, 
that will use the `data_sp` size. Which is set by `MapFileData`. Currently `-1` 
== the whole zip file size. (In fact, lldb-server is crashing due to SIGSEGV.) 
It is supposed to be the .so file size. 
In the unittest, the test .so file has CRC32 `7D6E4738`. And it should be the 
same for the file size.

offset-test.bin contains
-1024bytes of '\0'
- liboffset-test.so (file offset = 1024, file size = 3600, CRC32 '7D6E4738')
- 16bytes of '\0'


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152757/new/

https://reviews.llvm.org/D152757

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153073: [LLDB] Add DWARF definitions for the new Mojo language

2023-06-15 Thread River Riddle via Phabricator via lldb-commits
rriddle accepted this revision.
rriddle added a comment.
This revision is now accepted and ready to land.

LGTM, but deferring to code owners for full approval.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153073/new/

https://reviews.llvm.org/D153073

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153073: [LLDB] Add DWARF definitions for the new Mojo language

2023-06-15 Thread Joe Loser via Phabricator via lldb-commits
jloser added a comment.

Excited to see this come together!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153073/new/

https://reviews.llvm.org/D153073

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152757: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

2023-06-15 Thread Wanyi Ye via Phabricator via lldb-commits
kusmour added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:592
   if (data_sp->GetByteSize() < length)
-data_sp = MapFileData(file, -1, file_offset);
+data_sp = MapFileData(file, length, file_offset);
   if (data_sp)

splhack wrote:
> bulbazord wrote:
> > A length of `-1` means "map the whole file", if my understanding is 
> > correct. Why do we want to change this to `length` instead of just -1? Or 
> > is this `file` the entire zip file?
> I'm now updating this diff to add comments to clarify the intention.
> 
> For Android zip .so file (D152759),
> the ModuleSpec `spec` will have these info with this diff.
> - object offset = .so file offset inside the zip file
> - object size = .so file size
> 
> PlatformAndroid requires this module spec to call DownloadModuleSlice in 
> order to download the .so file from the zip file using adb shell dd.
> - file spec = "zip_path!/lib_path"
> - file offset = .so file offset inside the zip file == dd skip parameter
> - file size = .so file size == dd count parameter
> 
> And ObjectFileELF will calculate CRC32 if the ELF does not have GNU BuildID, 
> that will use the `data_sp` size. Which is set by `MapFileData`. Currently 
> `-1` == the whole zip file size. (In fact, lldb-server is crashing due to 
> SIGSEGV.) It is supposed to be the .so file size. 
> In the unittest, the test .so file has CRC32 `7D6E4738`. And it should be the 
> same for the file size.
> 
> offset-test.bin contains
> -1024bytes of '\0'
> - liboffset-test.so (file offset = 1024, file size = 3600, CRC32 '7D6E4738')
> - 16bytes of '\0'
Reading from [[ 
https://android.googlesource.com/platform/bionic/+/master/android-changes-for-ndk-developers.md#opening-shared-libraries-directly-from-an-apk
 | Opening shared libraries directly from an APK ]]
Because the `.so` file is now page aligned and has offset, the actual size 
should <= the size of `file`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152757/new/

https://reviews.llvm.org/D152757

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added inline comments.



Comment at: lldb/include/lldb/Host/common/ZipFileResolver.h:30
+
+  static bool ResolveBionicPath(const FileSpec &file_spec, FileKind &file_kind,
+std::string &file_path,

mib wrote:
> mib wrote:
> > This function name sounds like the the bionic linker is in the zip file 
> > which is not the case IIUC. I'm fine with adding a zip file resolver in 
> > lldb but I'd prefer if it had a generic name, otherwise, this should be a 
> > plugin.
> what about this instead ?
Yes, should be good. will update this diff.

(Why it had "Bionic" because I thought the path encoding "zip_path!/lib_path" 
is bionic specific.)



Comment at: lldb/source/Utility/ZipFile.cpp:19-21
+// Zip headers.
+// https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
+

mib wrote:
> Did you just copy & past the this file from somewhere else of did you 
> implement it yourself ?
I implemented this code.

logic
- Linear search the end of central directory record from the file end because 
it is located at the end of the file with comment (64KB max)
- Linear search the file from the central directory records that is pointed by 
the end of central directory record.
- Get the file offset and size from the local file header that is pointed by 
the central directory record
- Use unaligned_uint16_t/unaligned_uint32_t since Zip header is 1 byte aligned. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] b4d710e - [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

2023-06-15 Thread Kazuki Sakamoto via lldb-commits

Author: Kazuki Sakamoto
Date: 2023-06-15T14:24:14-07:00
New Revision: b4d710e410595905c6c1d40cd5d257dfa9143bbe

URL: 
https://github.com/llvm/llvm-project/commit/b4d710e410595905c6c1d40cd5d257dfa9143bbe
DIFF: 
https://github.com/llvm/llvm-project/commit/b4d710e410595905c6c1d40cd5d257dfa9143bbe.diff

LOG: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

lldb-server for Android does not build with NDK r21 and above due to
RetryAfterSignal and Bionic ::open mismatch.
https://github.com/llvm/llvm-project/issues/54727

Apply the LLVM patch to LLDB.
https://github.com/llvm/llvm-project/commit/0a0e411204a2baa520fd73a8d69b664f98b428ba

> In Bionic, open can be overloaded for _FORTIFY_SOURCE support, causing
> compile errors of RetryAfterSignal due to overload resolution. Wrapping
> the call in a lambda avoids this.

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

Added: 


Modified: 
lldb/include/lldb/Host/FileSystem.h
lldb/source/Host/common/PseudoTerminal.cpp
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/source/Host/posix/FileSystemPosix.cpp
lldb/source/Host/posix/PipePosix.cpp
lldb/source/Host/posix/ProcessLauncherPosixFork.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/FileSystem.h 
b/lldb/include/lldb/Host/FileSystem.h
index 823e7710b4efe..e25fc9983c1f0 100644
--- a/lldb/include/lldb/Host/FileSystem.h
+++ b/lldb/include/lldb/Host/FileSystem.h
@@ -52,7 +52,7 @@ class FileSystem {
   FILE *Fopen(const char *path, const char *mode);
 
   /// Wraps ::open in a platform-independent way.
-  int Open(const char *path, int flags, int mode);
+  int Open(const char *path, int flags, int mode = 0600);
 
   llvm::Expected>
   Open(const FileSpec &file_spec, File::OpenOptions options,

diff  --git a/lldb/source/Host/common/PseudoTerminal.cpp 
b/lldb/source/Host/common/PseudoTerminal.cpp
index be4c3c7928dfd..de49058beeb70 100644
--- a/lldb/source/Host/common/PseudoTerminal.cpp
+++ b/lldb/source/Host/common/PseudoTerminal.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Config.h"
+#include "lldb/Host/FileSystem.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Errno.h"
 #include 
@@ -95,7 +96,7 @@ llvm::Error PseudoTerminal::OpenSecondary(int oflag) {
   CloseSecondaryFileDescriptor();
 
   std::string name = GetSecondaryName();
-  m_secondary_fd = llvm::sys::RetryAfterSignal(-1, ::open, name.c_str(), 
oflag);
+  m_secondary_fd = FileSystem::Instance().Open(name.c_str(), oflag);
   if (m_secondary_fd >= 0)
 return llvm::Error::success();
 

diff  --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp 
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index dc5b24979fb54..825da09d76022 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -15,6 +15,7 @@
 
 #include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
 #include "lldb/Host/Config.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Socket.h"
 #include "lldb/Host/SocketAddress.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -726,7 +727,7 @@ ConnectionStatus ConnectionFileDescriptor::ConnectFile(
 #if LLDB_ENABLE_POSIX
   std::string addr_str = s.str();
   // file:///PATH
-  int fd = llvm::sys::RetryAfterSignal(-1, ::open, addr_str.c_str(), O_RDWR);
+  int fd = FileSystem::Instance().Open(addr_str.c_str(), O_RDWR);
   if (fd == -1) {
 if (error_ptr)
   error_ptr->SetErrorToErrno();
@@ -776,7 +777,7 @@ ConnectionStatus 
ConnectionFileDescriptor::ConnectSerialPort(
 return eConnectionStatusError;
   }
 
-  int fd = llvm::sys::RetryAfterSignal(-1, ::open, path.str().c_str(), O_RDWR);
+  int fd = FileSystem::Instance().Open(path.str().c_str(), O_RDWR);
   if (fd == -1) {
 if (error_ptr)
   error_ptr->SetErrorToErrno();

diff  --git a/lldb/source/Host/posix/FileSystemPosix.cpp 
b/lldb/source/Host/posix/FileSystemPosix.cpp
index 26a266e86382b..cdb76da626bc9 100644
--- a/lldb/source/Host/posix/FileSystemPosix.cpp
+++ b/lldb/source/Host/posix/FileSystemPosix.cpp
@@ -77,5 +77,8 @@ FILE *FileSystem::Fopen(const char *path, const char *mode) {
 }
 
 int FileSystem::Open(const char *path, int flags, int mode) {
-  return llvm::sys::RetryAfterSignal(-1, ::open, path, flags, mode);
+  // Call ::open in a lambda to avoid overload resolution in RetryAfterSignal
+  // when open is overloaded, such as in Bionic.
+  auto lambda = [&]() { return ::open(path, flags, mode); };
+  return llvm::sys::RetryAfterSignal(-1, lambda);
 }

diff  --git a/lldb/source/Host/posix/PipePosix.cpp 
b/lldb/source/Host/posix/PipePosix.cpp
index bd311ad8769a4..5e4e8618fa4f1 100644
--- a/lldb/source/Host/posix/PipePosix.cpp
+++ b/lldb/source/Host/posix/PipePosix.cpp
@@ -7,11 +7,11 @@
 
//===--===//
 
 #include "l

[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb4d710e41059: [lldb][Android] Use a lambda for calls to 
::open in RetryAfterSignal (authored by splhack).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152712/new/

https://reviews.llvm.org/D152712

Files:
  lldb/include/lldb/Host/FileSystem.h
  lldb/source/Host/common/PseudoTerminal.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Host/posix/FileSystemPosix.cpp
  lldb/source/Host/posix/PipePosix.cpp
  lldb/source/Host/posix/ProcessLauncherPosixFork.cpp

Index: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
===
--- lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "lldb/Host/posix/ProcessLauncherPosixFork.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Host/Pipe.h"
@@ -14,7 +15,6 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/Support/Errno.h"
-#include "llvm/Support/FileSystem.h"
 
 #include 
 #include 
@@ -71,7 +71,7 @@
 }
 
 static void DupDescriptor(int error_fd, const char *file, int fd, int flags) {
-  int target_fd = llvm::sys::RetryAfterSignal(-1, ::open, file, flags, 0666);
+  int target_fd = FileSystem::Instance().Open(file, flags, 0666);
 
   if (target_fd == -1)
 ExitWithError(error_fd, "DupDescriptor-open");
Index: lldb/source/Host/posix/PipePosix.cpp
===
--- lldb/source/Host/posix/PipePosix.cpp
+++ lldb/source/Host/posix/PipePosix.cpp
@@ -7,11 +7,11 @@
 //===--===//
 
 #include "lldb/Host/posix/PipePosix.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/SelectHelper.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Errno.h"
-#include "llvm/Support/FileSystem.h"
 #include 
 #include 
 
@@ -148,7 +148,7 @@
 flags |= O_CLOEXEC;
 
   Status error;
-  int fd = llvm::sys::RetryAfterSignal(-1, ::open, name.str().c_str(), flags);
+  int fd = FileSystem::Instance().Open(name.str().c_str(), flags);
   if (fd != -1)
 m_fds[READ] = fd;
   else
Index: lldb/source/Host/posix/FileSystemPosix.cpp
===
--- lldb/source/Host/posix/FileSystemPosix.cpp
+++ lldb/source/Host/posix/FileSystemPosix.cpp
@@ -77,5 +77,8 @@
 }
 
 int FileSystem::Open(const char *path, int flags, int mode) {
-  return llvm::sys::RetryAfterSignal(-1, ::open, path, flags, mode);
+  // Call ::open in a lambda to avoid overload resolution in RetryAfterSignal
+  // when open is overloaded, such as in Bionic.
+  auto lambda = [&]() { return ::open(path, flags, mode); };
+  return llvm::sys::RetryAfterSignal(-1, lambda);
 }
Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -15,6 +15,7 @@
 
 #include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
 #include "lldb/Host/Config.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Socket.h"
 #include "lldb/Host/SocketAddress.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -726,7 +727,7 @@
 #if LLDB_ENABLE_POSIX
   std::string addr_str = s.str();
   // file:///PATH
-  int fd = llvm::sys::RetryAfterSignal(-1, ::open, addr_str.c_str(), O_RDWR);
+  int fd = FileSystem::Instance().Open(addr_str.c_str(), O_RDWR);
   if (fd == -1) {
 if (error_ptr)
   error_ptr->SetErrorToErrno();
@@ -776,7 +777,7 @@
 return eConnectionStatusError;
   }
 
-  int fd = llvm::sys::RetryAfterSignal(-1, ::open, path.str().c_str(), O_RDWR);
+  int fd = FileSystem::Instance().Open(path.str().c_str(), O_RDWR);
   if (fd == -1) {
 if (error_ptr)
   error_ptr->SetErrorToErrno();
Index: lldb/source/Host/common/PseudoTerminal.cpp
===
--- lldb/source/Host/common/PseudoTerminal.cpp
+++ lldb/source/Host/common/PseudoTerminal.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Config.h"
+#include "lldb/Host/FileSystem.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Errno.h"
 #include 
@@ -95,7 +96,7 @@
   CloseSecondaryFileDescriptor();
 
   std::string name = GetSecondaryName();
-  m_secondary_fd = llvm::sys::RetryAfterSignal(-1, ::open, name.c_str(), oflag);
+  m_secondary_fd = FileSystem::Instance().Open(name.c_str(), oflag);
   if (m_secondary_fd >= 0)
 return llvm::Error::success();
 
Index: lldb/include/lldb/Host/FileSystem.h

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

If nobody else has any comments, I plan on landing this tomorrow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152569/new/

https://reviews.llvm.org/D152569

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152855: [lldb][Android] Add PlatformAndroidTest

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added inline comments.



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:209
 
-  return adb.ShellToFile(cmd, minutes(1), destination);
+  return adb->ShellToFile(cmd, minutes(1), destination);
 }

bulbazord wrote:
> `GetAdbClient()` calls `make_unique` which can fail. You'll want to add some 
> checks when using all the `adb` objects in this file.
will do!



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.h:73
 
+  typedef std::unique_ptr AdbClientSP;
+  virtual AdbClientSP GetAdbClient();

clayborg wrote:
> bulbazord wrote:
> > `SP` -> `shared_ptr`. Was this supposed to be `std::shared_ptr`? If it's 
> > supposed to be a unique_ptr, you probably want `AdbClientUP`.
> Using the "SP" suffix is for std::shared_ptr, "UP" is for unique pointers.
@bulbazord @clayborg thanks, and oops didn't realize UP. will fix.



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.h:74
+  typedef std::unique_ptr AdbClientSP;
+  virtual AdbClientSP GetAdbClient();
+

bulbazord wrote:
> I feel a little strange about this one because it's possible to create 2 
> `AdbClient`s from the same PlatformAndroid ID, both are unique pointers and 
> essentially do the same thing. Maybe `PlatformAndroid` can own an `AdbClient` 
> that it vends through `GetAdbClient`?
Yes, it's a bit strange how PlatformAndroid currently uses AdbClient. But, the 
scope of this diff is to unblock us first to write PlatformAndroid unittest 
with gmock + virtual. We will revisit for the structure change later.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152855/new/

https://reviews.llvm.org/D152855

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 8fb919a - [lldb] Symtab::SectionFileAddressesChanged should clear the file address map instead of name map

2023-06-15 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-06-15T14:46:46-07:00
New Revision: 8fb919a1154a593fdf01e759ece7904afc73f687

URL: 
https://github.com/llvm/llvm-project/commit/8fb919a1154a593fdf01e759ece7904afc73f687
DIFF: 
https://github.com/llvm/llvm-project/commit/8fb919a1154a593fdf01e759ece7904afc73f687.diff

LOG: [lldb] Symtab::SectionFileAddressesChanged should clear the file address 
map instead of name map

Currently, `SectionFileAddressesChanged` clears out the `name_to_index`
map and sets `m_file_addr_to_index_compute` to false. This is strange,
as these two fields are used for different purposes. What we should be
doing is clearing the file address to index mapping.

There are 2 bugs here:
1. If we call SectionFileAddressesChanged after the name indexes have
   been computed, we end up with an empty name to index map, so lookups
   will fail. This doesn't happen today because we don't initialize the
   name indexes before calling this, but this is a refactor away from
   failing in this way.
2. Because we don't clear `m_file_addr_to_index` but still set it's
   computed flag to false, it ends up with twice the amount of
   information. One entry will be correct (since it was recalculated),
   one entry will be outdated.

rdar://110192434

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

Added: 


Modified: 
lldb/source/Symbol/Symtab.cpp

Removed: 




diff  --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index 40777e03be784..cf8732530c1ae 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -80,8 +80,7 @@ size_t Symtab::GetNumSymbols() const {
 }
 
 void Symtab::SectionFileAddressesChanged() {
-  auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
-  name_to_index.Clear();
+  m_file_addr_to_index.Clear();
   m_file_addr_to_index_computed = false;
 }
 



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152579: [lldb] Symtab::SectionFileAddressesChanged should clear the file address map instead of name map

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8fb919a1154a: [lldb] Symtab::SectionFileAddressesChanged 
should clear the file address map… (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152579/new/

https://reviews.llvm.org/D152579

Files:
  lldb/source/Symbol/Symtab.cpp


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -80,8 +80,7 @@
 }
 
 void Symtab::SectionFileAddressesChanged() {
-  auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
-  name_to_index.Clear();
+  m_file_addr_to_index.Clear();
   m_file_addr_to_index_computed = false;
 }
 


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -80,8 +80,7 @@
 }
 
 void Symtab::SectionFileAddressesChanged() {
-  auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
-  name_to_index.Clear();
+  m_file_addr_to_index.Clear();
   m_file_addr_to_index_computed = false;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153079: Add an llvm::report_fatal_error for when the darwin kernel says we've finished an insn-step but the thread doesn't think it was insn-stepping

2023-06-15 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda created this revision.
jasonmolenda added a reviewer: jingham.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

We have some failures on an arm64 CI bot where the kernel has told us that 
we've completed an instruction step, but when lldb checks to confirm that the 
thread was performing an instruction step, it is not.  The debugger doesn't 
know how this could happen, so it stops at that point and notifies the user 
that there has been an anomalous occurrence (calls it an "EXC_ARM_BREAKPOINT"). 
 Fine, but under the testsuite this will cause a testsuite failure.

This adds an llvm::report_fatal_error when not built NDEBUG if this state 
happens, and include two additional pieces of information in the error message 
to help debug what the CI bot is hitting.  I'll change this to an assert 
without the additional information later, so it's more assertively erroring in 
development builds if this combination of events happens again.  But I can't 
dynamically construct a string for an assert and have it dumped, so I'm using 
fatal error to debug the CI bot.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153079

Files:
  lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp


Index: lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
===
--- lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -708,12 +708,49 @@
 
 case llvm::Triple::aarch64_32:
 case llvm::Triple::aarch64: {
+  // xnu describes three things with type EXC_BREAKPOINT:
+  //
+  //   exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
+  //  Watchpoint access.  exc_sub_code is the address of the
+  //  instruction which trigged the watchpoint trap.
+  //  debugserver may add the watchpoint number that was triggered
+  //  in exc_sub_sub_code.
+  //
+  //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code 0
+  //  Instruction step has completed.
+  //
+  //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code address-of-instruction
+  //  Software breakpoint instruction executed.
+
   if (exc_code == 1 && exc_sub_code == 0) // EXC_ARM_BREAKPOINT
   {
 // This is hit when we single instruction step aka MDSCR_EL1 SS bit 0
 // is set
-is_actual_breakpoint = false;
+is_actual_breakpoint = true;
 is_trace_if_actual_breakpoint_missing = true;
+#ifndef NDEBUG
+if (thread.GetTemporaryResumeState() != eStateStepping) {
+  StreamString s;
+  s.Printf("CreateStopReasonWithMachException got EXC_BREAKPOINT [1,0] 
"
+   "indicating trace event, but thread is not tracing, it has "
+   "ResumeState %d",
+   thread.GetTemporaryResumeState());
+  if (RegisterContextSP regctx = thread.GetRegisterContext()) {
+if (const RegisterInfo *ri = regctx->GetRegisterInfoByName("esr")) 
{
+  uint32_t esr =
+  (uint32_t)regctx->ReadRegisterAsUnsigned(ri, UINT32_MAX);
+  if (esr != UINT32_MAX) {
+s.Printf(" esr value: 0x%" PRIx32, esr);
+  }
+}
+  }
+  llvm::report_fatal_error(s.GetData());
+  lldbassert(
+  false &&
+  "CreateStopReasonWithMachException got EXC_BREAKPOINT [1,0] "
+  "indicating trace event, but thread was not doing a step.");
+}
+#endif
   }
   if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
   {


Index: lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
===
--- lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -708,12 +708,49 @@
 
 case llvm::Triple::aarch64_32:
 case llvm::Triple::aarch64: {
+  // xnu describes three things with type EXC_BREAKPOINT:
+  //
+  //   exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
+  //  Watchpoint access.  exc_sub_code is the address of the
+  //  instruction which trigged the watchpoint trap.
+  //  debugserver may add the watchpoint number that was triggered
+  //  in exc_sub_sub_code.
+  //
+  //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code 0
+  //  Instruction step has completed.
+  //
+  //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code address-of-instruction
+  //  Software breakpoint instruction executed.
+
   if (exc_code == 1 && exc_sub_code == 0) // EXC_ARM_BREAKPOINT
   {
 // This is hit when we single instruction step aka MDSCR_EL1 SS bit 0
   

[Lldb-commits] [PATCH] D152933: [lldb][Android] Add platform.plugin.remote-android.run-as

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added a comment.

@clayborg

> platform.plugin.remote-android.package-name

sure, sounds good. will update.

@bulbazord

> What happens if you do run-as with an empty package name?



  $ adb shell "run-as '' ls"; echo $?
  run-as: unknown package:
  1

which is the same as when specify non-existing package.

  $ adb shell "run-as 'unknown' ls"; echo $?
  run-as: unknown package: unknown
  1


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152933/new/

https://reviews.llvm.org/D152933

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] e29cc52 - [lldb][NFCI] Remove use of ConstString from IOHandler

2023-06-15 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-06-15T14:57:20-07:00
New Revision: e29cc5216a8608b026e390b69022878b2ec3071a

URL: 
https://github.com/llvm/llvm-project/commit/e29cc5216a8608b026e390b69022878b2ec3071a
DIFF: 
https://github.com/llvm/llvm-project/commit/e29cc5216a8608b026e390b69022878b2ec3071a.diff

LOG: [lldb][NFCI] Remove use of ConstString from IOHandler

None of these need to be in the ConstString StringPool. For the most
part they are constant strings and do not require fast comparisons.

I did change IOHandlerDelegateMultiline slightly -- specifically, the
`m_end_line` member always has a `\n` at the end of it now. This was so
that `IOHandlerGetControlSequence` can always return a StringRef. This
did require a slight change to `IOHandlerIsInputComplete` where we must
drop the newline before comparing it against the input parameter.

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

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Core/IOHandler.h
lldb/include/lldb/Expression/REPL.h
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/API/SBCommandInterpreter.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Expression/REPL.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 64d9fad8de20d..4e3c177d33e59 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -232,7 +232,7 @@ class Debugger : public 
std::enable_shared_from_this,
 
   void PrintAsync(const char *s, size_t len, bool is_stdout);
 
-  ConstString GetTopIOHandlerControlSequence(char ch);
+  llvm::StringRef GetTopIOHandlerControlSequence(char ch);
 
   const char *GetIOHandlerCommandPrefix();
 

diff  --git a/lldb/include/lldb/Core/IOHandler.h 
b/lldb/include/lldb/Core/IOHandler.h
index 18d87acbd8722..0eef77e81ccb8 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -12,7 +12,6 @@
 #include "lldb/Core/ValueObjectList.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Utility/CompletionRequest.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Flags.h"
 #include "lldb/Utility/Predicate.h"
 #include "lldb/Utility/Stream.h"
@@ -107,7 +106,7 @@ class IOHandler {
   }
   bool SetPrompt(const char *) = delete;
 
-  virtual ConstString GetControlSequence(char ch) { return ConstString(); }
+  virtual llvm::StringRef GetControlSequence(char ch) { return {}; }
 
   virtual const char *GetCommandPrefix() { return nullptr; }
 
@@ -271,9 +270,7 @@ class IOHandlerDelegate {
 return true;
   }
 
-  virtual ConstString IOHandlerGetControlSequence(char ch) {
-return ConstString();
-  }
+  virtual llvm::StringRef IOHandlerGetControlSequence(char ch) { return {}; }
 
   virtual const char *IOHandlerGetCommandPrefix() { return nullptr; }
 
@@ -295,24 +292,25 @@ class IOHandlerDelegate {
 // the last line is equal to "end_line" which is specified in the constructor.
 class IOHandlerDelegateMultiline : public IOHandlerDelegate {
 public:
-  IOHandlerDelegateMultiline(const char *end_line,
+  IOHandlerDelegateMultiline(llvm::StringRef end_line,
  Completion completion = Completion::None)
-  : IOHandlerDelegate(completion),
-m_end_line((end_line && end_line[0]) ? end_line : "") {}
+  : IOHandlerDelegate(completion), m_end_line(end_line.str() + "\n") {}
 
   ~IOHandlerDelegateMultiline() override = default;
 
-  ConstString IOHandlerGetControlSequence(char ch) override {
+  llvm::StringRef IOHandlerGetControlSequence(char ch) override {
 if (ch == 'd')
-  return ConstString(m_end_line + "\n");
-return ConstString();
+  return m_end_line;
+return {};
   }
 
   bool IOHandlerIsInputComplete(IOHandler &io_handler,
 StringList &lines) override {
 // Determine whether the end of input signal has been entered
 const size_t num_lines = lines.GetSize();
-if (num_lines > 0 && lines[num_lines - 1] == m_end_line) {
+const llvm::StringRef end_line =
+llvm::StringRef(m_end_line).drop_back(1); // Drop '\n'
+if (num_lines > 0 && llvm::StringRef(lines[num_lines - 1]) == end_line) {
   // Remove the terminal line from "lines" so it doesn't appear in the
   // resulting input and return true to indicate we are done getting lines
   lines.PopBack();
@@ -373,7 +371,7 @@ class IOHandlerEditline : public IOHandler {
 
   void TerminalSizeChanged() override;
 
-  ConstString GetControlSequence(char ch) override {
+  llvm::StringRef GetControlSequence(char ch) override {
 return m_delegate.IOHandlerGetControlSequence(ch);
   }
 
@@ -522,8 +520,9 @@ class IOHandlerStack {
 m_stack[num_io_handlers - 2]->GetType() == second_top_type);
   }
 
-  ConstString GetTopIO

[Lldb-commits] [PATCH] D151597: [lldb][NFCI] Remove use of ConstString from IOHandler

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe29cc5216a86: [lldb][NFCI] Remove use of ConstString from 
IOHandler (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151597/new/

https://reviews.llvm.org/D151597

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/IOHandler.h
  lldb/include/lldb/Expression/REPL.h
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -434,10 +434,11 @@
 
   ~IOHandlerPythonInterpreter() override = default;
 
-  ConstString GetControlSequence(char ch) override {
+  llvm::StringRef GetControlSequence(char ch) override {
+static constexpr llvm::StringLiteral control_sequence("quit()\n");
 if (ch == 'd')
-  return ConstString("quit()\n");
-return ConstString();
+  return control_sequence;
+return {};
   }
 
   void Run() override {
Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -117,10 +117,11 @@
   return (m_enable_auto_indent ? GetAutoIndentCharacters() : nullptr);
 }
 
-ConstString REPL::IOHandlerGetControlSequence(char ch) {
+llvm::StringRef REPL::IOHandlerGetControlSequence(char ch) {
+  static constexpr llvm::StringLiteral control_sequence(":quit\n");
   if (ch == 'd')
-return ConstString(":quit\n");
-  return ConstString();
+return control_sequence;
+  return {};
 }
 
 const char *REPL::IOHandlerGetCommandPrefix() { return ":"; }
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1129,7 +1129,7 @@
   }
 }
 
-ConstString Debugger::GetTopIOHandlerControlSequence(char ch) {
+llvm::StringRef Debugger::GetTopIOHandlerControlSequence(char ch) {
   return m_io_handler_stack.GetTopIOHandlerControlSequence(ch);
 }
 
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -155,11 +155,12 @@
 const char *SBCommandInterpreter::GetIOHandlerControlSequence(char ch) {
   LLDB_INSTRUMENT_VA(this, ch);
 
-  return (IsValid()
-  ? m_opaque_ptr->GetDebugger()
-.GetTopIOHandlerControlSequence(ch)
-.GetCString()
-  : nullptr);
+  if (!IsValid())
+return nullptr;
+
+  return ConstString(
+ m_opaque_ptr->GetDebugger().GetTopIOHandlerControlSequence(ch))
+  .GetCString();
 }
 
 lldb::ReturnStatus
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -652,10 +652,11 @@
   void IOHandlerInputComplete(IOHandler &io_handler,
   std::string &line) override;
 
-  ConstString IOHandlerGetControlSequence(char ch) override {
+  llvm::StringRef IOHandlerGetControlSequence(char ch) override {
+static constexpr llvm::StringLiteral control_sequence("quit\n");
 if (ch == 'd')
-  return ConstString("quit\n");
-return ConstString();
+  return control_sequence;
+return {};
   }
 
   void GetProcessOutput();
Index: lldb/include/lldb/Expression/REPL.h
===
--- lldb/include/lldb/Expression/REPL.h
+++ lldb/include/lldb/Expression/REPL.h
@@ -88,7 +88,7 @@
 
   const char *IOHandlerGetFixIndentationCharacters() override;
 
-  ConstString IOHandlerGetControlSequence(char ch) override;
+  llvm::StringRef IOHandlerGetControlSequence(char ch) override;
 
   const char *IOHandlerGetCommandPrefix() override;
 
Index: lldb/include/lldb/Core/IOHandler.h
===
--- lldb/include/lldb/Core/IOHandler.h
+++ lldb/include/lldb/Core/IOHandler.h
@@ -12,7 +12,6 @@
 #include "lldb/Core/ValueObjectList.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Utility/CompletionRequest.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Flags.h"
 #include "lldb/Utility/Predicate.h"
 #include "lldb/Utility/Stream.h"
@@ -107,7 +106,7 @@
   }
   bool SetPrompt(const char *) = delete;
 
-  virtual ConstString GetControlSequence(char ch) { return 

[Lldb-commits] [PATCH] D153079: Add an llvm::report_fatal_error for when the darwin kernel says we've finished an insn-step but the thread doesn't think it was insn-stepping

2023-06-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

LGTM.  Let's see if we can get this to actually trap the concurrent test 
failures.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153079/new/

https://reviews.llvm.org/D153079

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 6a8e253 - Add a fatal error for debug builds when disagreement about stepping

2023-06-15 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2023-06-15T15:41:37-07:00
New Revision: 6a8e2538afc10e6b7a321fd38bf5faf550518f2a

URL: 
https://github.com/llvm/llvm-project/commit/6a8e2538afc10e6b7a321fd38bf5faf550518f2a
DIFF: 
https://github.com/llvm/llvm-project/commit/6a8e2538afc10e6b7a321fd38bf5faf550518f2a.diff

LOG: Add a fatal error for debug builds when disagreement about stepping

On one CI bot we're seeing a failure where the kernel reports that
we have completed an instruction step (via a mach exception) and
lldb doesn't think the thread was doing an instruction step.  It
takes the conservative approach of stopping at this point, breaking
tests.

This patch adds an llvm fatal error for debug builds where it will
log the state of the thread and the AArch64 ESR, to confirm what
the hardware reported as the exception so we can double check the
kernel's interpretation.

I'll change this to an lldbassert without the runtime details in
the string once we have an idea what is happening.  the hope is
that this will get hit on the CI bot soon.

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp 
b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index aae15b2ef4624..b4301f21ac103 100644
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -708,12 +708,49 @@ StopInfoSP 
StopInfoMachException::CreateStopReasonWithMachException(
 
 case llvm::Triple::aarch64_32:
 case llvm::Triple::aarch64: {
+  // xnu describes three things with type EXC_BREAKPOINT:
+  //
+  //   exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
+  //  Watchpoint access.  exc_sub_code is the address of the
+  //  instruction which trigged the watchpoint trap.
+  //  debugserver may add the watchpoint number that was triggered
+  //  in exc_sub_sub_code.
+  //
+  //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code 0
+  //  Instruction step has completed.
+  //
+  //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code address-of-instruction
+  //  Software breakpoint instruction executed.
+
   if (exc_code == 1 && exc_sub_code == 0) // EXC_ARM_BREAKPOINT
   {
 // This is hit when we single instruction step aka MDSCR_EL1 SS bit 0
 // is set
-is_actual_breakpoint = false;
+is_actual_breakpoint = true;
 is_trace_if_actual_breakpoint_missing = true;
+#ifndef NDEBUG
+if (thread.GetTemporaryResumeState() != eStateStepping) {
+  StreamString s;
+  s.Printf("CreateStopReasonWithMachException got EXC_BREAKPOINT [1,0] 
"
+   "indicating trace event, but thread is not tracing, it has "
+   "ResumeState %d",
+   thread.GetTemporaryResumeState());
+  if (RegisterContextSP regctx = thread.GetRegisterContext()) {
+if (const RegisterInfo *ri = regctx->GetRegisterInfoByName("esr")) 
{
+  uint32_t esr =
+  (uint32_t)regctx->ReadRegisterAsUnsigned(ri, UINT32_MAX);
+  if (esr != UINT32_MAX) {
+s.Printf(" esr value: 0x%" PRIx32, esr);
+  }
+}
+  }
+  llvm::report_fatal_error(s.GetData());
+  lldbassert(
+  false &&
+  "CreateStopReasonWithMachException got EXC_BREAKPOINT [1,0] "
+  "indicating trace event, but thread was not doing a step.");
+}
+#endif
   }
   if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
   {



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153079: Add an llvm::report_fatal_error for when the darwin kernel says we've finished an insn-step but the thread doesn't think it was insn-stepping

2023-06-15 Thread Jason Molenda via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a8e2538afc1: Add a fatal error for debug builds when 
disagreement about stepping (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153079/new/

https://reviews.llvm.org/D153079

Files:
  lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp


Index: lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
===
--- lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -708,12 +708,49 @@
 
 case llvm::Triple::aarch64_32:
 case llvm::Triple::aarch64: {
+  // xnu describes three things with type EXC_BREAKPOINT:
+  //
+  //   exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
+  //  Watchpoint access.  exc_sub_code is the address of the
+  //  instruction which trigged the watchpoint trap.
+  //  debugserver may add the watchpoint number that was triggered
+  //  in exc_sub_sub_code.
+  //
+  //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code 0
+  //  Instruction step has completed.
+  //
+  //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code address-of-instruction
+  //  Software breakpoint instruction executed.
+
   if (exc_code == 1 && exc_sub_code == 0) // EXC_ARM_BREAKPOINT
   {
 // This is hit when we single instruction step aka MDSCR_EL1 SS bit 0
 // is set
-is_actual_breakpoint = false;
+is_actual_breakpoint = true;
 is_trace_if_actual_breakpoint_missing = true;
+#ifndef NDEBUG
+if (thread.GetTemporaryResumeState() != eStateStepping) {
+  StreamString s;
+  s.Printf("CreateStopReasonWithMachException got EXC_BREAKPOINT [1,0] 
"
+   "indicating trace event, but thread is not tracing, it has "
+   "ResumeState %d",
+   thread.GetTemporaryResumeState());
+  if (RegisterContextSP regctx = thread.GetRegisterContext()) {
+if (const RegisterInfo *ri = regctx->GetRegisterInfoByName("esr")) 
{
+  uint32_t esr =
+  (uint32_t)regctx->ReadRegisterAsUnsigned(ri, UINT32_MAX);
+  if (esr != UINT32_MAX) {
+s.Printf(" esr value: 0x%" PRIx32, esr);
+  }
+}
+  }
+  llvm::report_fatal_error(s.GetData());
+  lldbassert(
+  false &&
+  "CreateStopReasonWithMachException got EXC_BREAKPOINT [1,0] "
+  "indicating trace event, but thread was not doing a step.");
+}
+#endif
   }
   if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
   {


Index: lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
===
--- lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -708,12 +708,49 @@
 
 case llvm::Triple::aarch64_32:
 case llvm::Triple::aarch64: {
+  // xnu describes three things with type EXC_BREAKPOINT:
+  //
+  //   exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
+  //  Watchpoint access.  exc_sub_code is the address of the
+  //  instruction which trigged the watchpoint trap.
+  //  debugserver may add the watchpoint number that was triggered
+  //  in exc_sub_sub_code.
+  //
+  //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code 0
+  //  Instruction step has completed.
+  //
+  //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code address-of-instruction
+  //  Software breakpoint instruction executed.
+
   if (exc_code == 1 && exc_sub_code == 0) // EXC_ARM_BREAKPOINT
   {
 // This is hit when we single instruction step aka MDSCR_EL1 SS bit 0
 // is set
-is_actual_breakpoint = false;
+is_actual_breakpoint = true;
 is_trace_if_actual_breakpoint_missing = true;
+#ifndef NDEBUG
+if (thread.GetTemporaryResumeState() != eStateStepping) {
+  StreamString s;
+  s.Printf("CreateStopReasonWithMachException got EXC_BREAKPOINT [1,0] "
+   "indicating trace event, but thread is not tracing, it has "
+   "ResumeState %d",
+   thread.GetTemporaryResumeState());
+  if (RegisterContextSP regctx = thread.GetRegisterContext()) {
+if (const RegisterInfo *ri = regctx->GetRegisterInfoByName("esr")) {
+  uint32_t esr =
+  (uint32_t)regctx->ReadRegisterAsUnsigned(ri, UINT32_MAX);
+  if (esr != UINT32_MAX) {
+s.Printf(" esr value: 0x%" PRIx32, esr);
+  }
+}
+  }
+  llvm::report_fatal_error(s.Ge

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

2023-06-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/scripts/generate-project.py:21
+def generate_c_header(directory: str, index: int) -> None:
+header_path = f"{directory}/obj{index}.h"
+with open(header_path, "w") as f:

kastiglione wrote:
> kastiglione wrote:
> > os.path.join to be windows friendly?
> I just noticed the contents of the makefiles also have forward slashes, so I 
> retract my comment.
I would vote to use os.path.join() instead of manually formatting strings



Comment at: lldb/scripts/generate-project.py:50
+f.write(
+f"#ifndef _OBJ{index}_H\n"
+f"#define _OBJ{index}_H\n"

Add a "PP" to differentiate from c?



Comment at: lldb/scripts/generate-project.py:297-301
+supported_languages = ["c", "cpp", "swift"]
+if language not in supported_languages:
+print(f"Unrecognized language: {language}")
+print(f"Supported languages: {supported_languages}")
+sys.exit(1)

move into generate_sources(...)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152569/new/

https://reviews.llvm.org/D152569

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153043: [lldb] Fix handling of cfi_restore in the unwinder

2023-06-15 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

I can see why the existing code is written as it is, given the dwarf spec for 
DW_CFA_restore: "The DW_CFA_restore instruction takes a single operand (encoded 
with the opcode) that represents a register number. The required action is to 
change the rule for the indicated register to the rule assigned it by the 
`initial_instructions` in the CIE".  The mistake in the current code is that 
any register not mentioned in the CIE state (the unwind rules at Row 0 in this 
UnwindPlan) is the unmodified value of the caller.

This patch is correct; the register was not mentioned in the CIE so should say 
there is no unwind rule for this register - it is the unmodified value of the 
caller function.  I probably would have added a comment to this new line in the 
`else` block saying that explicitly, but that's just a personal preference.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153043/new/

https://reviews.llvm.org/D153043

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1336
+  // "zip_path!/so_path". Resolve the zip file path, .so file offset and size.
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;

might be good to init this to an invalid value in case code changes over time?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152757: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

2023-06-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

LGTM. Anyone else have any comments?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152757/new/

https://reviews.llvm.org/D152757

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

2023-06-15 Thread Leonard Chan via Phabricator via lldb-commits
leonardchan added a comment.

Hi. I think this is leading to the lldb test failures we're seeing on our linux 
builder 
(https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8778187770933863505/+/u/lldb/test/stdout?format=raw):

  
  FAIL: lldb-unit :: Host/./HostTests/13/39 (1722 of 2262)
   TEST 'lldb-unit :: Host/./HostTests/13/39' FAILED 

  Script(shard):
  --
  
GTEST_OUTPUT=json:/b/s/w/ir/x/w/staging/llvm_build/tools/lldb/unittests/Host/./HostTests-lldb-unit-1483998-13-39.json
 GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=39 GTEST_SHARD_INDEX=13 
/b/s/w/ir/x/w/staging/llvm_build/tools/lldb/unittests/Host/./HostTests
  --
  
  Note: This is test shard 14 of 39.
  [==] Running 2 tests from 2 test suites.
  [--] Global test environment set-up.
  [--] 1 test from File
  [ RUN  ] File.GetWaitableHandleFileno
  [   OK ] File.GetWaitableHandleFileno (1 ms)
  [--] 1 test from File (1 ms total)
  
  [--] 1 test from TerminalTest
  [ RUN  ] TerminalTest.SetRaw
  /b/s/w/ir/x/w/cipd/bin/../include/c++/v1/optional:1023: assertion 
this->has_value() failed: optional operator* called on a disengaged value#0 
0x560219fba7a8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/b/s/w/ir/x/w/staging/llvm_build/tools/lldb/unittests/Host/./HostTests+0x1417a8)
  
  --
  exit: -6
  --
  shard JSON output does not exist: 
/b/s/w/ir/x/w/staging/llvm_build/tools/lldb/unittests/Host/./HostTests-lldb-unit-1483998-13-39.json
  

Would you be able to send a fix or revert? Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152712/new/

https://reviews.llvm.org/D152712

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added a comment.

@leonardchan yes, I'm looking into. -6 is ENXIO? Is it using POSIX code path? 
Is it possible to see symbolicated stack trace of the crash?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152712/new/

https://reviews.llvm.org/D152712

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152757: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 531950.
splhack added a comment.

add comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152757/new/

https://reviews.llvm.org/D152757

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/unittests/ObjectFile/ELF/CMakeLists.txt
  lldb/unittests/ObjectFile/ELF/Inputs/liboffset-test.so
  lldb/unittests/ObjectFile/ELF/Inputs/offset-test.bin
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,39 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 0UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 3600UL);
+}
+
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithOffsetFile) {
+  // The contents of offset-test.bin are
+  // -0-1023: \0
+  // - 1024-4623: liboffset-test.so (offset: 1024, size: 3600, CRC32: 7D6E4738)
+  // - 4624-4639: \0
+  std::string SO = GetInputFilePath("offset-test.bin");
+  ModuleSpecList Specs;
+  ASSERT_EQ(
+  1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 1024, 3600, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 1024UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 4640UL);
+}
+
 TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
   /*
   // nosym-entrypoint-arm-thumb.s
Index: lldb/unittests/ObjectFile/ELF/CMakeLists.txt
===
--- lldb/unittests/ObjectFile/ELF/CMakeLists.txt
+++ lldb/unittests/ObjectFile/ELF/CMakeLists.txt
@@ -11,5 +11,7 @@
 
 set(test_inputs
   early-section-headers.so
+  liboffset-test.so
+  offset-test.bin
   )
 add_unittest_inputs(ObjectFileELFTests "${test_inputs}")
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -555,6 +555,13 @@
 if (header.Parse(data, &header_offset)) {
   if (data_sp) {
 ModuleSpec spec(file);
+// In Android API level 23 and above, bionic dynamic linker is able to
+// load .so file directly from zip file. In that case, .so file is
+// page aligned and uncompressed, and this module spec should retain 
the
+// .so file offset and file size to pass throught the information from
+// lldb-server to LLDB.
+spec.SetObjectOffset(file_offset);
+spec.SetObjectSize(length);
 
 const uint32_t sub_type = subTypeFromElfHeader(header);
 spec.GetArchitecture().SetArchitecture(
@@ -586,8 +593,12 @@
   __FUNCTION__, file.GetPath().c_str());
   }
 
+  // When ELF file does not contain GNU build ID, the later code will
+  // calculate CRC32 with this data_sp file_offset and length. It is
+  // important for Android zip .so file, which is a slice of a file,
+  // to not access the outside of the file slice range.
   if (data_sp->GetByteSize() < length)
-data_sp = MapFileData(file, -1, file_offset);
+data_sp = MapFileData(file, length, file_offset);
   if (data_sp)
 data.SetData(data_sp);
   // In case there is header extension in the section #0, the header we


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,39 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset

[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added a comment.

found symbolicated stack trace in
https://lab.llvm.org/buildbot/#/builders/96/builds/40769/steps/6/logs/stdio

TerminalTest does not instantiate FileSystem. now preparing to submit a diff to 
fix it.

   #0 0xcc46d6d4 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xa06d4)
   #1 0xcc46bc08 llvm::sys::RunSignalHandlers() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0x9ec08)
   #2 0xcc46df78 SignalHandler(int) Signals.cpp:0:0
   #3 0xb0a38598 (linux-vdso.so.1+0x598)
   #4 0xb023f200 __pthread_kill_implementation 
./nptl/./nptl/pthread_kill.c:44:76
   #5 0xb01fa67c gsignal ./signal/../sysdeps/posix/raise.c:27:6
   #6 0xb01e7130 abort ./stdlib/./stdlib/abort.c:81:7
   #7 0xcc3f62e4 testing::AssertionResult 
testing::internal::CmpHelperEQ(char 
const*, char const*, lldb_private::URI const&, lldb_private::URI const&) 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0x292e4)
   #8 0xcc4bb1d0 lldb_private::FileSystem::Instance() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xee1d0)
   #9 0xcc4c3f18 lldb_private::PseudoTerminal::OpenSecondary(int) 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xf6f18)
  #10 0xcc45304c TerminalTest::SetUp() crtstuff.c:0:0
  #11 0xcc48bccc testing::Test::Run() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xbeccc)
  #12 0xcc48d580 testing::TestInfo::Run() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xc0580)
  #13 0xcc48dca8 testing::TestSuite::Run() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xc0ca8)
  #14 0xcc49b55c testing::internal::UnitTestImpl::RunAllTests() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xce55c)
  #15 0xcc49adb8 testing::UnitTest::Run() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xcddb8)
  #16 0xcc483540 main 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xb6540)
  #17 0xb01e73fc __libc_start_call_main 
./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
  #18 0xb01e74cc call_init ./csu/../csu/libc-start.c:128:20
  #19 0xb01e74cc __libc_start_main ./csu/../csu/libc-start.c:379:5
  #20 0xcc3f4670 _start 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0x27670)
  --
  exit: -6
  --


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152712/new/

https://reviews.llvm.org/D152712

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152757: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 531954.
splhack added a comment.

added non zip .so file (normal file) case to the comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152757/new/

https://reviews.llvm.org/D152757

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/unittests/ObjectFile/ELF/CMakeLists.txt
  lldb/unittests/ObjectFile/ELF/Inputs/liboffset-test.so
  lldb/unittests/ObjectFile/ELF/Inputs/offset-test.bin
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,39 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 0UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 3600UL);
+}
+
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithOffsetFile) {
+  // The contents of offset-test.bin are
+  // -0-1023: \0
+  // - 1024-4623: liboffset-test.so (offset: 1024, size: 3600, CRC32: 7D6E4738)
+  // - 4624-4639: \0
+  std::string SO = GetInputFilePath("offset-test.bin");
+  ModuleSpecList Specs;
+  ASSERT_EQ(
+  1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 1024, 3600, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 1024UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 4640UL);
+}
+
 TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
   /*
   // nosym-entrypoint-arm-thumb.s
Index: lldb/unittests/ObjectFile/ELF/CMakeLists.txt
===
--- lldb/unittests/ObjectFile/ELF/CMakeLists.txt
+++ lldb/unittests/ObjectFile/ELF/CMakeLists.txt
@@ -11,5 +11,7 @@
 
 set(test_inputs
   early-section-headers.so
+  liboffset-test.so
+  offset-test.bin
   )
 add_unittest_inputs(ObjectFileELFTests "${test_inputs}")
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -555,6 +555,14 @@
 if (header.Parse(data, &header_offset)) {
   if (data_sp) {
 ModuleSpec spec(file);
+// In Android API level 23 and above, bionic dynamic linker is able to
+// load .so file directly from zip file. In that case, .so file is
+// page aligned and uncompressed, and this module spec should retain 
the
+// .so file offset and file size to pass throught the information from
+// lldb-server to LLDB. For normal file, file_offset should be 0,
+// length should be the size of the file.
+spec.SetObjectOffset(file_offset);
+spec.SetObjectSize(length);
 
 const uint32_t sub_type = subTypeFromElfHeader(header);
 spec.GetArchitecture().SetArchitecture(
@@ -586,8 +594,12 @@
   __FUNCTION__, file.GetPath().c_str());
   }
 
+  // When ELF file does not contain GNU build ID, the later code will
+  // calculate CRC32 with this data_sp file_offset and length. It is
+  // important for Android zip .so file, which is a slice of a file,
+  // to not access the outside of the file slice range.
   if (data_sp->GetByteSize() < length)
-data_sp = MapFileData(file, -1, file_offset);
+data_sp = MapFileData(file, length, file_offset);
   if (data_sp)
 data.SetData(data_sp);
   // In case there is header extension in the section #0, the header we


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,39 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIn

[Lldb-commits] [lldb] 538df1d - lldb [NFC] Add logging to Process when address masks are updated

2023-06-15 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2023-06-15T17:38:45-07:00
New Revision: 538df1d8a2428d06de7f26c859969ddec14928c1

URL: 
https://github.com/llvm/llvm-project/commit/538df1d8a2428d06de7f26c859969ddec14928c1
DIFF: 
https://github.com/llvm/llvm-project/commit/538df1d8a2428d06de7f26c859969ddec14928c1.diff

LOG: lldb [NFC] Add logging to Process when address masks are updated

To aid in integration testing/debugging. Verifying that the address
mask/addressable bits values from different sources are correctly
registered by lldb.

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 3b9de13206ac8..907ef50a7a61b 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1376,21 +1376,11 @@ class Process : public 
std::enable_shared_from_this,
   lldb::addr_t GetHighmemCodeAddressMask();
   lldb::addr_t GetHighmemDataAddressMask();
 
-  void SetCodeAddressMask(lldb::addr_t code_address_mask) {
-m_code_address_mask = code_address_mask;
-  }
-
-  void SetDataAddressMask(lldb::addr_t data_address_mask) {
-m_data_address_mask = data_address_mask;
-  }
+  void SetCodeAddressMask(lldb::addr_t code_address_mask);
+  void SetDataAddressMask(lldb::addr_t data_address_mask);
 
-  void SetHighmemCodeAddressMask(lldb::addr_t code_address_mask) {
-m_highmem_code_address_mask = code_address_mask;
-  }
-
-  void SetHighmemDataAddressMask(lldb::addr_t data_address_mask) {
-m_highmem_data_address_mask = data_address_mask;
-  }
+  void SetHighmemCodeAddressMask(lldb::addr_t code_address_mask);
+  void SetHighmemDataAddressMask(lldb::addr_t data_address_mask);
 
   /// Some targets might use bits in a code address to indicate a mode switch,
   /// ARM uses bit zero to signify a code address is thumb, so any ARM ABI

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 9730d7217eda3..7ad62976d968d 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -5688,6 +5688,34 @@ lldb::addr_t Process::GetHighmemDataAddressMask() {
   return GetDataAddressMask();
 }
 
+void Process::SetCodeAddressMask(lldb::addr_t code_address_mask) {
+  Log *log = GetLog(LLDBLog::Process);
+  LLDB_LOGF(log, "Setting Process code address mask to 0x%" PRIx64,
+code_address_mask);
+  m_code_address_mask = code_address_mask;
+}
+
+void Process::SetDataAddressMask(lldb::addr_t data_address_mask) {
+  Log *log = GetLog(LLDBLog::Process);
+  LLDB_LOGF(log, "Setting Process data address mask to 0x%" PRIx64,
+data_address_mask);
+  m_data_address_mask = data_address_mask;
+}
+
+void Process::SetHighmemCodeAddressMask(lldb::addr_t code_address_mask) {
+  Log *log = GetLog(LLDBLog::Process);
+  LLDB_LOGF(log, "Setting Process highmem code address mask to 0x%" PRIx64,
+code_address_mask);
+  m_highmem_code_address_mask = code_address_mask;
+}
+
+void Process::SetHighmemDataAddressMask(lldb::addr_t data_address_mask) {
+  Log *log = GetLog(LLDBLog::Process);
+  LLDB_LOGF(log, "Setting Process highmem data address mask to 0x%" PRIx64,
+data_address_mask);
+  m_highmem_data_address_mask = data_address_mask;
+}
+
 addr_t Process::FixCodeAddress(addr_t addr) {
   if (ABISP abi_sp = GetABI())
 addr = abi_sp->FixCodeAddress(addr);



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153088: [lldb] De-virtualize applicable functions in ValueObject (NFC)

2023-06-15 Thread Dave Lee via Phabricator via lldb-commits
kastiglione created this revision.
kastiglione added reviewers: bulbazord, jingham.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Remove `virtual` from `ValueObject` functions that aren't overridden.

One such function, `IsArrayItemForPointer`, is not called and so is instead 
deleted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153088

Files:
  lldb/include/lldb/Core/ValueObject.h


Index: lldb/include/lldb/Core/ValueObject.h
===
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -370,26 +370,26 @@
 return GetCompilerType().GetTypeName();
   }
 
-  virtual lldb::LanguageType GetObjectRuntimeLanguage() {
+  lldb::LanguageType GetObjectRuntimeLanguage() {
 return GetCompilerType().GetMinimumLanguage();
   }
 
-  virtual uint32_t
+  uint32_t
   GetTypeInfo(CompilerType *pointee_or_element_compiler_type = nullptr) {
 return GetCompilerType().GetTypeInfo(pointee_or_element_compiler_type);
   }
 
-  virtual bool IsPointerType() { return GetCompilerType().IsPointerType(); }
+  bool IsPointerType() { return GetCompilerType().IsPointerType(); }
 
-  virtual bool IsArrayType() { return GetCompilerType().IsArrayType(); }
+  bool IsArrayType() { return GetCompilerType().IsArrayType(); }
 
-  virtual bool IsScalarType() { return GetCompilerType().IsScalarType(); }
+  bool IsScalarType() { return GetCompilerType().IsScalarType(); }
 
-  virtual bool IsPointerOrReferenceType() {
+  bool IsPointerOrReferenceType() {
 return GetCompilerType().IsPointerOrReferenceType();
   }
 
-  virtual bool IsPossibleDynamicType();
+  bool IsPossibleDynamicType();
 
   bool IsNilReference();
 
@@ -429,10 +429,6 @@
 return (GetBitfieldBitSize() != 0) || (GetBitfieldBitOffset() != 0);
   }
 
-  virtual bool IsArrayItemForPointer() {
-return m_flags.m_is_array_item_for_pointer;
-  }
-
   virtual const char *GetValueAsCString();
 
   virtual bool GetValueAsCString(const lldb_private::TypeFormatImpl &format,
@@ -628,7 +624,7 @@
 
   // The backing bits of this value object were updated, clear any descriptive
   // string, so we know we have to refetch them.
-  virtual void ValueUpdated() {
+  void ValueUpdated() {
 ClearUserVisibleData(eClearUserVisibleDataItemsValue |
  eClearUserVisibleDataItemsSummary |
  eClearUserVisibleDataItemsDescription);


Index: lldb/include/lldb/Core/ValueObject.h
===
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -370,26 +370,26 @@
 return GetCompilerType().GetTypeName();
   }
 
-  virtual lldb::LanguageType GetObjectRuntimeLanguage() {
+  lldb::LanguageType GetObjectRuntimeLanguage() {
 return GetCompilerType().GetMinimumLanguage();
   }
 
-  virtual uint32_t
+  uint32_t
   GetTypeInfo(CompilerType *pointee_or_element_compiler_type = nullptr) {
 return GetCompilerType().GetTypeInfo(pointee_or_element_compiler_type);
   }
 
-  virtual bool IsPointerType() { return GetCompilerType().IsPointerType(); }
+  bool IsPointerType() { return GetCompilerType().IsPointerType(); }
 
-  virtual bool IsArrayType() { return GetCompilerType().IsArrayType(); }
+  bool IsArrayType() { return GetCompilerType().IsArrayType(); }
 
-  virtual bool IsScalarType() { return GetCompilerType().IsScalarType(); }
+  bool IsScalarType() { return GetCompilerType().IsScalarType(); }
 
-  virtual bool IsPointerOrReferenceType() {
+  bool IsPointerOrReferenceType() {
 return GetCompilerType().IsPointerOrReferenceType();
   }
 
-  virtual bool IsPossibleDynamicType();
+  bool IsPossibleDynamicType();
 
   bool IsNilReference();
 
@@ -429,10 +429,6 @@
 return (GetBitfieldBitSize() != 0) || (GetBitfieldBitOffset() != 0);
   }
 
-  virtual bool IsArrayItemForPointer() {
-return m_flags.m_is_array_item_for_pointer;
-  }
-
   virtual const char *GetValueAsCString();
 
   virtual bool GetValueAsCString(const lldb_private::TypeFormatImpl &format,
@@ -628,7 +624,7 @@
 
   // The backing bits of this value object were updated, clear any descriptive
   // string, so we know we have to refetch them.
-  virtual void ValueUpdated() {
+  void ValueUpdated() {
 ClearUserVisibleData(eClearUserVisibleDataItemsValue |
  eClearUserVisibleDataItemsSummary |
  eClearUserVisibleDataItemsDescription);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153088: [lldb] De-virtualize applicable functions in ValueObject (NFC)

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: JDevlieghere.

Big fan of this kind of cleanup. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153088/new/

https://reviews.llvm.org/D153088

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153091: [lldb][TerminalTest] Fix assertion failure

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
splhack requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

D152712  replaced 
`llvm::sys::RetryAfterSignal(-1, ::open)` with
`FileSystem::Instance().Open` for bionic in PseudoTerminal::OpenSecondary, and
FileSystem::Instance() is failing with assertion on arm Linux.

The assertion should be FileSystem re-initialization check, therefore the
hypothesis is that TerminalTest tests are initializing FileSystem instance
repeatedly.

Use SubsystemRAII to ensure tearing down the FileSystem instance.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153091

Files:
  lldb/unittests/Host/posix/TerminalTest.cpp


Index: lldb/unittests/Host/posix/TerminalTest.cpp
===
--- lldb/unittests/Host/posix/TerminalTest.cpp
+++ lldb/unittests/Host/posix/TerminalTest.cpp
@@ -6,9 +6,11 @@
 //
 
//===--===//
 
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Terminal.h"
 #include "llvm/Testing/Support/Error.h"
+#include "TestingSupport/SubsystemRAII.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -20,6 +22,7 @@
 
 class TerminalTest : public ::testing::Test {
 protected:
+  SubsystemRAII subsystems;
   PseudoTerminal m_pty;
   int m_fd;
   Terminal m_term;


Index: lldb/unittests/Host/posix/TerminalTest.cpp
===
--- lldb/unittests/Host/posix/TerminalTest.cpp
+++ lldb/unittests/Host/posix/TerminalTest.cpp
@@ -6,9 +6,11 @@
 //
 //===--===//
 
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Terminal.h"
 #include "llvm/Testing/Support/Error.h"
+#include "TestingSupport/SubsystemRAII.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -20,6 +22,7 @@
 
 class TerminalTest : public ::testing::Test {
 protected:
+  SubsystemRAII subsystems;
   PseudoTerminal m_pty;
   int m_fd;
   Terminal m_term;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added a comment.

@leonardchan I think D153091  will fix the 
abort (`-6` should be `SIGABRT`), therefore it should be the assertion check of 
re-initialization in `FileSystem::Instance`.
Is there a way to run the Linux builder with D153091 
 to make sure it will fix it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152712/new/

https://reviews.llvm.org/D152712

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153060: lldb: do more than 1 kilobyte at a time to vastly increase binary sync speed

2023-06-15 Thread Russell Greene via Phabricator via lldb-commits
russelltg added a comment.

Thanks for the reviews and approval. I don't have git access so if someone 
could push this that'd be fantastic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153060/new/

https://reviews.llvm.org/D153060

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 531969.
splhack added a comment.

Rename `ResolveBionicPath` to `ResolveSharedLibraryPath`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

Files:
  lldb/include/lldb/Host/common/ZipFileResolver.h
  lldb/include/lldb/Utility/ZipFile.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/ZipFileResolver.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/ZipFile.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/common/CMakeLists.txt
  lldb/unittests/Host/common/Inputs/zip-test.zip
  lldb/unittests/Host/common/ZipFileResolverTest.cpp

Index: lldb/unittests/Host/common/ZipFileResolverTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/common/ZipFileResolverTest.cpp
@@ -0,0 +1,72 @@
+//===-- ZipFileResolverTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/common/ZipFileResolver.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace llvm;
+
+namespace {
+class ZipFileResolverTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+};
+
+std::string TestZipPath() {
+  FileSpec zip_spec(GetInputFilePath("zip-test.zip"));
+  FileSystem::Instance().Resolve(zip_spec);
+  return zip_spec.GetPath();
+}
+} // namespace
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithNormalFile) {
+  const FileSpec file_spec("/system/lib64/libtest.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_TRUE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+
+  EXPECT_EQ(file_kind, ZipFileResolver::FileKind::eFileKindNormal);
+  EXPECT_EQ(file_path, file_spec.GetPath());
+  EXPECT_EQ(file_offset, 0UL);
+  EXPECT_EQ(file_size, 0UL);
+}
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithZipMissing) {
+  const std::string zip_path = TestZipPath();
+  const FileSpec file_spec(zip_path + "!/lib/arm64-v8a/libmissing.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_FALSE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+}
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithZipExisting) {
+  const std::string zip_path = TestZipPath();
+  const FileSpec file_spec(zip_path + "!/lib/arm64-v8a/libzip-test.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_TRUE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+
+  EXPECT_EQ(file_kind, ZipFileResolver::FileKind::eFileKindZip);
+  EXPECT_EQ(file_path, zip_path);
+  EXPECT_EQ(file_offset, 4096UL);
+  EXPECT_EQ(file_size, 3600UL);
+}
Index: lldb/unittests/Host/common/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Host/common/CMakeLists.txt
@@ -0,0 +1,15 @@
+set (FILES
+  ZipFileResolverTest.cpp
+)
+
+add_lldb_unittest(HostCommonTests
+  ${FILES}
+  LINK_LIBS
+lldbHost
+lldbUtilityHelpers
+  )
+
+set(test_inputs
+  zip-test.zip
+  )
+add_unittest_inputs(HostCommonTests "${test_inputs}")
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -38,3 +38,5 @@
 LLVMTestingSupport
 LLVMTargetParser
   )
+
+add_subdirectory(common)
Index: lldb/source/Utility/ZipFile.cpp
===
--- /dev/null
+++ lldb/source/Utility/ZipFile.cpp
@@ -0,0 +1,180 @@
+//===-- ZipFile.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Utility/ZipFile.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/Support/Endian.h"
+
+using namespace 

[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1336
+  // "zip_path!/so_path". Resolve the zip file path, .so file offset and size.
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;

clayborg wrote:
> might be good to init this to an invalid value in case code changes over time?
yup, sounds good. will add with assert.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 531974.
splhack added a comment.

Added eFileKindInvalid with assert


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

Files:
  lldb/include/lldb/Host/common/ZipFileResolver.h
  lldb/include/lldb/Utility/ZipFile.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/ZipFileResolver.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/ZipFile.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/common/CMakeLists.txt
  lldb/unittests/Host/common/Inputs/zip-test.zip
  lldb/unittests/Host/common/ZipFileResolverTest.cpp

Index: lldb/unittests/Host/common/ZipFileResolverTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/common/ZipFileResolverTest.cpp
@@ -0,0 +1,72 @@
+//===-- ZipFileResolverTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/common/ZipFileResolver.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace llvm;
+
+namespace {
+class ZipFileResolverTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+};
+
+std::string TestZipPath() {
+  FileSpec zip_spec(GetInputFilePath("zip-test.zip"));
+  FileSystem::Instance().Resolve(zip_spec);
+  return zip_spec.GetPath();
+}
+} // namespace
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithNormalFile) {
+  const FileSpec file_spec("/system/lib64/libtest.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_TRUE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+
+  EXPECT_EQ(file_kind, ZipFileResolver::FileKind::eFileKindNormal);
+  EXPECT_EQ(file_path, file_spec.GetPath());
+  EXPECT_EQ(file_offset, 0UL);
+  EXPECT_EQ(file_size, 0UL);
+}
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithZipMissing) {
+  const std::string zip_path = TestZipPath();
+  const FileSpec file_spec(zip_path + "!/lib/arm64-v8a/libmissing.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_FALSE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+}
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithZipExisting) {
+  const std::string zip_path = TestZipPath();
+  const FileSpec file_spec(zip_path + "!/lib/arm64-v8a/libzip-test.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_TRUE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+
+  EXPECT_EQ(file_kind, ZipFileResolver::FileKind::eFileKindZip);
+  EXPECT_EQ(file_path, zip_path);
+  EXPECT_EQ(file_offset, 4096UL);
+  EXPECT_EQ(file_size, 3600UL);
+}
Index: lldb/unittests/Host/common/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Host/common/CMakeLists.txt
@@ -0,0 +1,15 @@
+set (FILES
+  ZipFileResolverTest.cpp
+)
+
+add_lldb_unittest(HostCommonTests
+  ${FILES}
+  LINK_LIBS
+lldbHost
+lldbUtilityHelpers
+  )
+
+set(test_inputs
+  zip-test.zip
+  )
+add_unittest_inputs(HostCommonTests "${test_inputs}")
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -38,3 +38,5 @@
 LLVMTestingSupport
 LLVMTargetParser
   )
+
+add_subdirectory(common)
Index: lldb/source/Utility/ZipFile.cpp
===
--- /dev/null
+++ lldb/source/Utility/ZipFile.cpp
@@ -0,0 +1,180 @@
+//===-- ZipFile.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Utility/ZipFile.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/Support/Endian.h"
+
+using namespace lldb_private;
+using na

[Lldb-commits] [PATCH] D153091: [lldb][TerminalTest] Fix assertion failure

2023-06-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153091/new/

https://reviews.llvm.org/D153091

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 75e93ec - [lldb][TerminalTest] Fix assertion failure

2023-06-15 Thread Kazuki Sakamoto via lldb-commits

Author: Kazuki Sakamoto
Date: 2023-06-15T21:24:40-07:00
New Revision: 75e93ec720739291640005681f062b0e1e8e0f53

URL: 
https://github.com/llvm/llvm-project/commit/75e93ec720739291640005681f062b0e1e8e0f53
DIFF: 
https://github.com/llvm/llvm-project/commit/75e93ec720739291640005681f062b0e1e8e0f53.diff

LOG: [lldb][TerminalTest] Fix assertion failure

D152712 replaced `llvm::sys::RetryAfterSignal(-1, ::open)` with
`FileSystem::Instance().Open` for bionic in PseudoTerminal::OpenSecondary, and
FileSystem::Instance() is failing with assertion on arm Linux.

The assertion should be FileSystem re-initialization check, therefore the
hypothesis is that TerminalTest tests are initializing FileSystem instance
repeatedly.

Use SubsystemRAII to ensure tearing down the FileSystem instance.

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

Added: 


Modified: 
lldb/unittests/Host/posix/TerminalTest.cpp

Removed: 




diff  --git a/lldb/unittests/Host/posix/TerminalTest.cpp 
b/lldb/unittests/Host/posix/TerminalTest.cpp
index 1cf7b9bc8f3b0..5187a0c20a68b 100644
--- a/lldb/unittests/Host/posix/TerminalTest.cpp
+++ b/lldb/unittests/Host/posix/TerminalTest.cpp
@@ -6,9 +6,11 @@
 //
 
//===--===//
 
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Terminal.h"
 #include "llvm/Testing/Support/Error.h"
+#include "TestingSupport/SubsystemRAII.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -20,6 +22,7 @@ using namespace lldb_private;
 
 class TerminalTest : public ::testing::Test {
 protected:
+  SubsystemRAII subsystems;
   PseudoTerminal m_pty;
   int m_fd;
   Terminal m_term;



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153091: [lldb][TerminalTest] Fix assertion failure

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG75e93ec72073: [lldb][TerminalTest] Fix assertion failure 
(authored by splhack).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153091/new/

https://reviews.llvm.org/D153091

Files:
  lldb/unittests/Host/posix/TerminalTest.cpp


Index: lldb/unittests/Host/posix/TerminalTest.cpp
===
--- lldb/unittests/Host/posix/TerminalTest.cpp
+++ lldb/unittests/Host/posix/TerminalTest.cpp
@@ -6,9 +6,11 @@
 //
 
//===--===//
 
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Terminal.h"
 #include "llvm/Testing/Support/Error.h"
+#include "TestingSupport/SubsystemRAII.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -20,6 +22,7 @@
 
 class TerminalTest : public ::testing::Test {
 protected:
+  SubsystemRAII subsystems;
   PseudoTerminal m_pty;
   int m_fd;
   Terminal m_term;


Index: lldb/unittests/Host/posix/TerminalTest.cpp
===
--- lldb/unittests/Host/posix/TerminalTest.cpp
+++ lldb/unittests/Host/posix/TerminalTest.cpp
@@ -6,9 +6,11 @@
 //
 //===--===//
 
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Terminal.h"
 #include "llvm/Testing/Support/Error.h"
+#include "TestingSupport/SubsystemRAII.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -20,6 +22,7 @@
 
 class TerminalTest : public ::testing::Test {
 protected:
+  SubsystemRAII subsystems;
   PseudoTerminal m_pty;
   int m_fd;
   Terminal m_term;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added a comment.

Verified D153091  fixed TerminalTest failures.

from `Failed Tests (13)`
https://lab.llvm.org/buildbot/#/builders/96/builds/40769

to `Failed Tests (1)`
https://lab.llvm.org/buildbot/#/builders/96/builds/40781

will submit a diff to fix the last one, MainLoopTest.DetectsEOF

  [ RUN  ] MainLoopTest.DetectsEOF
   #0 0xe2a5c738 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xa0738)
   #1 0xe2a5ac6c llvm::sys::RunSignalHandlers() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0x9ec6c)
   #2 0xe2a5cfdc SignalHandler(int) Signals.cpp:0:0
   #3 0x957c0598 (linux-vdso.so.1+0x598)
   #4 0x94fcf200 __pthread_kill_implementation 
./nptl/./nptl/pthread_kill.c:44:76
   #5 0x94f8a67c gsignal ./signal/../sysdeps/posix/raise.c:27:6
   #6 0x94f77130 abort ./stdlib/./stdlib/abort.c:81:7
   #7 0xe29e52e4 testing::AssertionResult 
testing::internal::CmpHelperEQ(char 
const*, char const*, lldb_private::URI const&, lldb_private::URI const&) 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0x292e4)
   #8 0xe2aaa234 lldb_private::FileSystem::Instance() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xee234)
   #9 0xe2ab2f7c lldb_private::PseudoTerminal::OpenSecondary(int) 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xf6f7c)
  #10 0xe29f5458 MainLoopTest_DetectsEOF_Test::TestBody() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0x39458)
  #11 0xe2a7ae80 testing::Test::Run() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xbee80)
  #12 0xe2a7c5e4 testing::TestInfo::Run() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xc05e4)
  #13 0xe2a7cd0c testing::TestSuite::Run() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xc0d0c)
  #14 0xe2a8a5c0 testing::internal::UnitTestImpl::RunAllTests() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xce5c0)
  #15 0xe2a89e1c testing::UnitTest::Run() 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xcde1c)
  #16 0xe2a725a4 main 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0xb65a4)
  #17 0x94f773fc __libc_start_call_main 
./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
  #18 0x94f774cc call_init ./csu/../csu/libc-start.c:128:20
  #19 0x94f774cc __libc_start_main ./csu/../csu/libc-start.c:379:5
  #20 0xe29e3670 _start 
(/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/unittests/Host/./HostTests+0x27670)
  
  --
  exit: -6


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152712/new/

https://reviews.llvm.org/D152712

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153043: [lldb] Fix handling of cfi_restore in the unwinder

2023-06-15 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin updated this revision to Diff 531987.
jarin added a comment.

Added a comment explaining that we are restoring the caller's value of the 
register.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153043/new/

https://reviews.llvm.org/D153043

Files:
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
  lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test


Index: lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
@@ -0,0 +1,21 @@
+# Test restoring of register values.
+
+# UNSUPPORTED: system-windows
+# REQUIRES: target-x86_64, native
+
+# RUN: %clang_host %p/Inputs/call-asm.c 
%p/Inputs/eh-frame-dwarf-unwind-abort.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+process launch
+# CHECK: stop reason = signal SIGTRAP
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`asm_main + 23
+# CHECK: frame #1: {{.*}}`main + {{.*}}
+
+target modules show-unwind -n asm_main
+# CHECK: eh_frame UnwindPlan:
+# CHECK: row[0]:0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK: row[1]:   14: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[2]:   17: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[3]:   22: CFA=rsp +8 => rip=[CFA-8]
Index: lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
@@ -0,0 +1,25 @@
+.text
+.globl  asm_main
+asm_main:
+.cfi_startproc
+cmpb $0x0, g_hard_abort(%rip)
+jne .L
+
+pushq   %rbp
+.cfi_def_cfa_offset 16
+.cfi_offset 6, -16
+movq%rsp, %rbp
+.cfi_def_cfa_register 6
+callq   abort
+.L:
+.cfi_def_cfa 7, 8
+.cfi_restore 6
+int3
+ud2
+.cfi_endproc
+
+   .data
+   .globl  g_hard_abort
+g_hard_abort:
+   .byte   1
+   .size   g_hard_abort, 1
\ No newline at end of file
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -674,6 +674,11 @@
   unwind_plan.GetRowAtIndex(0)->GetRegisterInfo(reg_num,
 reg_location))
 row->SetRegisterInfo(reg_num, reg_location);
+  else {
+// If the register was not set in the first row, remove the
+// register info to keep the unmodified value from the caller.
+row->RemoveRegisterInfo(reg_num);
+  }
   break;
 }
 }


Index: lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
@@ -0,0 +1,21 @@
+# Test restoring of register values.
+
+# UNSUPPORTED: system-windows
+# REQUIRES: target-x86_64, native
+
+# RUN: %clang_host %p/Inputs/call-asm.c %p/Inputs/eh-frame-dwarf-unwind-abort.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+process launch
+# CHECK: stop reason = signal SIGTRAP
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`asm_main + 23
+# CHECK: frame #1: {{.*}}`main + {{.*}}
+
+target modules show-unwind -n asm_main
+# CHECK: eh_frame UnwindPlan:
+# CHECK: row[0]:0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK: row[1]:   14: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[2]:   17: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[3]:   22: CFA=rsp +8 => rip=[CFA-8]
Index: lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
@@ -0,0 +1,25 @@
+.text
+.globl  asm_main
+asm_main:
+.cfi_startproc
+cmpb $0x0, g_hard_abort(%rip)
+jne .L
+
+pushq   %rbp
+.cfi_def_cfa_offset 16
+.cfi_offset 6, -16
+movq%rsp, %rbp
+.cfi_def_cfa_register 6
+callq   abort
+.L:
+.cfi_def_cfa 7, 8
+.cfi_restore 6
+int3
+ud2
+.cfi_endproc
+
+	.data
+	.globl  g_hard_abort
+g_hard_abort:
+	.byte   1
+	.size   g_hard_abort, 1
\ No newline at end of file
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -674,6 +674,11 @@
   unwind_plan.GetRowAtIndex(0)->GetRegisterInfo(reg_num,
 reg_location))
 row->SetRegisterInfo(reg_num, reg_location);
+  else {
+// If the register was not set in the 

[Lldb-commits] [PATCH] D153102: [lldb][MainLoopTest] Fix assertion failure on arm Linux

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack created this revision.
splhack added reviewers: clayborg, labath, bulbazord, yinghuitan, JDevlieghere, 
leonardchan.
Herald added subscribers: omjavaid, kristof.beyls.
Herald added a project: All.
splhack requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

D152712  replaced 
`llvm::sys::RetryAfterSignal(-1, ::open)` with
`FileSystem::Instance().Open()` for bionic and `FileSystem::Instance()` is
failing with the re-initialization assertion on arm Linux test job.
This is because MainLoopTest does not tear down the FileSystem instance.

https://lab.llvm.org/buildbot/#/builders/96/builds/40781

Add FileSystem to SubsystemRAII<> to ensure destroying the FileSystem instance.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153102

Files:
  lldb/unittests/Host/MainLoopTest.cpp


Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -9,6 +9,7 @@
 #include "lldb/Host/MainLoop.h"
 #include "TestingSupport/SubsystemRAII.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "llvm/Testing/Support/Error.h"
@@ -20,7 +21,7 @@
 namespace {
 class MainLoopTest : public testing::Test {
 public:
-  SubsystemRAII subsystems;
+  SubsystemRAII subsystems;
 
   void SetUp() override {
 bool child_processes_inherit = false;


Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -9,6 +9,7 @@
 #include "lldb/Host/MainLoop.h"
 #include "TestingSupport/SubsystemRAII.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "llvm/Testing/Support/Error.h"
@@ -20,7 +21,7 @@
 namespace {
 class MainLoopTest : public testing::Test {
 public:
-  SubsystemRAII subsystems;
+  SubsystemRAII subsystems;
 
   void SetUp() override {
 bool child_processes_inherit = false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152712: [lldb][Android] Use a lambda for calls to ::open in RetryAfterSignal

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack added a comment.

D153102  should fix the last test failure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152712/new/

https://reviews.llvm.org/D152712

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152757: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 531990.
splhack added a comment.

fixed typo and rebase onto D153102 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152757/new/

https://reviews.llvm.org/D152757

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/unittests/ObjectFile/ELF/CMakeLists.txt
  lldb/unittests/ObjectFile/ELF/Inputs/liboffset-test.so
  lldb/unittests/ObjectFile/ELF/Inputs/offset-test.bin
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,39 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 0UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 3600UL);
+}
+
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithOffsetFile) {
+  // The contents of offset-test.bin are
+  // -0-1023: \0
+  // - 1024-4623: liboffset-test.so (offset: 1024, size: 3600, CRC32: 7D6E4738)
+  // - 4624-4639: \0
+  std::string SO = GetInputFilePath("offset-test.bin");
+  ModuleSpecList Specs;
+  ASSERT_EQ(
+  1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 1024, 3600, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 1024UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 4640UL);
+}
+
 TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
   /*
   // nosym-entrypoint-arm-thumb.s
Index: lldb/unittests/ObjectFile/ELF/CMakeLists.txt
===
--- lldb/unittests/ObjectFile/ELF/CMakeLists.txt
+++ lldb/unittests/ObjectFile/ELF/CMakeLists.txt
@@ -11,5 +11,7 @@
 
 set(test_inputs
   early-section-headers.so
+  liboffset-test.so
+  offset-test.bin
   )
 add_unittest_inputs(ObjectFileELFTests "${test_inputs}")
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -555,6 +555,14 @@
 if (header.Parse(data, &header_offset)) {
   if (data_sp) {
 ModuleSpec spec(file);
+// In Android API level 23 and above, bionic dynamic linker is able to
+// load .so file directly from zip file. In that case, .so file is
+// page aligned and uncompressed, and this module spec should retain 
the
+// .so file offset and file size to pass through the information from
+// lldb-server to LLDB. For normal file, file_offset should be 0,
+// length should be the size of the file.
+spec.SetObjectOffset(file_offset);
+spec.SetObjectSize(length);
 
 const uint32_t sub_type = subTypeFromElfHeader(header);
 spec.GetArchitecture().SetArchitecture(
@@ -586,8 +594,12 @@
   __FUNCTION__, file.GetPath().c_str());
   }
 
+  // When ELF file does not contain GNU build ID, the later code will
+  // calculate CRC32 with this data_sp file_offset and length. It is
+  // important for Android zip .so file, which is a slice of a file,
+  // to not access the outside of the file slice range.
   if (data_sp->GetByteSize() < length)
-data_sp = MapFileData(file, -1, file_offset);
+data_sp = MapFileData(file, length, file_offset);
   if (data_sp)
 data.SetData(data_sp);
   // In case there is header extension in the section #0, the header we


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,39 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetMo

[Lldb-commits] [lldb] 07b9e6e - [lldb] Fix handling of cfi_restore in the unwinder

2023-06-15 Thread Jaroslav Sevcik via lldb-commits

Author: Jaroslav Sevcik
Date: 2023-06-16T08:01:29+02:00
New Revision: 07b9e6ed0db206912b99c153a6bcfcb41f792cc5

URL: 
https://github.com/llvm/llvm-project/commit/07b9e6ed0db206912b99c153a6bcfcb41f792cc5
DIFF: 
https://github.com/llvm/llvm-project/commit/07b9e6ed0db206912b99c153a6bcfcb41f792cc5.diff

LOG: [lldb] Fix handling of cfi_restore in the unwinder

Currently, lldb's unwinder ignores cfi_restore opcodes for registers
that are not set in the first row of the unwinding info. This prevents
unwinding of failed assertion in Chrome/v8 (https://github.com/v8/v8).
The attached test is an x64 copy of v8's function that failed to unwind
correctly (V8_Fatal).

This patch changes handling of cfi_restore to reset the location if
the first unwind table row does not map the restored register.

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

Added: 
lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test

Modified: 
lldb/source/Symbol/DWARFCallFrameInfo.cpp

Removed: 




diff  --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp 
b/lldb/source/Symbol/DWARFCallFrameInfo.cpp
index be9f64356b4e0..dc54d13ae23cb 100644
--- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -674,6 +674,11 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t 
dwarf_offset,
   unwind_plan.GetRowAtIndex(0)->GetRegisterInfo(reg_num,
 reg_location))
 row->SetRegisterInfo(reg_num, reg_location);
+  else {
+// If the register was not set in the first row, remove the
+// register info to keep the unmodified value from the caller.
+row->RemoveRegisterInfo(reg_num);
+  }
   break;
 }
 }

diff  --git a/lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s 
b/lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
new file mode 100644
index 0..e03a69f67f8f9
--- /dev/null
+++ b/lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
@@ -0,0 +1,25 @@
+.text
+.globl  asm_main
+asm_main:
+.cfi_startproc
+cmpb $0x0, g_hard_abort(%rip)
+jne .L
+
+pushq   %rbp
+.cfi_def_cfa_offset 16
+.cfi_offset 6, -16
+movq%rsp, %rbp
+.cfi_def_cfa_register 6
+callq   abort
+.L:
+.cfi_def_cfa 7, 8
+.cfi_restore 6
+int3
+ud2
+.cfi_endproc
+
+   .data
+   .globl  g_hard_abort
+g_hard_abort:
+   .byte   1
+   .size   g_hard_abort, 1
\ No newline at end of file

diff  --git a/lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test 
b/lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
new file mode 100644
index 0..ccf973d9313c2
--- /dev/null
+++ b/lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
@@ -0,0 +1,21 @@
+# Test restoring of register values.
+
+# UNSUPPORTED: system-windows
+# REQUIRES: target-x86_64, native
+
+# RUN: %clang_host %p/Inputs/call-asm.c 
%p/Inputs/eh-frame-dwarf-unwind-abort.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+process launch
+# CHECK: stop reason = signal SIGTRAP
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`asm_main + 23
+# CHECK: frame #1: {{.*}}`main + {{.*}}
+
+target modules show-unwind -n asm_main
+# CHECK: eh_frame UnwindPlan:
+# CHECK: row[0]:0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK: row[1]:   14: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[2]:   17: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[3]:   22: CFA=rsp +8 => rip=[CFA-8]



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D153043: [lldb] Fix handling of cfi_restore in the unwinder

2023-06-15 Thread Jaroslav Sevcik via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG07b9e6ed0db2: [lldb] Fix handling of cfi_restore in the 
unwinder (authored by jarin).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153043/new/

https://reviews.llvm.org/D153043

Files:
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
  lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test


Index: lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
@@ -0,0 +1,21 @@
+# Test restoring of register values.
+
+# UNSUPPORTED: system-windows
+# REQUIRES: target-x86_64, native
+
+# RUN: %clang_host %p/Inputs/call-asm.c 
%p/Inputs/eh-frame-dwarf-unwind-abort.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+process launch
+# CHECK: stop reason = signal SIGTRAP
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`asm_main + 23
+# CHECK: frame #1: {{.*}}`main + {{.*}}
+
+target modules show-unwind -n asm_main
+# CHECK: eh_frame UnwindPlan:
+# CHECK: row[0]:0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK: row[1]:   14: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[2]:   17: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[3]:   22: CFA=rsp +8 => rip=[CFA-8]
Index: lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
@@ -0,0 +1,25 @@
+.text
+.globl  asm_main
+asm_main:
+.cfi_startproc
+cmpb $0x0, g_hard_abort(%rip)
+jne .L
+
+pushq   %rbp
+.cfi_def_cfa_offset 16
+.cfi_offset 6, -16
+movq%rsp, %rbp
+.cfi_def_cfa_register 6
+callq   abort
+.L:
+.cfi_def_cfa 7, 8
+.cfi_restore 6
+int3
+ud2
+.cfi_endproc
+
+   .data
+   .globl  g_hard_abort
+g_hard_abort:
+   .byte   1
+   .size   g_hard_abort, 1
\ No newline at end of file
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -674,6 +674,11 @@
   unwind_plan.GetRowAtIndex(0)->GetRegisterInfo(reg_num,
 reg_location))
 row->SetRegisterInfo(reg_num, reg_location);
+  else {
+// If the register was not set in the first row, remove the
+// register info to keep the unmodified value from the caller.
+row->RemoveRegisterInfo(reg_num);
+  }
   break;
 }
 }


Index: lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test
@@ -0,0 +1,21 @@
+# Test restoring of register values.
+
+# UNSUPPORTED: system-windows
+# REQUIRES: target-x86_64, native
+
+# RUN: %clang_host %p/Inputs/call-asm.c %p/Inputs/eh-frame-dwarf-unwind-abort.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+process launch
+# CHECK: stop reason = signal SIGTRAP
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`asm_main + 23
+# CHECK: frame #1: {{.*}}`main + {{.*}}
+
+target modules show-unwind -n asm_main
+# CHECK: eh_frame UnwindPlan:
+# CHECK: row[0]:0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK: row[1]:   14: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[2]:   17: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[3]:   22: CFA=rsp +8 => rip=[CFA-8]
Index: lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s
@@ -0,0 +1,25 @@
+.text
+.globl  asm_main
+asm_main:
+.cfi_startproc
+cmpb $0x0, g_hard_abort(%rip)
+jne .L
+
+pushq   %rbp
+.cfi_def_cfa_offset 16
+.cfi_offset 6, -16
+movq%rsp, %rbp
+.cfi_def_cfa_register 6
+callq   abort
+.L:
+.cfi_def_cfa 7, 8
+.cfi_restore 6
+int3
+ud2
+.cfi_endproc
+
+	.data
+	.globl  g_hard_abort
+g_hard_abort:
+	.byte   1
+	.size   g_hard_abort, 1
\ No newline at end of file
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -674,6 +674,11 @@
   unwind_plan.GetRowAtIndex(0)->GetRegisterInfo(reg_num,
 reg_location))
 row->SetRegisterInfo(reg_num, reg_location);
+

[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits
splhack updated this revision to Diff 531992.
splhack added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

Files:
  lldb/include/lldb/Host/common/ZipFileResolver.h
  lldb/include/lldb/Utility/ZipFile.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/ZipFileResolver.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/ZipFile.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/common/CMakeLists.txt
  lldb/unittests/Host/common/Inputs/zip-test.zip
  lldb/unittests/Host/common/ZipFileResolverTest.cpp

Index: lldb/unittests/Host/common/ZipFileResolverTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/common/ZipFileResolverTest.cpp
@@ -0,0 +1,72 @@
+//===-- ZipFileResolverTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/common/ZipFileResolver.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace llvm;
+
+namespace {
+class ZipFileResolverTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+};
+
+std::string TestZipPath() {
+  FileSpec zip_spec(GetInputFilePath("zip-test.zip"));
+  FileSystem::Instance().Resolve(zip_spec);
+  return zip_spec.GetPath();
+}
+} // namespace
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithNormalFile) {
+  const FileSpec file_spec("/system/lib64/libtest.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_TRUE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+
+  EXPECT_EQ(file_kind, ZipFileResolver::FileKind::eFileKindNormal);
+  EXPECT_EQ(file_path, file_spec.GetPath());
+  EXPECT_EQ(file_offset, 0UL);
+  EXPECT_EQ(file_size, 0UL);
+}
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithZipMissing) {
+  const std::string zip_path = TestZipPath();
+  const FileSpec file_spec(zip_path + "!/lib/arm64-v8a/libmissing.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_FALSE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+}
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithZipExisting) {
+  const std::string zip_path = TestZipPath();
+  const FileSpec file_spec(zip_path + "!/lib/arm64-v8a/libzip-test.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_TRUE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+
+  EXPECT_EQ(file_kind, ZipFileResolver::FileKind::eFileKindZip);
+  EXPECT_EQ(file_path, zip_path);
+  EXPECT_EQ(file_offset, 4096UL);
+  EXPECT_EQ(file_size, 3600UL);
+}
Index: lldb/unittests/Host/common/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Host/common/CMakeLists.txt
@@ -0,0 +1,15 @@
+set (FILES
+  ZipFileResolverTest.cpp
+)
+
+add_lldb_unittest(HostCommonTests
+  ${FILES}
+  LINK_LIBS
+lldbHost
+lldbUtilityHelpers
+  )
+
+set(test_inputs
+  zip-test.zip
+  )
+add_unittest_inputs(HostCommonTests "${test_inputs}")
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -38,3 +38,5 @@
 LLVMTestingSupport
 LLVMTargetParser
   )
+
+add_subdirectory(common)
Index: lldb/source/Utility/ZipFile.cpp
===
--- /dev/null
+++ lldb/source/Utility/ZipFile.cpp
@@ -0,0 +1,180 @@
+//===-- ZipFile.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Utility/ZipFile.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/Support/Endian.h"
+
+using namespace lldb_private;
+using namespace llvm::support;
+
+na