[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

2022-12-16 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

Hi David, thanks for the ping sorry for not replying earlier to this.  I looked 
at this a little bit today but want to look at how I solved the same issue a 
while ago (but never upstreamed, which is my bad, I need to clean those up).  
(I did solved this in OptionArgParser::ToAddress and I haven't looked at this 
enough to argue for mine over your approach -- but mine gets you any address 
expression command like `source lookup -a` and `image lookup -a` etc.  I'll 
look at this more closely this weekend and follow up though.

(I got distracted way too much when I tried to make a repo test case, and my 
function pointer variable was called `f` and it would never work.  I looked at 
it with Jim and we were both confused mightily, until we noticed that 
OptionArgParser::ToAddress throws things through a "see if it can be parsed as 
base16 even without a prefix" so you can't use a variable name that consists of 
[a-f][0-9a-f]* lol.  Jim or I are definitely going to be removing that little 
bit of too-cleverness.)

At first I was wondering if the ABI should save the ArchSpec it is created with 
so it can call the Architecture's GetBreakableLoadAddress instead of requiring 
all the callers to call both of these, but I see from the ppc64 description of 
what that method does, it would not be applicable to most code addresses, only 
function entry, and only when we were calling into this function I think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136938

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


[Lldb-commits] [lldb] 9464bd8 - [lldb] llvm::Optional::value => operator*/operator->

2022-12-16 Thread Fangrui Song via lldb-commits

Author: Fangrui Song
Date: 2022-12-17T05:01:54Z
New Revision: 9464bd8c78d142225957abbf38ed3c2abaa9e180

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

LOG: [lldb] llvm::Optional::value => operator*/operator->

std::optional::value() has undesired exception checking semantics and is
unavailable in some older Xcode. The call sites block std::optional migration.

Added: 


Modified: 
lldb/include/lldb/Target/MemoryRegionInfo.h
lldb/source/API/SBMemoryRegionInfo.cpp
lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
lldb/source/Core/DumpDataExtractor.cpp
lldb/source/Core/ValueObjectChild.cpp
lldb/source/Host/common/File.cpp
lldb/source/Host/common/Terminal.cpp
lldb/source/Plugins/ABI/X86/ABIX86.cpp
lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Target/UnixSignals.cpp
lldb/source/Utility/SelectHelper.cpp
lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
lldb/unittests/Process/minidump/MinidumpParserTest.cpp
lldb/unittests/Target/FindFileTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/MemoryRegionInfo.h 
b/lldb/include/lldb/Target/MemoryRegionInfo.h
index cf38b6ea3345f..bfb8cc417b72b 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -132,7 +132,7 @@ class MemoryRegionInfo {
 
   void SetDirtyPageList(std::vector pagelist) {
 if (m_dirty_pages)
-  m_dirty_pages.value().clear();
+  m_dirty_pages->clear();
 m_dirty_pages = std::move(pagelist);
   }
 

diff  --git a/lldb/source/API/SBMemoryRegionInfo.cpp 
b/lldb/source/API/SBMemoryRegionInfo.cpp
index 23d22fbe86c82..d8aafdc4b47f2 100644
--- a/lldb/source/API/SBMemoryRegionInfo.cpp
+++ b/lldb/source/API/SBMemoryRegionInfo.cpp
@@ -136,7 +136,7 @@ uint32_t SBMemoryRegionInfo::GetNumDirtyPages() {
   const llvm::Optional> _page_list =
   m_opaque_up->GetDirtyPageList();
   if (dirty_page_list)
-num_dirty_pages = dirty_page_list.value().size();
+num_dirty_pages = dirty_page_list->size();
 
   return num_dirty_pages;
 }
@@ -147,8 +147,8 @@ addr_t 
SBMemoryRegionInfo::GetDirtyPageAddressAtIndex(uint32_t idx) {
   addr_t dirty_page_addr = LLDB_INVALID_ADDRESS;
   const llvm::Optional> _page_list =
   m_opaque_up->GetDirtyPageList();
-  if (dirty_page_list && idx < dirty_page_list.value().size())
-dirty_page_addr = dirty_page_list.value()[idx];
+  if (dirty_page_list && idx < dirty_page_list->size())
+dirty_page_addr = (*dirty_page_list)[idx];
 
   return dirty_page_addr;
 }

diff  --git a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp 
b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
index 3b6509bd8462f..2ee993a56bac1 100644
--- a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -241,12 +241,12 @@ void BreakpointResolverFileLine::DeduceSourceMapping(
 // Adding back any potentially reverse mapping stripped prefix.
 // for new_mapping_to.
 if (m_removed_prefix_opt.has_value())
-  llvm::sys::path::append(new_mapping_to, m_removed_prefix_opt.value());
+  llvm::sys::path::append(new_mapping_to, *m_removed_prefix_opt);
 
 llvm::Optional new_mapping_from_opt =
 check_suffix(sc_file_dir, request_file_dir, case_sensitive);
 if (new_mapping_from_opt) {
-  new_mapping_from = new_mapping_from_opt.value();
+  new_mapping_from = *new_mapping_from_opt;
   if (new_mapping_to.empty())
 new_mapping_to = ".";
 } else {
@@ -254,7 +254,7 @@ void BreakpointResolverFileLine::DeduceSourceMapping(
   check_suffix(request_file_dir, sc_file_dir, case_sensitive);
   if (new_mapping_to_opt) {
 new_mapping_from = ".";
-llvm::sys::path::append(new_mapping_to, new_mapping_to_opt.value());
+llvm::sys::path::append(new_mapping_to, *new_mapping_to_opt);
   }
 }
 

diff  --git a/lldb/source/Core/DumpDataExtractor.cpp 
b/lldb/source/Core/DumpDataExtractor.cpp
index 85b0dd2d943a6..87672bee7d30b 100644
--- a/lldb/source/Core/DumpDataExtractor.cpp
+++ b/lldb/source/Core/DumpDataExtractor.cpp
@@ -100,7 +100,7 @@ static lldb::offset_t DumpAPInt(Stream *s, const 
DataExtractor ,
 bool is_signed, unsigned radix) {
   llvm::Optional apint = GetAPInt(data, , byte_size);
   if (apint) {
-std::string apint_str = toString(apint.value(), radix, is_signed);
+std::string apint_str = toString(*apint, radix, 

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

2022-12-16 Thread Alex Langford via Phabricator via lldb-commits
bulbazord updated this revision to Diff 483692.
bulbazord added a comment.

Add a comment explaining why we do not dealloc after exec


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140249

Files:
  lldb/include/lldb/Target/Memory.h
  lldb/source/Target/Memory.cpp
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -542,7 +542,7 @@
   m_notifications.swap(empty_notifications);
   m_image_tokens.clear();
   m_memory_cache.Clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*did_exec=*/false);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
@@ -5656,7 +5656,7 @@
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_image_tokens.clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*did_exec=*/true);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
Index: lldb/source/Target/Memory.cpp
===
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -332,9 +332,11 @@
 
 AllocatedMemoryCache::~AllocatedMemoryCache() = default;
 
-void AllocatedMemoryCache::Clear() {
+void AllocatedMemoryCache::Clear(bool did_exec) {
   std::lock_guard guard(m_mutex);
-  if (m_process.IsAlive()) {
+  // After an exec, the inferior is a new process and these memory regions are
+  // no longer allocated.
+  if (m_process.IsAlive() && !did_exec) {
 PermissionsToBlockMap::iterator pos, end = m_memory_map.end();
 for (pos = m_memory_map.begin(); pos != end; ++pos)
   m_process.DoDeallocateMemory(pos->second->GetBaseAddress());
Index: lldb/include/lldb/Target/Memory.h
===
--- lldb/include/lldb/Target/Memory.h
+++ lldb/include/lldb/Target/Memory.h
@@ -116,7 +116,7 @@
 
   ~AllocatedMemoryCache();
 
-  void Clear();
+  void Clear(bool did_exec);
 
   lldb::addr_t AllocateMemory(size_t byte_size, uint32_t permissions,
   Status );


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -542,7 +542,7 @@
   m_notifications.swap(empty_notifications);
   m_image_tokens.clear();
   m_memory_cache.Clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*did_exec=*/false);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
@@ -5656,7 +5656,7 @@
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_image_tokens.clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*did_exec=*/true);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
Index: lldb/source/Target/Memory.cpp
===
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -332,9 +332,11 @@
 
 AllocatedMemoryCache::~AllocatedMemoryCache() = default;
 
-void AllocatedMemoryCache::Clear() {
+void AllocatedMemoryCache::Clear(bool did_exec) {
   std::lock_guard guard(m_mutex);
-  if (m_process.IsAlive()) {
+  // After an exec, the inferior is a new process and these memory regions are
+  // no longer allocated.
+  if (m_process.IsAlive() && !did_exec) {
 PermissionsToBlockMap::iterator pos, end = m_memory_map.end();
 for (pos = m_memory_map.begin(); pos != end; ++pos)
   m_process.DoDeallocateMemory(pos->second->GetBaseAddress());
Index: lldb/include/lldb/Target/Memory.h
===
--- lldb/include/lldb/Target/Memory.h
+++ lldb/include/lldb/Target/Memory.h
@@ -116,7 +116,7 @@
 
   ~AllocatedMemoryCache();
 
-  void Clear();
+  void Clear(bool did_exec);
 
   lldb::addr_t AllocateMemory(size_t byte_size, uint32_t permissions,
   Status );
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

2022-12-16 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/source/Target/Memory.cpp:337
   std::lock_guard guard(m_mutex);
-  if (m_process.IsAlive()) {
+  if (m_process.IsAlive() && !did_exec) {
 PermissionsToBlockMap::iterator pos, end = m_memory_map.end();

jasonmolenda wrote:
> Should we comment that if we've just exec'ed, the inferior is a new process 
> and does not have these memory regions allocated.
Yeah, I think that would be helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140249

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


[Lldb-commits] [PATCH] D140253: [debugserver] Clear memory allocations after exec

2022-12-16 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added a reviewer: jasonmolenda.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.

After an exec, the inferior is a new process and none of these memory
regions are still allocated. Clear them out.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140253

Files:
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/MacOSX/MachTask.h
  lldb/tools/debugserver/source/MacOSX/MachTask.mm


Index: lldb/tools/debugserver/source/MacOSX/MachTask.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachTask.mm
+++ lldb/tools/debugserver/source/MacOSX/MachTask.mm
@@ -999,6 +999,13 @@
   return false;
 }
 
+//--
+// MachTask::ClearAllocations
+//--
+void MachTask::ClearAllocations() {
+  m_allocations.clear();
+}
+
 void MachTask::TaskPortChanged(task_t task)
 {
   m_task = task;
Index: lldb/tools/debugserver/source/MacOSX/MachTask.h
===
--- lldb/tools/debugserver/source/MacOSX/MachTask.h
+++ lldb/tools/debugserver/source/MacOSX/MachTask.h
@@ -61,6 +61,7 @@
 
   nub_addr_t AllocateMemory(nub_size_t size, uint32_t permissions);
   nub_bool_t DeallocateMemory(nub_addr_t addr);
+  void ClearAllocations();
 
   mach_port_t ExceptionPort() const;
   bool ExceptionPortIsValid() const;
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -2285,6 +2285,7 @@
 m_thread_list.Clear();
 m_activities.Clear();
 m_breakpoints.DisableAll();
+m_task.ClearAllocations();
   }
 
   if (m_sent_interrupt_signo != 0) {


Index: lldb/tools/debugserver/source/MacOSX/MachTask.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachTask.mm
+++ lldb/tools/debugserver/source/MacOSX/MachTask.mm
@@ -999,6 +999,13 @@
   return false;
 }
 
+//--
+// MachTask::ClearAllocations
+//--
+void MachTask::ClearAllocations() {
+  m_allocations.clear();
+}
+
 void MachTask::TaskPortChanged(task_t task)
 {
   m_task = task;
Index: lldb/tools/debugserver/source/MacOSX/MachTask.h
===
--- lldb/tools/debugserver/source/MacOSX/MachTask.h
+++ lldb/tools/debugserver/source/MacOSX/MachTask.h
@@ -61,6 +61,7 @@
 
   nub_addr_t AllocateMemory(nub_size_t size, uint32_t permissions);
   nub_bool_t DeallocateMemory(nub_addr_t addr);
+  void ClearAllocations();
 
   mach_port_t ExceptionPort() const;
   bool ExceptionPortIsValid() const;
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -2285,6 +2285,7 @@
 m_thread_list.Clear();
 m_activities.Clear();
 m_breakpoints.DisableAll();
+m_task.ClearAllocations();
   }
 
   if (m_sent_interrupt_signo != 0) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

2022-12-16 Thread Alexander Yermolovich via Phabricator via lldb-commits
ayermolo added a comment.

In D138618#3999154 , @labath wrote:

> Thanks for splitting this up. We still need to figure out what to do with the 
> first patch, but these two are looking very good now.
>
> At least, in the sense that one can clearly see what is happening -- I'd 
> still like to have to discussion about the DIERef->user_id conversion. 
> Essentially, I'd like to see if we can preserve the property that the 
> conversion always produces a valid user_id. With this patch that is not true 
> anymore, because the OSO DIERefs need to be passed through the 
> SymbolFileDWARF::GetUID function, even though it is _very_ tempting to just 
> call `get_id()` on them. Previously, there was no get_id function, so one had 
> no choice but to call GetUID. If we can make sure that the OSO symbol files 
> always construct DIERefs with the oso field populated correctly, then I think 
> this approach would be fine (and we should be able to delete the GetUID 
> function). Otherwise, I'd like to explore some options of keeping the 
> DIERef->user_id conversion tightly controlled. Personally, I am still not 
> convinced that doing the conversion in the GetUID function is a bad thing. 
> IIUC, the main problem is the part where we do a bitwise or of the DIERef 
> (the die offset part) and the OSO ID (`GetID() | ref.die_offset()`), but I 
> don't see why we couldn't do something about that. Basically we could just 
> create an inverse function of `GetOSOIndexFromUserID` (which extracts the OSO 
> symbol file from the user id) -- and make sure the functions are close to 
> each other and their implementation matches.

I don't think creation of ID was that tightly controlled. 
For example
oso_symfile->SetID(((uint64_t)m_cu_idx + 1ull) << 32ull);
in SymbolFileDWARFDebugMap.cpp
But on more general note all the assumptions about bit layout scattered through 
few places:
GetOSOIndexFromUserID
GetUID
SymbolFileDWARFDwo constructor
GetDwoNum
Encode/Decode UID
and even something like 
const dw_offset_t function_die_offset = func.GetID();
return DecodedUID{

  *dwarf, {std::nullopt, DIERef::Section::DebugInfo, dw_offset_t(uid)}};

So probably moving to something uniform to construct uid for dwo and oso cases, 
and extract relevant fields is a step in a right direction.

I do see your point that it's currently not quite there.

This

  if (GetDebugMapSymfile()) {
  DIERef die_ref(GetID());
  die_ref.set_die_offset(ref.die_offset());
  return die_ref.get_id();
}

Is kind of not ideal.
In other places it does work.

  static uint32_t GetOSOIndexFromUserID(lldb::user_id_t uid) {
  llvm::Optional OsoNum = DIERef(uid).oso_num();
  lldbassert(OsoNum && "Invalid OSO Index");
  return *OsoNum;
}



  oso_symfile->SetID(DIERef(DIERef::IndexType::OSONum, m_cu_idx,
DIERef::Section(0), 0)
 .get_id());



  llvm::Optional GetDwoNum() override {
  return DIERef(GetID()).dwo_num();
}

I guess main issue with GetUID is that we rely on an internal state of 
SymbolFileDWARF to

1. figure out if we are dealing with dwo or oso with check for 
GetDebugMapSymfile
2. get extra data GetDwoNum(), and GetID()

We can either push that logic on the caller side of things (not I deal I would 
think) and remove GetUID, or extend the constructor to be a bit more explicit. 
This way all the bit settings are still consolidated, but the logic of when to 
create what is still hidden in GetDebugMapSymfile.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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


[Lldb-commits] [lldb] 20c213a - [lldb][NFC] Remove unused/unimplemented Type methods

2022-12-16 Thread Arthur Eubanks via lldb-commits

Author: Arthur Eubanks
Date: 2022-12-16T15:16:52-08:00
New Revision: 20c213a13dfa19a262b15c4d7a8977f547471b43

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

LOG: [lldb][NFC] Remove unused/unimplemented Type methods

Added: 


Modified: 
lldb/include/lldb/Symbol/Type.h

Removed: 




diff  --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index 1b0e608ca88c3..362db0ac054ad 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -168,14 +168,6 @@ class Type : public std::enable_shared_from_this, 
public UserID {
   bool WriteToMemory(ExecutionContext *exe_ctx, lldb::addr_t address,
  AddressType address_type, DataExtractor );
 
-  bool GetIsDeclaration() const;
-
-  void SetIsDeclaration(bool b);
-
-  bool GetIsExternal() const;
-
-  void SetIsExternal(bool b);
-
   lldb::Format GetFormat();
 
   lldb::Encoding GetEncoding(uint64_t );



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


[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

2022-12-16 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added inline comments.



Comment at: lldb/source/Target/Memory.cpp:337
   std::lock_guard guard(m_mutex);
-  if (m_process.IsAlive()) {
+  if (m_process.IsAlive() && !did_exec) {
 PermissionsToBlockMap::iterator pos, end = m_memory_map.end();

Should we comment that if we've just exec'ed, the inferior is a new process and 
does not have these memory regions allocated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140249

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


[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

2022-12-16 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: jasonmolenda, jingham, JDevlieghere.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.

After an exec has occured, resources used to manage the state of a
Process are cleaned up. One such resource is the AllocatedMemoryCache
which keeps track of memory allocations made in the process for things
like expression evaluation. After an exec is performed, the allocated
memory regions in the process are gone, so it does not make sense to try
to deallocate those regions.

rdar://103188106


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140249

Files:
  lldb/include/lldb/Target/Memory.h
  lldb/source/Target/Memory.cpp
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -542,7 +542,7 @@
   m_notifications.swap(empty_notifications);
   m_image_tokens.clear();
   m_memory_cache.Clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*did_exec=*/false);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
@@ -5656,7 +5656,7 @@
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_image_tokens.clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*did_exec=*/true);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
Index: lldb/source/Target/Memory.cpp
===
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -332,9 +332,9 @@
 
 AllocatedMemoryCache::~AllocatedMemoryCache() = default;
 
-void AllocatedMemoryCache::Clear() {
+void AllocatedMemoryCache::Clear(bool did_exec) {
   std::lock_guard guard(m_mutex);
-  if (m_process.IsAlive()) {
+  if (m_process.IsAlive() && !did_exec) {
 PermissionsToBlockMap::iterator pos, end = m_memory_map.end();
 for (pos = m_memory_map.begin(); pos != end; ++pos)
   m_process.DoDeallocateMemory(pos->second->GetBaseAddress());
Index: lldb/include/lldb/Target/Memory.h
===
--- lldb/include/lldb/Target/Memory.h
+++ lldb/include/lldb/Target/Memory.h
@@ -116,7 +116,7 @@
 
   ~AllocatedMemoryCache();
 
-  void Clear();
+  void Clear(bool did_exec);
 
   lldb::addr_t AllocateMemory(size_t byte_size, uint32_t permissions,
   Status );


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -542,7 +542,7 @@
   m_notifications.swap(empty_notifications);
   m_image_tokens.clear();
   m_memory_cache.Clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*did_exec=*/false);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
@@ -5656,7 +5656,7 @@
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_image_tokens.clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*did_exec=*/true);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
Index: lldb/source/Target/Memory.cpp
===
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -332,9 +332,9 @@
 
 AllocatedMemoryCache::~AllocatedMemoryCache() = default;
 
-void AllocatedMemoryCache::Clear() {
+void AllocatedMemoryCache::Clear(bool did_exec) {
   std::lock_guard guard(m_mutex);
-  if (m_process.IsAlive()) {
+  if (m_process.IsAlive() && !did_exec) {
 PermissionsToBlockMap::iterator pos, end = m_memory_map.end();
 for (pos = m_memory_map.begin(); pos != end; ++pos)
   m_process.DoDeallocateMemory(pos->second->GetBaseAddress());
Index: lldb/include/lldb/Target/Memory.h
===
--- lldb/include/lldb/Target/Memory.h
+++ lldb/include/lldb/Target/Memory.h
@@ -116,7 +116,7 @@
 
   ~AllocatedMemoryCache();
 
-  void Clear();
+  void Clear(bool did_exec);
 
   lldb::addr_t AllocateMemory(size_t byte_size, uint32_t permissions,
   Status );
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 27249c0 - Temporarily skip test under ASAN

2022-12-16 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2022-12-16T13:50:21-08:00
New Revision: 27249c06b775c73b7fa9f2d8e48cac1a85169481

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

LOG: Temporarily skip test under ASAN

Added: 


Modified: 
lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py

Removed: 




diff  --git 
a/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py 
b/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
index dd0cc559ecad6..0d579dc6b8faa 100644
--- a/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
+++ b/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
@@ -13,6 +13,7 @@ class TestEarlyProcessLaunch(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipUnlessDarwin
+@skipIfAsan # rdar://103359354
 @skipIfOutOfTreeDebugserver  # 2022-12-13 FIXME: skipping system 
debugserver 
  # until this feature is included in the system
  # debugserver.



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


[Lldb-commits] [PATCH] D140056: [lldb] Report clang module build remarks

2022-12-16 Thread Dave Lee via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c5b97570502: [lldb] Report clang module build remarks 
(authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140056

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  lldb/test/API/functionalities/progress_reporting/clang_modules/Makefile
  lldb/test/API/functionalities/progress_reporting/clang_modules/MyModule.h
  
lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
  lldb/test/API/functionalities/progress_reporting/clang_modules/main.m
  
lldb/test/API/functionalities/progress_reporting/clang_modules/module.modulemap

Index: lldb/test/API/functionalities/progress_reporting/clang_modules/module.modulemap
===
--- /dev/null
+++ lldb/test/API/functionalities/progress_reporting/clang_modules/module.modulemap
@@ -0,0 +1,4 @@
+module MyModule {
+  header "MyModule.h"
+  export *
+}
Index: lldb/test/API/functionalities/progress_reporting/clang_modules/main.m
===
--- /dev/null
+++ lldb/test/API/functionalities/progress_reporting/clang_modules/main.m
@@ -0,0 +1 @@
+int main() {}
Index: lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
===
--- /dev/null
+++ lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
@@ -0,0 +1,46 @@
+"""
+Test clang module build progress events.
+"""
+import os
+import shutil
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestCase(TestBase):
+@skipUnlessDarwin
+def test_clang_module_build_progress_report(self):
+"""Test receipt of progress events for clang module builds"""
+self.build()
+
+# Ensure an empty module cache.
+mod_cache = self.getBuildArtifact("new-modules")
+if os.path.isdir(mod_cache):
+shutil.rmtree(mod_cache)
+self.runCmd(f"settings set symbols.clang-modules-cache-path '{mod_cache}'")
+
+# TODO: The need for this seems like a bug.
+self.runCmd(
+f"settings set target.clang-module-search-paths '{self.getSourceDir()}'"
+)
+
+lldbutil.run_to_name_breakpoint(self, "main")
+
+# Just before triggering module builds, start listening for progress
+# events. Listening any earlier would result in a queue filled with
+# other unrelated progress events.
+broadcaster = self.dbg.GetBroadcaster()
+listener = lldbutil.start_listening_from(
+broadcaster, lldb.SBDebugger.eBroadcastBitProgress
+)
+
+# Trigger module builds.
+self.expect("expression @import MyModule")
+
+event = lldbutil.fetch_next_event(self, listener, broadcaster)
+payload = lldb.SBDebugger.GetProgressFromEvent(event)
+message = payload[0]
+self.assertEqual(message, "Currently building module MyModule")
Index: lldb/test/API/functionalities/progress_reporting/clang_modules/MyModule.h
===
--- /dev/null
+++ lldb/test/API/functionalities/progress_reporting/clang_modules/MyModule.h
@@ -0,0 +1 @@
+extern int doesNotActuallyExist;
Index: lldb/test/API/functionalities/progress_reporting/clang_modules/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/progress_reporting/clang_modules/Makefile
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+CFLAGS_EXTRAS = -fmodules -I$(BUILDDIR)
+
+include Makefile.rules
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -6,6 +6,9 @@
 //
 //===--===//
 
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticFrontend.h"
+#include "clang/Basic/DiagnosticSerialization.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -15,7 +18,9 @@
 #include "clang/Parse/Parser.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Serialization/ASTReader.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Threading.h"
 
@@ -25,6 +30,7 @@
 
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include 

[Lldb-commits] [lldb] 9c5b975 - [lldb] Report clang module build remarks

2022-12-16 Thread Dave Lee via lldb-commits

Author: Dave Lee
Date: 2022-12-16T13:49:17-08:00
New Revision: 9c5b97570502c5c6648730f75d097910ae2faa22

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

LOG: [lldb] Report clang module build remarks

Update the Clang diagnostic consumer (in ClangModulesDeclVendor) to report
progress on Clang module builds, as both progress events and expression logs.

Module build remarks are enabled by with clang's `-Rmodule-build` flag.

With this change, command line users of lldb will see progress events showing
which modules are being built, and - by how long they stay on screen - how much
time it takes to build them. IDEs that show progress events can show these
updates if desired.

This does not show module-import remarks, although that may be added as a
future change.

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

Added: 
lldb/test/API/functionalities/progress_reporting/clang_modules/Makefile
lldb/test/API/functionalities/progress_reporting/clang_modules/MyModule.h

lldb/test/API/functionalities/progress_reporting/clang_modules/TestClangModuleBuildProgress.py
lldb/test/API/functionalities/progress_reporting/clang_modules/main.m

lldb/test/API/functionalities/progress_reporting/clang_modules/module.modulemap

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 eea21f3f05fc9..2b98fc83097a2 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -6,6 +6,9 @@
 //
 
//===--===//
 
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticFrontend.h"
+#include "clang/Basic/DiagnosticSerialization.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -15,7 +18,9 @@
 #include "clang/Parse/Parser.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Serialization/ASTReader.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Threading.h"
 
@@ -25,6 +30,7 @@
 
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/ModuleList.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/CompileUnit.h"
@@ -61,6 +67,9 @@ class StoringDiagnosticConsumer : public 
clang::DiagnosticConsumer {
   void EndSourceFile() override;
 
 private:
+  bool HandleModuleRemark(const clang::Diagnostic );
+  void SetCurrentModuleProgress(llvm::StringRef module_name);
+
   typedef std::pair
   IDAndDiagnostic;
   std::vector m_diagnostics;
@@ -72,6 +81,9 @@ class StoringDiagnosticConsumer : public 
clang::DiagnosticConsumer {
   /// Output string filled by m_os. Will be reused for 
diff erent diagnostics.
   std::string m_output;
   Log *m_log;
+  /// A Progress with explicitly managed lifetime.
+  std::unique_ptr m_current_progress_up;
+  std::vector m_module_build_stack;
 };
 
 /// The private implementation of our ClangModulesDeclVendor.  Contains all the
@@ -140,6 +152,9 @@ StoringDiagnosticConsumer::StoringDiagnosticConsumer() {
 
 void StoringDiagnosticConsumer::HandleDiagnostic(
 clang::DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic ) {
+  if (HandleModuleRemark(info))
+return;
+
   // Print the diagnostic to m_output.
   m_output.clear();
   m_diag_printer->HandleDiagnostic(DiagLevel, info);
@@ -170,9 +185,54 @@ void StoringDiagnosticConsumer::BeginSourceFile(
 }
 
 void StoringDiagnosticConsumer::EndSourceFile() {
+  m_current_progress_up = nullptr;
   m_diag_printer->EndSourceFile();
 }
 
+bool StoringDiagnosticConsumer::HandleModuleRemark(
+const clang::Diagnostic ) {
+  Log *log = GetLog(LLDBLog::Expressions);
+  switch (info.getID()) {
+  case clang::diag::remark_module_build: {
+const auto _name = info.getArgStdStr(0);
+SetCurrentModuleProgress(module_name);
+m_module_build_stack.push_back(module_name);
+
+const auto _path = info.getArgStdStr(1);
+LLDB_LOG(log, "Building Clang module {0} as {1}", module_name, 
module_path);
+return true;
+  }
+  case clang::diag::remark_module_build_done: {
+// The current module is done.
+m_module_build_stack.pop_back();
+if (m_module_build_stack.empty()) {
+  m_current_progress_up = nullptr;
+} else {
+  // Update the progress to re-show the module that was currently being
+  // built from the time the 

[Lldb-commits] [lldb] 320b29e - Fix a syntax error

2022-12-16 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2022-12-16T13:03:45-08:00
New Revision: 320b29e7ed64c37560c263d4d4eeaa9dad87cb43

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

LOG: Fix a syntax error

Added: 


Modified: 
lldb/test/API/functionalities/signal/TestSendSignal.py

Removed: 




diff  --git a/lldb/test/API/functionalities/signal/TestSendSignal.py 
b/lldb/test/API/functionalities/signal/TestSendSignal.py
index be74f00aa10a3..0fbfdfee594e4 100644
--- a/lldb/test/API/functionalities/signal/TestSendSignal.py
+++ b/lldb/test/API/functionalities/signal/TestSendSignal.py
@@ -104,4 +104,4 @@ def match_state(self, process_listener, expected_state):
 state = lldb.SBProcess.GetStateFromEvent(event)
 self.assertEquals(state, expected_state,
 "It was the %s state." %
-lldb.SBDebugger_StateAsCString(expected_state))
+lldb.SBDebugger.StateAsCString(expected_state))



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


[Lldb-commits] [PATCH] D140240: [lldb] Prevent false positives with simple template names in SymbolFileDWARF::FindTypes

2022-12-16 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2519
+GetTypeForDIE(die)->GetBaseName().AsCString();
+if (name_bracket_index == llvm::StringRef::npos && base_name.contains('<'))
+  return true;

I'm looking into whether or not this can be sped up by checking if the type is 
templated some other way


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140240

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


[Lldb-commits] [PATCH] D140240: [lldb] Prevent false positives with simple template names in SymbolFileDWARF::FindTypes

2022-12-16 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks created this revision.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The provided test case was crashing because of confusion attempting to find 
types for `ns::Foo` under -gsimple-template-names. (This looks broken normally 
because it's attempting to find `ns::Foo` rather than `ns::Foo`)

Looking up types can't give false positives, as opposed to looking up functions 
as mentioned in https://reviews.llvm.org/D137098.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140240

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/API/lang/cpp/unique-types4/Makefile
  lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py
  lldb/test/API/lang/cpp/unique-types4/main.cpp

Index: lldb/test/API/lang/cpp/unique-types4/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types4/main.cpp
@@ -0,0 +1,14 @@
+namespace ns {
+
+template  struct Foo {
+  static T value;
+};
+
+} // namespace ns
+
+ns::Foo a;
+ns::Foo b;
+
+int main() {
+  // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py
@@ -0,0 +1,27 @@
+"""
+Test that we return only the requested template instantiation.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class UniqueTypesTestCase4(TestBase):
+def do_test(self, debug_flags):
+"""Test that we display the correct template instantiation."""
+self.build(dictionary=debug_flags)
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp"))
+# FIXME: thes should successfully print the values
+self.expect("print ns::Foo::value", "failed to parse", error=True)
+self.expect("print ns::Foo::value", "failed to parse", error=True)
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_simple_template_names(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gsimple-template-names"))
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_no_simple_template_names(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))
Index: lldb/test/API/lang/cpp/unique-types4/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types4/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2501,6 +2501,8 @@
   if (!DeclContextMatchesThisSymbolFile(parent_decl_ctx))
 return;
 
+  const llvm::StringRef name_ref = name.GetStringRef();
+  auto name_bracket_index = name_ref.find('<');
   m_index->GetTypes(name, [&](DWARFDIE die) {
 if (!DIEInDeclContext(parent_decl_ctx, die))
   return true; // The containing decl contexts don't match
@@ -2509,6 +2511,14 @@
 if (!matching_type)
   return true;
 
+// With -gsimple-template-names, a templated type's DW_AT_name will not
+// contain the template parameters. Make sure that if the original query
+// didn't contain a '<', we filter out entries with template parameters.
+const llvm::StringRef base_name =
+GetTypeForDIE(die)->GetBaseName().AsCString();
+if (name_bracket_index == llvm::StringRef::npos && base_name.contains('<'))
+  return true;
+
 // We found a type pointer, now find the shared pointer form our type
 // list
 types.InsertUnique(matching_type->shared_from_this());
@@ -2519,11 +2529,11 @@
   // contain the template parameters. Try again stripping '<' and anything
   // after, filtering out entries with template parameters that don't match.
   if (types.GetSize() < max_matches) {
-const llvm::StringRef name_ref = name.GetStringRef();
-auto it = name_ref.find('<');
-if (it != llvm::StringRef::npos) {
-  const llvm::StringRef name_no_template_params = name_ref.slice(0, it);
-  const llvm::StringRef template_params = name_ref.slice(it, name_ref.size());
+if (name_bracket_index != llvm::StringRef::npos) {
+  const llvm::StringRef name_no_template_params =
+  name_ref.slice(0, name_bracket_index);
+  const llvm::StringRef template_params =
+  name_ref.slice(name_bracket_index, name_ref.size());
   m_index->GetTypes(ConstString(name_no_template_params), [&](DWARFDIE die) {
 if (!DIEInDeclContext(parent_decl_ctx, die))
   return 

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-12-16 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a comment.

In D138176#4002154 , @JDevlieghere 
wrote:

> I'll add the assert you suggested to this patch. Do you want to commit the 
> test yourself or can I include it in my commit (I don't want to take credit 
> for your hard work, but also don't want to give you more work :-)

thank you for asking, include it to your commit please.


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

https://reviews.llvm.org/D138176

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


[Lldb-commits] [lldb] daa6305 - [trace] Migrate uses of operator<<(raw_ostream , const Optional ) to std::optional

2022-12-16 Thread Fangrui Song via lldb-commits

Author: Fangrui Song
Date: 2022-12-16T19:30:47Z
New Revision: daa6305cf7ece2a85aa37e6a880ea6460499

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

LOG: [trace] Migrate uses of operator<<(raw_ostream , const Optional ) 
to std::optional

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
lldb/source/Plugins/Process/Linux/Perf.cpp
lldb/source/Plugins/Process/Linux/Perf.h

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 82f46469df61f..fecfbf5077da8 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -627,7 +627,7 @@ class CommandInterpreter : public Broadcaster,
   /// \return \b true if the session transcript was successfully written to
   /// disk, \b false otherwise.
   bool SaveTranscript(CommandReturnObject ,
-  llvm::Optional output_file = std::nullopt);
+  std::optional output_file = std::nullopt);
 
   FileSpec GetCurrentSourceDir();
 

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 180ff9cb9ed5c..4aee2e48a13c4 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3191,7 +3191,7 @@ bool CommandInterpreter::IOHandlerInterrupt(IOHandler 
_handler) {
 }
 
 bool CommandInterpreter::SaveTranscript(
-CommandReturnObject , llvm::Optional output_file) {
+CommandReturnObject , std::optional output_file) {
   if (output_file == std::nullopt || output_file->empty()) {
 std::string now = llvm::to_string(std::chrono::system_clock::now());
 std::replace(now.begin(), now.end(), ' ', '_');

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
index 66ef90fa96a90..f0e4d0e071142 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -65,8 +65,8 @@ Error IntelPTCollector::TraceStop(const TraceStopRequest 
) {
 /// \return
 ///   some file descriptor in /sys/fs/ associated with the cgroup of the given
 ///   pid, or \a std::nullopt if the pid is not part of a cgroup.
-static Optional GetCGroupFileDescriptor(lldb::pid_t pid) {
-  static Optional fd;
+static std::optional GetCGroupFileDescriptor(lldb::pid_t pid) {
+  static std::optional fd;
   if (fd)
 return fd;
 
@@ -119,7 +119,7 @@ Error IntelPTCollector::TraceStart(const 
TraceIntelPTStartRequest ) {
   effective_request.enable_tsc = true;
 
   // We try to use cgroup filtering whenever possible
-  Optional cgroup_fd;
+  std::optional cgroup_fd;
   if (!request.disable_cgroup_filtering.value_or(false))
 cgroup_fd = GetCGroupFileDescriptor(m_process.GetID());
 

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
index 0453de25faf4a..cc7f91f3e1dd4 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
@@ -35,7 +35,7 @@ static Error IncludePerfEventParanoidMessageInError(Error 
&) {
 Expected>
 IntelPTMultiCoreTrace::StartOnAllCores(const TraceIntelPTStartRequest ,
NativeProcessProtocol ,
-   Optional cgroup_fd) {
+   std::optional cgroup_fd) {
   Expected> cpu_ids = GetAvailableLogicalCoreIDs();
   if (!cpu_ids)
 return cpu_ids.takeError();

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h 
b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
index b5b420335b438..1f042c63f134a 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
+++ b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
@@ -43,7 +43,7 @@ class IntelPTMultiCoreTrace : public IntelPTProcessTrace {
   static llvm::Expected>
   StartOnAllCores(const TraceIntelPTStartRequest ,
   NativeProcessProtocol ,
-  llvm::Optional cgroup_fd = std::nullopt);
+  std::optional cgroup_fd = std::nullopt);
 
   /// Execute the provided callback on each core that is being traced.
   ///

diff  

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-12-16 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D138176#4001739 , @avl wrote:

> Looks like I have a test case for this problem:

That's awesome, thank you for spending time on that!

I'll add the assert you suggested to this patch. Do you want to commit the test 
yourself or can I include it in my commit (I don't want to take credit for your 
hard work :-)


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

https://reviews.llvm.org/D138176

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


[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-12-16 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a comment.

Since this patch extends number of cases when uncloned dies are created, it is 
worth to add following check:

  void CompileUnit::fixupForwardReferences() {
for (const auto  : ForwardDIEReferences) {
  DIE *RefDie;
  const CompileUnit *RefUnit;
  PatchLocation Attr;
  DeclContext *Ctxt;
  std::tie(RefDie, RefUnit, Ctxt, Attr) = Ref;
  if (Ctxt && Ctxt->hasCanonicalDIE()) {
assert(Ctxt->getCanonicalDIEOffset() &&
   "Canonical die offset is not set");
Attr.set(Ctxt->getCanonicalDIEOffset());
  } else {
assert(RefDie->getOffset() && "Referenced die offset is not set");

Attr.set(RefDie->getOffset() + RefUnit->getStartOffset());
  }
}
  }


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

https://reviews.llvm.org/D138176

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


[Lldb-commits] [PATCH] D140113: [lldb] Force override when adding crashlog command

2022-12-16 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.

Do we want to do this for all the modules in `examples/python` that do 
something to this effect? e.g. `memory.py` as well.

It also might be worth changing some of those messages from errors to warnings, 
since that's what they actually are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140113

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


[Lldb-commits] [PATCH] D140051: [lldb] Add LTO dependency to lldb test suite

2022-12-16 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

> Well that was my confusion, no there isn't an option. So how does one end up 
> with a build that doesn't include it. Perhaps a standalone build of lldb, 
> built with a prebuilt llvm that didn't package libLTO?

This is just about adding a dependency between lldb-test-deps and libLTO. If 
you just run `ninja check-lldb` prior to this patch libLTO would not be built. 
Now it is. This is not about a build setting, just about a build dependency. 
Without this dependency why would we expect libLTO get built?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140051

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


[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-12-16 Thread Alexey Lapshin via Phabricator via lldb-commits
avl added a comment.

Looks like I have a test case for this problem: F25661135: 
odr-two-units-in-single-file11.test 

compile units cross reference each other in this test case:

  CU1:
0x10 type 1
ref to 0x40
  CU2:
0x40 type 1
ref to 0x10


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

https://reviews.llvm.org/D138176

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


[Lldb-commits] [PATCH] D139252: [lldb/Plugins] Introduce Scripted Platform Plugin

2022-12-16 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked 3 inline comments as done.
mib added inline comments.



Comment at: lldb/source/Plugins/Platform/scripted/ScriptedPlatform.cpp:261
+  if (!proc_info_or_error) {
+llvm::consumeError(proc_info_or_error.takeError());
+return false;

labath wrote:
> When I saw the amount of effort put into the error messages in the 
> ParseProcessFunction, I assumed you are going to give those messages to the 
> user somehow, but now I see they are just thrown away. Do you plan to change 
> that? Maybe you could at least log (LLDB_LOG_ERROR) them?
I wish there was a way to surface these error but the current implementation of 
the platform plugin doesn't allow it. I guess I'll just log them for now.


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

https://reviews.llvm.org/D139252

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


[Lldb-commits] [PATCH] D139249: [lldb] Add Debugger & ScriptedMetadata reference to Platform::CreateInstance

2022-12-16 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added inline comments.



Comment at: lldb/include/lldb/Interpreter/OptionGroupPlatform.h:73
   bool m_include_platform_option;
+  OptionGroupPythonClassWithDict m_class_options;
 };

labath wrote:
> labath wrote:
> > These nested groups are fairly unusual? Couldn't 
> > CommandObjectPlatformSelect just have two OptionGroup members?
> I see that this is referenced in 
> `OptionGroupPlatform::CreatePlatformWithOptions`. Maybe that could be fixed 
> by moving that functions somewhere else (making it a free function) and 
> having it take the two groups as arguments? Creating a platforms seems like 
> too high-level of a task for an option group anyway...
This special option group is used by many other scripted objects (Scripted 
Process, Breakpoint Callbacks, Stop Hooks, ...) since their basically an 
affordance to take a script name and a dictionary from the command line. I 
think it totally makes sense to use it here.


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

https://reviews.llvm.org/D139249

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


[Lldb-commits] [PATCH] D139248: [lldb/Interpreter] Improve ScriptedPythonInterface::GetStatusFromMethod

2022-12-16 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked an inline comment as done.
mib added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h:105
+Status error;
+Dispatch(method_name, error, args...);
+

labath wrote:
> `std::forward(args)...` maybe?
Indeed, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139248

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


[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

2022-12-16 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

ping @JDevlieghere


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

https://reviews.llvm.org/D139250

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


[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

2022-12-16 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 483509.
mib added a comment.

Update comments for `scripted_platform.list_processes`


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

https://reviews.llvm.org/D139250

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/scripted_platform.py
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -213,6 +213,12 @@
   return python::PythonObject();
 }
 
+python::PythonObject lldb_private::LLDBSwigPythonCreateScriptedPlatform(
+const char *python_class_name, const char *session_dictionary_name,
+const StructuredDataImpl _impl, std::string _string) {
+  return python::PythonObject();
+}
+
 python::PythonObject lldb_private::LLDBSWIGPython_CreateFrameRecognizer(
 const char *python_class_name, const char *session_dictionary_name) {
   return python::PythonObject();
Index: lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py
@@ -0,0 +1,38 @@
+import os
+
+import lldb
+from lldb.plugins.scripted_platform import ScriptedPlatform
+
+class MyScriptedPlatform(ScriptedPlatform):
+
+def __init__(self, args):
+self.processes = {}
+
+proc = {}
+proc['name'] = 'a.out'
+proc['arch'] = 'arm64-apple-macosx'
+proc['pid'] = 420
+proc['parent'] = 42
+proc['uid'] = 501
+proc['gid'] = 20
+self.processes[420] = proc
+
+def list_processes(self):
+return self.processes
+
+def get_process_info(self, pid):
+return self.processes[pid]
+
+def launch_process(self, launch_info):
+return lldb.SBError()
+
+def kill_process(self, pid):
+return lldb.SBError()
+
+def __lldb_init_module(debugger, dict):
+if not 'SKIP_SCRIPTED_PLATFORM_SELECT' in os.environ:
+debugger.HandleCommand(
+"platform select scripted-platform -C %s.%s" % (__name__, MyScriptedPlatform.__name__))
+else:
+print("Name of the class that will manage the scripted platform: '%s.%s'"
+% (__name__, MyScriptedPlatform.__name__))
Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
@@ -105,6 +105,10 @@
 const lldb::ProcessSP _sp, const StructuredDataImpl _impl,
 std::string _string);
 
+python::PythonObject LLDBSwigPythonCreateScriptedPlatform(
+const char *python_class_name, const char *session_dictionary_name,
+const StructuredDataImpl _impl, std::string _string);
+
 llvm::Expected LLDBSwigPythonBreakpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP _frame,
Index: lldb/examples/python/scripted_process/scripted_platform.py
===
--- /dev/null
+++ lldb/examples/python/scripted_process/scripted_platform.py
@@ -0,0 +1,83 @@
+from abc import ABCMeta, abstractmethod
+
+import lldb
+
+class ScriptedPlatform(metaclass=ABCMeta):
+
+"""
+The base class for a scripted platform.
+
+Most of the base class methods are `@abstractmethod` that need to be
+overwritten by the inheriting class.
+
+DISCLAIMER: THIS INTERFACE IS STILL UNDER DEVELOPMENT AND NOT STABLE.
+THE METHODS EXPOSED MIGHT CHANGE IN THE FUTURE.
+"""
+
+processes = None
+
+@abstractmethod
+def __init__(self, args):
+""" Construct a scripted platform.
+
+Args:
+args (lldb.SBStructuredData): A Dictionary holding arbitrary
+key/value pairs used by the scripted platform.
+"""
+processes = []
+
+@abstractmethod
+def list_processes(self):
+""" Get a list of processes that can be ran on or attached to the platform.
+
+processes = {
+420: {
+name: a.out,
+arch: aarch64,
+pid: 420,
+parent_pid: 42 (optional),
+uid: 0 (optional),
+gid: 0 (optional),
+},
+}
+
+Returns:
+Dict: The processes represented as a dictionary, with at least the
+process ID, name, 

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

2022-12-16 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added inline comments.



Comment at: lldb/bindings/python/python-wrapper.swig:317
 
+PythonObject lldb_private::LLDBSwigPythonCreateScriptedPlatform(
+const char *python_class_name, const char *session_dictionary_name,

JDevlieghere wrote:
> This looks pretty similar to `LLDBSwigPythonCreateScriptedThread` and 
> `LLDBSwigPythonCreateScriptedProcess`. Can we factor out the common parts?
True, but this need some refactor unrelated to this patch, so I'll do it in a 
follow-up


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

https://reviews.llvm.org/D139250

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


[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

2022-12-16 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked 3 inline comments as done.
mib added a subscriber: jingham.
mib added inline comments.



Comment at: lldb/examples/python/scripted_process/scripted_platform.py:31
+def list_processes(self):
+""" Get a list of processes that can be ran on the platform.
+

labath wrote:
> mib wrote:
> > mib wrote:
> > > mib wrote:
> > > > labath wrote:
> > > > > I am surprised that you want to go down the "run" path for this 
> > > > > functionality. I think most of the launch functionality does not make 
> > > > > sense for this use case (e.g., you can't provide arguments to these 
> > > > > processes, when you "run" them, can you?), and it is not consistent 
> > > > > with what the "process listing" functionality does for regular 
> > > > > platforms.
> > > > > 
> > > > > OTOH, the "attach" flow makes perfect sense here -- you take the pid 
> > > > > of an existing process, attach to it, and stop it at a random point 
> > > > > in its execution. You can't customize anything about how that process 
> > > > > is run (because it's already running) -- all you can do is choose how 
> > > > > you want to select the target process.
> > > > For now, there is no support for attaching to a scripted process, 
> > > > because we didn't have any use for it quite yet: cripted processes were 
> > > > mostly used for doing post-mortem debugging, so we "ran" them 
> > > > artificially in lldb by providing some launch options (the name of the 
> > > > class managing the process and an optional user-provided dictionary) 
> > > > through the command line or using an `SBLaunchInfo` object.
> > > > 
> > > > I guess I'll need to extend the `platform process launch/attach` 
> > > > commands and `SBAttachInfo` object to also support these options since 
> > > > they're required for the scripted process instantiation.
> > > > 
> > > > Note that we aren't really attaching to the real running process, we're 
> > > > creating a scripted process that knows how to read memory to mock the 
> > > > real process.
> > > @labath, I'll do that work on a follow-up patch
> > @labath here D139945 :) 
> Thanks. However, are you still planning to use the launch path for your 
> feature? Because if you're not, then I think this comment should say "Get a 
> list of processes that **are running**" (or that **can be attached to**).
> 
> And if you are, then I'd like to hear your thoughts on the discrepancy 
> between what "launching" means for scripted and non-scripted platforms.
> 
The way I see it is that the scripted platform will create a process with the 
right process plugin. In the case of scripted processes, the 
`ProcessLaunchInfo` argument should have the script class name set (which 
automatically sets the process plugin name to "ScriptedProcess" in the launch 
info). Once the process is instantiated (before the launch), the scripted 
platform will need to redirect to process stop events through its event 
multiplexer. So the way I see it essentially, running a regular process with 
the scripted platform should be totally transparent.

Something that is also worth discussing IMO, is the discrepancy between 
launching and attaching from the scripted platform:

One possibility could be that `platform process launch` would launch all the 
scripted processes listed by the scripted platform and set them up with the 
multiplexer, whereas `platform process attach` would just create a scripted 
process individually. I know this doesn't match the current behavior of the 
platform commands so if you guys think we should preserve the expected 
behavior, I guess.

May be @jingham has some opinion about this ?



Comment at: 
lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py:33
+def __lldb_init_module(debugger, dict):
+if not 'SKIP_SCRIPTED_PLATFORM_SELECT' in os.environ:
+debugger.HandleCommand(

labath wrote:
> ??
This is so the user can decide if the `platform select` command should be ran 
when the script is imported in lldb or not. If the user sets this env variable, 
the command will only be printed to the user, so they can copy it and run it 
whenever they want. 


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

https://reviews.llvm.org/D139250

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


[Lldb-commits] [PATCH] D137838: [Support] Move TargetParsers to new component

2022-12-16 Thread Francesco Petrogalli via Phabricator via lldb-commits
fpetrogalli accepted this revision.
fpetrogalli added a comment.

In D137838#4000959 , @lenary wrote:

> [...] I think this is ready to land on Monday?

SGTM - thank you again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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


[Lldb-commits] [PATCH] D137838: [Support] Move TargetParsers to new component

2022-12-16 Thread Sam Elliott via Phabricator via lldb-commits
lenary added a comment.

The most recent update is all the fixes needed after the builds @MaskRay asked 
me for. I think this is ready to land on Monday?

@thakis there will be GN fallout from this change. I do not intend to update GN 
in this patchset, but wanted to give you a heads-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137838

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


[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

2022-12-16 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

Completely forgot about this, ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136938

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


[Lldb-commits] [PATCH] D140051: [lldb] Add LTO dependency to lldb test suite

2022-12-16 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

Well that was my confusion, no there isn't an option. So how does one end up 
with a build that doesn't include it. Perhaps a standalone build of lldb, built 
with a prebuilt llvm that didn't package libLTO?

Which sounds perfectly legitimate and the test should be skipped. I'd just like 
to see whatever the use case is added to the commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140051

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


[Lldb-commits] [PATCH] D139833: [LLDB][LoongArch] Add branch instructions for EmulateInstructionLoongArch

2022-12-16 Thread Lu Weining via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeafe2d4cf17b: [LLDB][LoongArch] Add branch instructions for 
EmulateInstructionLoongArch (authored by lh03061238, committed by SixWeining).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139833

Files:
  lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
  lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h

Index: lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
===
--- lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
+++ lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h
@@ -38,8 +38,9 @@
   static void Terminate();
 
 public:
-  EmulateInstructionLoongArch(const ArchSpec )
-  : EmulateInstruction(arch) {}
+  EmulateInstructionLoongArch(const ArchSpec ) : EmulateInstruction(arch) {
+m_arch_subtype = arch.GetMachine();
+  }
 
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 
@@ -57,6 +58,7 @@
uint32_t reg_num) override;
   lldb::addr_t ReadPC(bool *success);
   bool WritePC(lldb::addr_t pc);
+  bool IsLoongArch64() { return m_arch_subtype == llvm::Triple::loongarch64; }
 
 private:
   struct Opcode {
@@ -66,9 +68,33 @@
 const char *name;
   };
 
+  llvm::Triple::ArchType m_arch_subtype;
   Opcode *GetOpcodeForInstruction(uint32_t inst);
 
+  bool EmulateBEQZ(uint32_t inst);
+  bool EmulateBNEZ(uint32_t inst);
+  bool EmulateJIRL(uint32_t inst);
+  bool EmulateB(uint32_t inst);
+  bool EmulateBL(uint32_t inst);
+  bool EmulateBEQ(uint32_t inst);
+  bool EmulateBNE(uint32_t inst);
+  bool EmulateBLT(uint32_t inst);
+  bool EmulateBGE(uint32_t inst);
+  bool EmulateBLTU(uint32_t inst);
+  bool EmulateBGEU(uint32_t inst);
   bool EmulateNonJMP(uint32_t inst);
+
+  bool EmulateBEQZ64(uint32_t inst);
+  bool EmulateBNEZ64(uint32_t inst);
+  bool EmulateJIRL64(uint32_t inst);
+  bool EmulateB64(uint32_t inst);
+  bool EmulateBL64(uint32_t inst);
+  bool EmulateBEQ64(uint32_t inst);
+  bool EmulateBNE64(uint32_t inst);
+  bool EmulateBLT64(uint32_t inst);
+  bool EmulateBGE64(uint32_t inst);
+  bool EmulateBLTU64(uint32_t inst);
+  bool EmulateBGEU64(uint32_t inst);
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
===
--- lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
+++ lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
@@ -33,8 +33,30 @@
 
 EmulateInstructionLoongArch::Opcode *
 EmulateInstructionLoongArch::GetOpcodeForInstruction(uint32_t inst) {
-  // TODO: Add the mask of jump instruction.
+  // TODO: Add the mask for other instruction.
   static EmulateInstructionLoongArch::Opcode g_opcodes[] = {
+  {0xfc00, 0x4000, ::EmulateBEQZ,
+   "beqz rj, offs21"},
+  {0xfc00, 0x4400, ::EmulateBNEZ,
+   "bnez rj, offs21"},
+  {0xfc00, 0x4c00, ::EmulateJIRL,
+   "jirl rd, rj, offs16"},
+  {0xfc00, 0x5000, ::EmulateB,
+   " b  offs26"},
+  {0xfc00, 0x5400, ::EmulateBL,
+   "bl  offs26"},
+  {0xfc00, 0x5800, ::EmulateBEQ,
+   "beq  rj, rd, offs16"},
+  {0xfc00, 0x5c00, ::EmulateBNE,
+   "bne  rj, rd, offs16"},
+  {0xfc00, 0x6000, ::EmulateBLT,
+   "blt  rj, rd, offs16"},
+  {0xfc00, 0x6400, ::EmulateBGE,
+   "bge  rj, rd, offs16"},
+  {0xfc00, 0x6800, ::EmulateBLTU,
+   "bltu rj, rd, offs16"},
+  {0xfc00, 0x6c00, ::EmulateBGEU,
+   "bgeu rj, rd, offs16"},
   {0x, 0x, ::EmulateNonJMP,
"NonJMP"}};
   static const size_t num_loongarch_opcodes = std::size(g_opcodes);
@@ -176,6 +198,339 @@
   return arch.GetTriple().isLoongArch();
 }
 
+bool EmulateInstructionLoongArch::EmulateBEQZ(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBEQZ64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBNEZ(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBNEZ64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateJIRL(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateJIRL64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateB(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateB64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBL(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBL64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBEQ(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBEQ64(inst);
+  else
+return 

[Lldb-commits] [lldb] eafe2d4 - [LLDB][LoongArch] Add branch instructions for EmulateInstructionLoongArch

2022-12-16 Thread Weining Lu via lldb-commits

Author: Hui Li
Date: 2022-12-16T17:48:37+08:00
New Revision: eafe2d4cf17b7448db5750aff79ee3bb0158f945

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

LOG: [LLDB][LoongArch] Add branch instructions for EmulateInstructionLoongArch

Add conditional and unconditional branch instructions for loongarch64.
Note that this does not include floating-point branch instructions, that will 
come in a later patch.

Reviewed By: SixWeining, DavidSpickett

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

Added: 


Modified: 
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp 
b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
index 14ac993d9ac0..b37bb860254e 100644
--- a/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
+++ b/lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.cpp
@@ -33,8 +33,30 @@ namespace lldb_private {
 
 EmulateInstructionLoongArch::Opcode *
 EmulateInstructionLoongArch::GetOpcodeForInstruction(uint32_t inst) {
-  // TODO: Add the mask of jump instruction.
+  // TODO: Add the mask for other instruction.
   static EmulateInstructionLoongArch::Opcode g_opcodes[] = {
+  {0xfc00, 0x4000, ::EmulateBEQZ,
+   "beqz rj, offs21"},
+  {0xfc00, 0x4400, ::EmulateBNEZ,
+   "bnez rj, offs21"},
+  {0xfc00, 0x4c00, ::EmulateJIRL,
+   "jirl rd, rj, offs16"},
+  {0xfc00, 0x5000, ::EmulateB,
+   " b  offs26"},
+  {0xfc00, 0x5400, ::EmulateBL,
+   "bl  offs26"},
+  {0xfc00, 0x5800, ::EmulateBEQ,
+   "beq  rj, rd, offs16"},
+  {0xfc00, 0x5c00, ::EmulateBNE,
+   "bne  rj, rd, offs16"},
+  {0xfc00, 0x6000, ::EmulateBLT,
+   "blt  rj, rd, offs16"},
+  {0xfc00, 0x6400, ::EmulateBGE,
+   "bge  rj, rd, offs16"},
+  {0xfc00, 0x6800, ::EmulateBLTU,
+   "bltu rj, rd, offs16"},
+  {0xfc00, 0x6c00, ::EmulateBGEU,
+   "bgeu rj, rd, offs16"},
   {0x, 0x, ::EmulateNonJMP,
"NonJMP"}};
   static const size_t num_loongarch_opcodes = std::size(g_opcodes);
@@ -176,6 +198,339 @@ bool EmulateInstructionLoongArch::SupportsThisArch(const 
ArchSpec ) {
   return arch.GetTriple().isLoongArch();
 }
 
+bool EmulateInstructionLoongArch::EmulateBEQZ(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBEQZ64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBNEZ(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBNEZ64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateJIRL(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateJIRL64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateB(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateB64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBL(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBL64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBEQ(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBEQ64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBNE(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateJIRL64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBLT(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBLT64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBGE(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBGE64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBLTU(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBLTU64(inst);
+  else
+return false;
+}
+
+bool EmulateInstructionLoongArch::EmulateBGEU(uint32_t inst) {
+  if (IsLoongArch64())
+return EmulateBGEU64(inst);
+  else
+return false;
+}
+
 bool EmulateInstructionLoongArch::EmulateNonJMP(uint32_t inst) { return false; 
}
 
+// beqz rj, offs21
+// if GR[rj] == 0:
+//   PC = PC + SignExtend({offs21, 2'b0}, GRLEN)
+bool EmulateInstructionLoongArch::EmulateBEQZ64(uint32_t inst) {
+  uint64_t next_pc, imm_sign_extend;
+  bool success = false;
+  uint32_t rj = Bits32(inst, 9, 5);
+  uint64_t rj_val;
+  uint64_t pc = ReadPC();
+  if (!success)
+return false;
+  uint32_t offs21 = Bits32(inst, 25, 10) + (Bits32(inst, 4, 0) << 16);
+  rj_val = ReadRegisterUnsigned(eRegisterKindLLDB, rj, 0, );
+  if (!success)
+return false;
+  if (rj_val == 0) {
+

[Lldb-commits] [PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

2022-12-16 Thread Kadir Cetinkaya via Phabricator via lldb-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:469
 // Also grab prefixes for each option, these are not fully exposed.
-const char *const *Prefixes[DriverID::LastOption] = {nullptr};
-#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE;
+llvm::StringLiteral Prefixes[DriverID::LastOption] = {};
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] = 
VALUE;

this is still wrong semantically (and i believe are failing tests/builds, you 
can see it in the premerge bot builds for yourself 
https://buildkite.com/llvm-project/premerge-checks/builds/126513#018510e5-592b-453e-a213-a2cffc9c9ac2).

This was an array of pointers to null-terminated string blocks. Now you're 
turning it into just an array of strings.

IIUC, your intention is to change a list of strings terminated with a nullptr 
into an arrayref. so this should itself be an `ArrayRef`s.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470
+llvm::StringLiteral Prefixes[DriverID::LastOption] = {};
+#define PREFIX(NAME, VALUE) static constexpr llvm::StringLiteral NAME[] = 
VALUE;
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\

why not directly store ArrayRef's here? instead of an empty string terminated 
array? (same for rest of the patch)



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:502
   for (unsigned A = ID; A != DriverID::OPT_INVALID; A = NextAlias[A]) {
-if (Prefixes[A] == nullptr) // option groups.
+if (Prefixes[A].empty()) // option groups.
   continue;

previously this was checking for an empty array, now it's checking for an empty 
string. so semantics are completely diffrenet.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:511
 // Iterate over each spelling of the alias, e.g. -foo vs --foo.
-for (auto *Prefix = Prefixes[A]; *Prefix != nullptr; ++Prefix) {
-  llvm::SmallString<64> Buf(*Prefix);
+for (StringRef Prefix : ArrayRef(Prefixes, 
DriverID::LastOption - 1) ) {
+  llvm::SmallString<64> Buf(Prefix);

not even sure what was your intention here, but this was previously iterating 
over all the possible alternatives of a prefix until it hit the nullptr 
terminator. now you're making it iterate over something completely different 
(which i don't know how to describe).


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

https://reviews.llvm.org/D139881

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


[Lldb-commits] [lldb] 95ec1a6 - [trace] Change /sys/bus const char * variables to const char []

2022-12-16 Thread Fangrui Song via lldb-commits

Author: Fangrui Song
Date: 2022-12-16T08:07:07Z
New Revision: 95ec1a60986c13628c6f608fc0ed1f1fafb1b033

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

LOG: [trace] Change /sys/bus const char * variables to const char []

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
index 269b7ca9dee9..36ee0260d3d2 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -22,19 +22,19 @@ using namespace lldb_private;
 using namespace process_linux;
 using namespace llvm;
 
-const char *kOSEventIntelPTTypeFile =
+const char kOSEventIntelPTTypeFile[] =
 "/sys/bus/event_source/devices/intel_pt/type";
 
-const char *kPSBPeriodCapFile =
+const char kPSBPeriodCapFile[] =
 "/sys/bus/event_source/devices/intel_pt/caps/psb_cyc";
 
-const char *kPSBPeriodValidValuesFile =
+const char kPSBPeriodValidValuesFile[] =
 "/sys/bus/event_source/devices/intel_pt/caps/psb_periods";
 
-const char *kPSBPeriodBitOffsetFile =
+const char kPSBPeriodBitOffsetFile[] =
 "/sys/bus/event_source/devices/intel_pt/format/psb_period";
 
-const char *kTSCBitOffsetFile =
+const char kTSCBitOffsetFile[] =
 "/sys/bus/event_source/devices/intel_pt/format/tsc";
 
 enum IntelPTConfigFileType {



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


[Lldb-commits] [PATCH] D140056: [lldb] Report clang module build remarks

2022-12-16 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib accepted this revision.
mib added a comment.

Yep, LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140056

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