[Lldb-commits] [PATCH] D115137: [formatters] Add a pointer and reference tests for a list and forward_list formatters for libstdcpp and libcxx

[Danil Stefaniuc via Phabricator via lldb-commits]

danilashtefan created this revision.
danilashtefan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115137

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/main.cpp
@@ -4,6 +4,12 @@
 typedef std::list int_list;
 typedef std::list string_list;
 
+
+template  void by_ref_and_ptr(T &ref, T *ptr) {
+  // Check ref and ptr
+  return;
+}
+
 int main()
 {
 int_list numbers_list;
@@ -21,13 +27,16 @@
 numbers_list.push_back(2);
 numbers_list.push_back(3);
 numbers_list.push_back(4);
+
+by_ref_and_ptr(numbers_list, &numbers_list);
 
 string_list text_list;
 text_list.push_back(std::string("goofy")); // Optional break point at this line.
 text_list.push_back(std::string("is"));
 text_list.push_back(std::string("smart"));
-
 text_list.push_back(std::string("!!!"));
+
+by_ref_and_ptr(text_list, &text_list);
 
 return 0; // Set final break point at this line.
 }
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
@@ -205,11 +205,58 @@
 self.assertTrue(
 self.frame().FindVariable("text_list").MightHaveChildren(),
 "text_list.MightHaveChildren() says False for non empty!")
+
+def do_test_ptr_and_ref(self, stdlib_type):
+"""Test that ref and ptr to std::list is displayed correctly"""
+self.build(dictionary={stdlib_type: "1"})
+
+(_, process, _, bkpt) = lldbutil.run_to_source_breakpoint(self,
+'Check ref and ptr',
+lldb.SBFileSpec("main.cpp", False))
+
+self.expect("frame variable ref",
+substrs=['size=4',
+ '[0] = ', '1',
+ '[1] = ', '2',
+ '[2] = ', '3',
+ '[3] = ', '4'])
+
+
+self.expect("frame variable *ptr",
+substrs=['size=4',
+ '[0] = ', '1',
+ '[1] = ', '2',
+ '[2] = ', '3',
+ '[3] = ', '4'])
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+self.expect("frame variable ref",
+substrs=['size=4',
+ '[0]', 'goofy',
+ '[1]', 'is',
+ '[2]', 'smart',
+ '[3]', '!!!'])
+
+self.expect("frame variable *ptr",
+substrs=['size=4',
+ '[0]', 'goofy',
+ '[1]', 'is',
+ '[2]', 'smart',
+ '[3]', '!!!'])
 
 @add_test_categories(["libstdcxx"])
 def test_with_run_command_libstdcpp(self):
 self.do_test_with_run_command(USE_LIBSTDCPP)
 
+@add_test_categories(["libstdcxx"])
+def test_ptr_and_ref_libstdcpp(self):
+self.do_test_ptr_and_ref(USE_LIBSTDCPP)
+
 @add_test_categories(["libc++"])
 def test_with_run_command_libcpp(self):
-self.do_test_with_run_command(USE_LIBCPP)
\ No newline at end of file
+self.do_test_with_run_command(USE_LIBCPP)
+
+@add_test_categories(["libc++"])
+def test_ptr_and_ref_libcpp(self):
+self.do_test_ptr_and_ref(USE_LIBCPP)
\ No newline at end of file
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
@@ -1,10 +1,21 @@
 #include 
 
+
+void by_ref_and_ptr(std::forward_list &ref, std::fo

[Lldb-commits] [PATCH] D114861: Don't consider frames with different PCs as a loop

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It would be better if Jason reviewed this, but my first thoughts after seeing 
this are:

If we're going to be using both the PC and the CFA for frame comparison, do we 
really need to also skip a frame when doing the comparing? It seems like it 
should be sufficient to compare the `(cfa, pc)` pair of /this/ frame with the 
/next/ one.

If they are the same, then we're definitely looping, as the unwind on the next 
will proceed in exactly the same way. And if they're different then we're most 
likely making some kind of progress (as in, one can still create artificial 
examples where the stack alternates between two PCs, but that is also true for 
the other implementation).

I am also a bit worried about these frames with no CFA. I fear that a lot of 
other things could break when it is absent, it is sort of expected that every 
frame has one. Upon re-reading the dwarf spec, I couldn't find a place where it 
would explicitly say it's mandatory (which isn't completely surprising as dwarf 
rarely makes anything mandatory), but I definitely got the feeling that is what 
the authors intended. I don't know if it's possible to change the compiler, but 
it doesn't seem like putting an extra `cfa = sp` line into the CIE would waste 
too much space.

It would also be nice to have a test for funky functions like this. It should 
be possible to create something similar to the existing tests in 
`test/Shell/Unwind`. The form still leaves a lot to be desired, but it is the 
best we have right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114861

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

[Lldb-commits] [PATCH] D114862: Replace StackID's operator "<" with a function returning FrameComparison

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: jingham.
labath added a comment.

I don't really know what to make of this, but adding Jim for the thread plan 
changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114862

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

[Lldb-commits] [PATCH] D114819: [lldb] Split TestCxxChar8_t

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks good. Thanks for fixing this up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114819

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

[Lldb-commits] [PATCH] D115062: [LLDB][Clang] add AccessSpecDecl for methods and fields in RecordType

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like this feature, but I gotta say that a map feels a bit wasteful as well, 
given that the last access specifier is only needed for the short duration of 
time while the class definition is being parsed.

Maybe if you remove the relevant entry when `CompleteTagDeclarationDefinition` 
gets called, then we wouldn't at least have an unboundedly growing map on our 
hands?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115062

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

[Lldb-commits] [PATCH] D114911: [lldb] Introduce a FreeBSDKernel plugin for vmcores

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 392003.
mgorny added a comment.

Force `DynamicLoaderStatic`. This fixes determining load address for symbols.


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

https://reviews.llvm.org/D114911

Files:
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSDKernel/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_arm64.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_arm64.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_i386.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_i386.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_x86_64.h
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h

Index: lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
@@ -0,0 +1,36 @@
+//===-- ThreadFreeBSDKernel.h - C++ -*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_PLUGINS_PROCESS_FREEBSDKERNEL_THREADFREEBSDKERNEL_H
+#define LLDB_SOURCE_PLUGINS_PROCESS_FREEBSDKERNEL_THREADFREEBSDKERNEL_H
+
+#include "lldb/Target/Thread.h"
+
+class ThreadFreeBSDKernel : public lldb_private::Thread {
+public:
+  ThreadFreeBSDKernel(lldb_private::Process &process, lldb::tid_t tid,
+  lldb::addr_t pcb_addr);
+
+  ~ThreadFreeBSDKernel() override;
+
+  void RefreshStateAfterStop() override;
+
+  lldb::RegisterContextSP GetRegisterContext() override;
+
+  lldb::RegisterContextSP
+  CreateRegisterContextForFrame(lldb_private::StackFrame *frame) override;
+
+protected:
+  bool CalculateStopInfo() override;
+
+private:
+  lldb::RegisterContextSP m_thread_reg_ctx_sp;
+  lldb::addr_t m_pcb_addr;
+};
+
+#endif // LLDB_SOURCE_PLUGINS_PROCESS_FREEBSDKERNEL_THREADFREEBSDKERNEL_H
Index: lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
@@ -0,0 +1,85 @@
+//===-- ThreadFreeBSDKernel.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 "ThreadFreeBSDKernel.h"
+
+#include "lldb/Target/Unwind.h"
+#include "lldb/Utility/Log.h"
+
+#include "Plugins/Process/Utility/RegisterContextFreeBSD_i386.h"
+#include "Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.h"
+#include "Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h"
+#include "ProcessFreeBSDKernel.h"
+#include "RegisterContextFreeBSDKernel_arm64.h"
+#include "RegisterContextFreeBSDKernel_i386.h"
+#include "RegisterContextFreeBSDKernel_x86_64.h"
+#include "ThreadFreeBSDKernel.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+ThreadFreeBSDKernel::ThreadFreeBSDKernel(Process &process, lldb::tid_t tid,
+ lldb::addr_t pcb_addr)
+: Thread(process, tid), m_pcb_addr(pcb_addr) {}
+
+ThreadFreeBSDKernel::~ThreadFreeBSDKernel() {}
+
+void ThreadFreeBSDKernel::RefreshStateAfterStop() {}
+
+lldb::RegisterContextSP ThreadFreeBSDKernel::GetRegisterContext() {
+  if (!m_reg_context_sp)
+m_reg_context_sp = CreateRegisterContextForFrame(nullptr);
+  return m_reg_context_sp;
+}
+
+lldb::RegisterContextSP
+ThreadFreeBSDKernel::CreateRegisterContextForFrame(StackFrame *frame) {
+  RegisterContextSP reg_ctx_sp;
+  uint32_t concrete_frame_idx = 0;
+
+  if (frame)
+concrete_frame_idx = frame->GetConcreteFrameIndex();
+
+  if (concrete_frame_idx == 0) {
+if (m_thread_reg_ctx_sp)
+  return m_thread_reg_ctx_sp;
+
+ProcessFreeBSDKernel *process =
+static_cast(GetProcess().get());
+ArchSpec arch = process->GetTarget().GetArchitecture();
+
+switch (arch.GetMachine()) {
+case llvm::Triple::aarch64:
+  m_thread_reg_ctx_sp =
+  std::make_shared(
+  *this, std::make_unique(arch, 0),
+  m_pcb_addr);
+  break;
+case llvm::Triple::x86

[Lldb-commits] [PATCH] D114675: [lldb] [Target] Support fallback to file address in ReadMemory()

[Michał Górny via Phabricator via lldb-commits]

mgorny abandoned this revision.
mgorny added a comment.

Closing in favor of D114911 .


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

https://reviews.llvm.org/D114675

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

[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It seems to me that the DataEncoder class is a lot more complicated than what 
is necessary for our existing use cases. All of them involve creating a new 
buffer (either empty, or with some pre-existing content), modifying it, and 
passing it on. A lot of the DataEncoder complexity (`IsDynamic`, 
`UpdateMemberVariables`, ...) stems from the fact that it wants to support 
writing into unowned buffers, functionality that we don't even use.

If we removed that, we could make this code a lot simpler. See inline comments 
for one way to achieve that.




Comment at: lldb/include/lldb/Utility/DataEncoder.h:79-80
+  /// A size of an address in bytes. \see PutAddress
   DataEncoder(void *data, uint32_t data_length, lldb::ByteOrder byte_order,
   uint8_t addr_size);
 

Take `const void*data` here and make a copy for internal use inside the 
constructor.



Comment at: lldb/include/lldb/Utility/DataEncoder.h:283-291
+  /// Return if this object has a dynamic buffer that can be appended to.
+  ///
+  /// The buffer is dynamic if the "m_data_sp" is valid. "m_data_sp" is only 
set
+  /// by constructors mentioned below.
+  ///
+  /// \see DataEncoder(lldb::ByteOrder byte_order, uint8_t addr_size);
+  bool IsDynamic() const {

delete



Comment at: lldb/include/lldb/Utility/DataEncoder.h:309-314
+  /// Update the m_start and m_end after appending data.
   ///
-  /// \param[in] offset
-  /// The offset into \a data_sp at which the subset starts.
-  ///
-  /// \param[in] length
-  /// The length in bytes of the subset of \a data_sp.
-  ///
-  /// \return
-  /// The number of bytes that this object now contains.
-  uint32_t SetData(const lldb::DataBufferSP &data_sp, uint32_t offset = 0,
-   uint32_t length = UINT32_MAX);
+  /// Any of the member functions that append data to the end of the owned 
+  /// data should call this function after appending data to update the 
required
+  /// member variables.
+  void UpdateMemberVariables();

delete, along with the member variables it updates.



Comment at: lldb/source/Expression/DWARFExpression.cpp:463-465
   // So first we copy the data into a heap buffer
   std::unique_ptr head_data_up(
   new DataBufferHeap(m_data.GetDataStart(), m_data.GetByteSize()));

delete, and create data encoder from `m_data.GetDataStart()` directly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

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

[Lldb-commits] [PATCH] D113930: [LLDB][NativePDB] Fix function decl creation for class methods

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I am somewhat confused by this deduplication logic. I've read 
https://reviews.llvm.org/D113930#3136404, so I understand why the duplication 
is happening, but I don't understand _should_ it be happening. IIUC, 
`SymbolFileNativePDB::ParseTypes` tries to force the completion of all types. 
If it succeeds, then there should be no need to add anything else to that type. 
In fact, adding methods to an already-complete type sounds like a pretty bad 
idea.

While I'm not very familiar with PDBs or the process of clang ast creation, it 
sounds to me like there is something wrong here, and I want to make sure we're 
not using the deduplication to cover up some other issue. Were you able to 
understand (and ideally, explain) why we have the two parsing methods and why 
they end up trying to add the same method twice?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113930

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

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

All of the changes seem fine, but it's not clear to me what is the value of the 
new test. To me, it just seems like a change-detector forcing the two versions 
of the keybindings to stay in sync. The need for the friend declarations and 
everything also enforces the feeling that this test is not checking what it 
should be. Did you actually run into some kind of a problem/bug here that you 
think this test would have prevented?

Personally, I think it would be more useful if we verified the actual behavior 
of our editline code. We have some code here, in TestMultilineNavigation.py, 
and a couple of other places, but it would definitely be useful to have more of 
those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115126

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

[Lldb-commits] [PATCH] D114911: [lldb] Introduce a FreeBSDKernel plugin for vmcores

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't have a problem with incomplete patches. I (and llvm, in general) 
actually strongly incremental development.

I'm more interested in the testing story.




Comment at: lldb/source/Plugins/Process/FreeBSDKernel/CMakeLists.txt:1
+find_library(FBSDVMCORE fbsdvmcore)
+

external dependencies are dealt with in LLDBConfig.cmake


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

https://reviews.llvm.org/D114911

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

[Lldb-commits] [lldb] fdc1638 - [lldb] [Process/elf-core] Disable for FreeBSD vmcores

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-12-06T14:40:02+01:00
New Revision: fdc1638b5cbd7f93937dce56f8ea29db52390502

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

LOG: [lldb] [Process/elf-core] Disable for FreeBSD vmcores

Recognize FreeBSD vmcores (kernel core dumps) through OS ABI = 0xFF
+ ELF version = 0, and do not process them via the elf-core plugin.
While these files use ELF as a container format, they contain raw memory
dump rather than proper VM segments and therefore are not usable
to the elf-core plugin.

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

Added: 


Modified: 
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index b852a01643753..65dbc8ea95b31 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -64,6 +64,10 @@ lldb::ProcessSP 
ProcessElfCore::CreateInstance(lldb::TargetSP target_sp,
   DataExtractor data(data_sp, lldb::eByteOrderLittle, 4);
   lldb::offset_t data_offset = 0;
   if (elf_header.Parse(data, &data_offset)) {
+// Check whether we're dealing with a raw FreeBSD "full memory dump"
+// ELF vmcore that needs to be handled via FreeBSDKernel plugin 
instead.
+if (elf_header.e_ident[7] == 0xFF && elf_header.e_version == 0)
+  return process_sp;
 if (elf_header.e_type == llvm::ELF::ET_CORE)
   process_sp = std::make_shared(target_sp, listener_sp,
 *crash_file);



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

[Lldb-commits] [PATCH] D114967: [lldb] [Process/elf-core] Disable for FreeBSD vmcores

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfdc1638b5cbd: [lldb] [Process/elf-core] Disable for FreeBSD 
vmcores (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114967

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp


Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -64,6 +64,10 @@
   DataExtractor data(data_sp, lldb::eByteOrderLittle, 4);
   lldb::offset_t data_offset = 0;
   if (elf_header.Parse(data, &data_offset)) {
+// Check whether we're dealing with a raw FreeBSD "full memory dump"
+// ELF vmcore that needs to be handled via FreeBSDKernel plugin 
instead.
+if (elf_header.e_ident[7] == 0xFF && elf_header.e_version == 0)
+  return process_sp;
 if (elf_header.e_type == llvm::ELF::ET_CORE)
   process_sp = std::make_shared(target_sp, listener_sp,
 *crash_file);


Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -64,6 +64,10 @@
   DataExtractor data(data_sp, lldb::eByteOrderLittle, 4);
   lldb::offset_t data_offset = 0;
   if (elf_header.Parse(data, &data_offset)) {
+// Check whether we're dealing with a raw FreeBSD "full memory dump"
+// ELF vmcore that needs to be handled via FreeBSDKernel plugin instead.
+if (elf_header.e_ident[7] == 0xFF && elf_header.e_version == 0)
+  return process_sp;
 if (elf_header.e_type == llvm::ELF::ET_CORE)
   process_sp = std::make_shared(target_sp, listener_sp,
 *crash_file);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] a52af6d - [lldb] Remove extern "C" from lldb-swig-lua interface

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-12-06T14:57:44+01:00
New Revision: a52af6d3714fa6ea749783400a98dcfad94814b0

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

LOG: [lldb] Remove extern "C" from lldb-swig-lua interface

This is the lua equivalent of 9a14adeae0.

Added: 


Modified: 
lldb/bindings/lua/lua-wrapper.swig
lldb/bindings/lua/lua.swig
lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Removed: 




diff  --git a/lldb/bindings/lua/lua-wrapper.swig 
b/lldb/bindings/lua/lua-wrapper.swig
index c51911bb6bf73..4ca406137d6cf 100644
--- a/lldb/bindings/lua/lua-wrapper.swig
+++ b/lldb/bindings/lua/lua-wrapper.swig
@@ -4,26 +4,9 @@ template 
 void
 PushSBClass(lua_State* L, T* obj);
 
-%}
-
-%runtime %{
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-void LLDBSwigLuaCallLuaLogOutputCallback(const char *str, void *baton);
-int LLDBSwigLuaCloseFileHandle(lua_State *L);
-
-#ifdef __cplusplus
-}
-#endif
-%}
-
-%wrapper %{
-
 // This function is called from Lua::CallBreakpointCallback
-SWIGEXPORT llvm::Expected
-LLDBSwigLuaBreakpointCallbackFunction
+llvm::Expected
+lldb_private::LLDBSwigLuaBreakpointCallbackFunction
 (
lua_State *L,
lldb::StackFrameSP stop_frame_sp,
@@ -67,8 +50,8 @@ LLDBSwigLuaBreakpointCallbackFunction
 }
 
 // This function is called from Lua::CallWatchpointCallback
-SWIGEXPORT llvm::Expected
-LLDBSwigLuaWatchpointCallbackFunction
+llvm::Expected
+lldb_private::LLDBSwigLuaWatchpointCallbackFunction
 (
lua_State *L,
lldb::StackFrameSP stop_frame_sp,
@@ -101,7 +84,7 @@ LLDBSwigLuaWatchpointCallbackFunction
return stop;
 }
 
-SWIGEXPORT void
+static void
 LLDBSwigLuaCallLuaLogOutputCallback(const char *str, void *baton) {
lua_State *L = (lua_State *)baton;
 
@@ -113,7 +96,7 @@ LLDBSwigLuaCallLuaLogOutputCallback(const char *str, void 
*baton) {
lua_pcall(L, 1, 0, 0);
 }
 
-int LLDBSwigLuaCloseFileHandle(lua_State *L) {
+static int LLDBSwigLuaCloseFileHandle(lua_State *L) {
return luaL_error(L, "You cannot close a file handle used by lldb.");
 }
 

diff  --git a/lldb/bindings/lua/lua.swig b/lldb/bindings/lua/lua.swig
index 21fa44c8b4d86..92f85fee22fc3 100644
--- a/lldb/bindings/lua/lua.swig
+++ b/lldb/bindings/lua/lua.swig
@@ -17,6 +17,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "../bindings/lua/lua-swigsafecast.swig"
+#include "../source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h"
 
 // required headers for typemaps
 #include "lldb/Host/File.h"

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
index e99b7b88379a7..ce726c8de6254 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "Lua.h"
+#include "SWIGLuaBridge.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Utility/FileSpec.h"
 #include "llvm/Support/Error.h"
@@ -15,30 +16,6 @@
 using namespace lldb_private;
 using namespace lldb;
 
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
-
-// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
-// C-linkage specified, but returns UDT 'llvm::Expected' which is
-// incompatible with C
-#if _MSC_VER
-#pragma warning (push)
-#pragma warning (disable : 4190)
-#endif
-
-extern "C" llvm::Expected LLDBSwigLuaBreakpointCallbackFunction(
-lua_State *L, lldb::StackFrameSP stop_frame_sp,
-lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl);
-
-extern "C" llvm::Expected LLDBSwigLuaWatchpointCallbackFunction(
-lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp);
-
-#if _MSC_VER
-#pragma warning (pop)
-#endif
-
-#pragma clang diagnostic pop
-
 static int lldb_print(lua_State *L) {
   int n = lua_gettop(L);
   lua_getglobal(L, "io");

diff  --git a/lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp 
b/lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
index 8d849cae4fbaa..7483fdeb0c306 100644
--- a/lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ b/lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -7,40 +7,24 @@
 
//===--===//
 
 #include "Plugins/ScriptInterpreter/Lua/Lua.h"
+#include "Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
 
 extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
-
-// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
-/

[Lldb-commits] [lldb] 85578db - [lldb/lua] Add a file that should have been a part of a52af6d3

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-12-06T14:58:39+01:00
New Revision: 85578db68aa9e3ad3f62e9aa6830d2a4c362cd1d

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

LOG: [lldb/lua] Add a file that should have been a part of a52af6d3

Added: 
lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h

Modified: 


Removed: 




diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h 
b/lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
new file mode 100644
index ..9efc20ce07fc
--- /dev/null
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
@@ -0,0 +1,27 @@
+//===-- SWIGLuaBridge.h -*- C++ 
-*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_LUA_SWIGLUABRIDGE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_LUA_SWIGLUABRIDGE_H
+
+#include "lldb/lldb-forward.h"
+#include "lua.hpp"
+#include "llvm/Support/Error.h"
+
+namespace lldb_private {
+
+llvm::Expected LLDBSwigLuaBreakpointCallbackFunction(
+lua_State *L, lldb::StackFrameSP stop_frame_sp,
+lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl);
+
+llvm::Expected LLDBSwigLuaWatchpointCallbackFunction(
+lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp);
+
+} // namespace lldb_private
+
+#endif // LLDB_PLUGINS_SCRIPTINTERPRETER_LUA_SWIGLUABRIDGE_H



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

[Lldb-commits] [PATCH] D115074: [lldb/lua] Suppress warnings about C-linkage in generated wrapper

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I just went ahead and did that in a52af6d3714 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115074

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

[Lldb-commits] [lldb] 5c4cb32 - [lldb/qemu] Add support for pty redirection

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-12-06T15:03:21+01:00
New Revision: 5c4cb323e86aaf816c0dd45191dad08e5d4691cf

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

LOG: [lldb/qemu] Add support for pty redirection

Lldb uses a pty to read/write to the standard input and output of the
debugged process. For host processes this would be automatically set up
by Target::FinalizeFileActions. The Qemu platform is in a unique
position of not really being a host platform, but not being remote
either. It reports IsHost() = false, but it is sufficiently host-like
that we can use the usual pty mechanism.

This patch adds the necessary glue code to enable pty redirection. It
includes a small refactor of Target::FinalizeFileActions and
ProcessLaunchInfo::SetUpPtyRedirection to reduce the amount of
boilerplate that would need to be copied.

I will note that qemu is not able to separate output from the emulated
program from the output of the emulator itself, so the two will arrive
intertwined. Normally this should not be a problem since qemu should not
produce any output during regular operation, but some output can slip
through in case of errors. This situation should be pretty obvious (to a
human), and it is the best we can do anyway.

For testing purposes, and inspired by lldb-server tests, I have extended
the mock emulator with the ability "program" the behavior of the
"emulated" program via command-line arguments.

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

Added: 


Modified: 
lldb/source/Host/common/ProcessLaunchInfo.cpp
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
lldb/source/Target/Target.cpp
lldb/test/API/qemu/TestQemuLaunch.py
lldb/test/API/qemu/qemu.py

Removed: 




diff  --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp 
b/lldb/source/Host/common/ProcessLaunchInfo.cpp
index 8d281b80e8f0b..07e4f15c9e9ec 100644
--- a/lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -212,6 +212,14 @@ void ProcessLaunchInfo::SetDetachOnError(bool enable) {
 
 llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() {
   Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS);
+
+  bool stdin_free = GetFileActionForFD(STDIN_FILENO) == nullptr;
+  bool stdout_free = GetFileActionForFD(STDOUT_FILENO) == nullptr;
+  bool stderr_free = GetFileActionForFD(STDERR_FILENO) == nullptr;
+  bool any_free = stdin_free || stdout_free || stderr_free;
+  if (!any_free)
+return llvm::Error::success();
+
   LLDB_LOG(log, "Generating a pty to use for stdin/out/err");
 
   int open_flags = O_RDWR | O_NOCTTY;
@@ -226,19 +234,13 @@ llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() {
 
   const FileSpec secondary_file_spec(m_pty->GetSecondaryName());
 
-  // Only use the secondary tty if we don't have anything specified for
-  // input and don't have an action for stdin
-  if (GetFileActionForFD(STDIN_FILENO) == nullptr)
+  if (stdin_free)
 AppendOpenFileAction(STDIN_FILENO, secondary_file_spec, true, false);
 
-  // Only use the secondary tty if we don't have anything specified for
-  // output and don't have an action for stdout
-  if (GetFileActionForFD(STDOUT_FILENO) == nullptr)
+  if (stdout_free)
 AppendOpenFileAction(STDOUT_FILENO, secondary_file_spec, false, true);
 
-  // Only use the secondary tty if we don't have anything specified for
-  // error and don't have an action for stderr
-  if (GetFileActionForFD(STDERR_FILENO) == nullptr)
+  if (stderr_free)
 AppendOpenFileAction(STDERR_FILENO, secondary_file_spec, false, true);
   return llvm::Error::success();
 }

diff  --git a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp 
b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
index 90c290b6fbc79..36cf0f010b4cc 100644
--- a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -126,6 +126,11 @@ lldb::ProcessSP 
PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info,
   launch_info.SetMonitorProcessCallback(ProcessLaunchInfo::NoOpMonitorCallback,
 false);
 
+  // This is automatically done for host platform in
+  // Target::FinalizeFileActions, but we're not a host platform.
+  llvm::Error Err = launch_info.SetUpPtyRedirection();
+  LLDB_LOG_ERROR(log, std::move(Err), "SetUpPtyRedirection failed: {0}");
+
   error = Host::LaunchProcess(launch_info);
   if (error.Fail())
 return nullptr;
@@ -134,6 +139,7 @@ lldb::ProcessSP 
PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info,
   launch_info.GetListener(),
   process_gdb_remote::ProcessGDBRemote::GetPluginNameStatic(), nullptr,
   true);
+
   ListenerSP listener_sp =
   Listener::MakeListener(

[Lldb-commits] [PATCH] D114796: [lldb/qemu] Add support for pty redirection

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5c4cb323e86a: [lldb/qemu] Add support for pty redirection 
(authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114796

Files:
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/qemu/TestQemuLaunch.py
  lldb/test/API/qemu/qemu.py

Index: lldb/test/API/qemu/qemu.py
===
--- lldb/test/API/qemu/qemu.py
+++ lldb/test/API/qemu/qemu.py
@@ -1,36 +1,63 @@
-from textwrap import dedent
 import argparse
 import socket
 import json
+import sys
 
 import use_lldb_suite
 from lldbsuite.test.gdbclientutils import *
 
+_description = """\
+Implements a fake qemu for testing purposes. The executable program
+is not actually run. Instead a very basic mock process is presented
+to lldb. This allows us to check the invocation parameters.
+
+The behavior of the emulated "process" is controlled via its command line
+arguments, which should take the form of key:value pairs. Currently supported
+actions are:
+- dump: Dump the state of the emulator as a json dictionary.  specifies
+  the target filename.
+- stdout: Write  to program stdout.
+- stderr: Write  to program stderr.
+- stdin: Read a line from stdin and store it in the emulator state. 
+  specifies the dictionary key.
+"""
+
 class MyResponder(MockGDBServerResponder):
+def __init__(self, state):
+super().__init__()
+self._state = state
+
 def cont(self):
+for a in self._state["args"]:
+action, data = a.split(":", 1)
+if action == "dump":
+with open(data, "w") as f:
+json.dump(self._state, f)
+elif action == "stdout":
+sys.stdout.write(data)
+elif action == "stderr":
+sys.stderr.write(data)
+elif action == "stdin":
+self._state[data] = sys.stdin.readline()
+else:
+print("Unknown action: %r\n" % a)
+return "X01"
 return "W47"
 
 class FakeEmulator(MockGDBServer):
-def __init__(self, addr):
+def __init__(self, addr, state):
 super().__init__(UnixServerSocket(addr))
-self.responder = MyResponder()
+self.responder = MyResponder(state)
 
 def main():
-parser = argparse.ArgumentParser(description=dedent("""\
-Implements a fake qemu for testing purposes. The executable program
-is not actually run. Instead a very basic mock process is presented
-to lldb. The emulated program must accept at least one argument.
-This should be a path where the emulator will dump its state. This
-allows us to check the invocation parameters.
-"""))
+parser = argparse.ArgumentParser(description=_description,
+formatter_class=argparse.RawDescriptionHelpFormatter)
 parser.add_argument('-g', metavar="unix-socket", required=True)
 parser.add_argument('program', help="The program to 'emulate'.")
-parser.add_argument('state_file', help="Where to dump the emulator state.")
-parsed, rest = parser.parse_known_args()
-with open(parsed.state_file, "w") as f:
-json.dump({"program":parsed.program, "rest":rest}, f)
+parser.add_argument("args", nargs=argparse.REMAINDER)
+args = parser.parse_args()
 
-emulator = FakeEmulator(parsed.g)
+emulator = FakeEmulator(args.g, vars(args))
 emulator.run()
 
 if __name__ == "__main__":
Index: lldb/test/API/qemu/TestQemuLaunch.py
===
--- lldb/test/API/qemu/TestQemuLaunch.py
+++ lldb/test/API/qemu/TestQemuLaunch.py
@@ -6,6 +6,7 @@
 import stat
 import sys
 from textwrap import dedent
+import lldbsuite.test.lldbutil
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.decorators import *
 from lldbsuite.test.gdbclientutils import *
@@ -46,7 +47,7 @@
 self.build()
 exe = self.getBuildArtifact()
 
-# Create a target using out platform
+# Create a target using our platform
 error = lldb.SBError()
 target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error)
 self.assertSuccess(error)
@@ -55,7 +56,7 @@
 # "Launch" the process. Our fake qemu implementation will pretend it
 # immediately exited.
 process = target.LaunchSimple(
-[self.getBuildArtifact("state.log"), "arg2", "arg3"], None, None)
+["dump:" + self.getBuildArtifact("state.log")], None, None)
 self.assertIsNotNone(process)
 self.assertEqual(process.GetState(), lldb.eStateExited)
 self.assertEqual(process.GetExitStatus(), 0x47)
@@ -64,7 +65,84 @@
 with open(sel

[Lldb-commits] [PATCH] D115151: [lldb/qemu] Add emulator-args setting

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: DavidSpickett, mgorny, JDevlieghere.
labath requested review of this revision.
Herald added a project: LLDB.

This setting allows the user to pass additional arguments to the qemu instance.
While we may want to introduce dedicated settings for the most common qemu
arguments (-cpu, for one), having this setting allows us to avoid creating a
setting for every possible argument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115151

Files:
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
  lldb/test/API/qemu/TestQemuLaunch.py
  lldb/test/API/qemu/qemu.py

Index: lldb/test/API/qemu/qemu.py
===
--- lldb/test/API/qemu/qemu.py
+++ lldb/test/API/qemu/qemu.py
@@ -53,6 +53,7 @@
 parser = argparse.ArgumentParser(description=_description,
 formatter_class=argparse.RawDescriptionHelpFormatter)
 parser.add_argument('-g', metavar="unix-socket", required=True)
+parser.add_argument('-fake-arg', dest="fake-arg")
 parser.add_argument('program', help="The program to 'emulate'.")
 parser.add_argument("args", nargs=argparse.REMAINDER)
 args = parser.parse_args()
Index: lldb/test/API/qemu/TestQemuLaunch.py
===
--- lldb/test/API/qemu/TestQemuLaunch.py
+++ lldb/test/API/qemu/TestQemuLaunch.py
@@ -20,7 +20,7 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 def set_emulator_setting(self, name, value):
-self.runCmd("settings set platform.plugin.qemu-user.%s %s" %
+self.runCmd("settings set -- platform.plugin.qemu-user.%s %s" %
 (name, value))
 
 def setUp(self):
@@ -43,7 +43,7 @@
 self.set_emulator_setting("architecture", self.getArchitecture())
 self.set_emulator_setting("emulator-path", emulator)
 
-def test_basic_launch(self):
+def _run_and_get_state(self):
 self.build()
 exe = self.getBuildArtifact()
 
@@ -63,7 +63,11 @@
 
 # Verify the qemu invocation parameters.
 with open(self.getBuildArtifact("state.log")) as s:
-state = json.load(s)
+return json.load(s)
+
+def test_basic_launch(self):
+state = self._run_and_get_state()
+
 self.assertEqual(state["program"], self.getBuildArtifact())
 self.assertEqual(state["args"],
 ["dump:" + self.getBuildArtifact("state.log")])
@@ -159,3 +163,9 @@
 target.Launch(info, error)
 self.assertTrue(error.Fail())
 self.assertIn("doesn't exist", error.GetCString())
+
+def test_extra_args(self):
+self.set_emulator_setting("emulator-args", "-fake-arg fake-value")
+state = self._run_and_get_state()
+
+self.assertEqual(state["fake-arg"], "fake-value")
Index: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
===
--- lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
+++ lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
@@ -9,4 +9,8 @@
 Global,
 DefaultStringValue<"">,
 Desc<"Path to the emulator binary.">;
+  def EmulatorArgs: Property<"emulator-args", "Args">,
+Global,
+DefaultStringValue<"">,
+Desc<"Extra arguments to pass to the emulator.">;
 }
Index: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
===
--- lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -47,6 +47,13 @@
 return m_collection_sp->GetPropertyAtIndexAsFileSpec(nullptr,
  ePropertyEmulatorPath);
   }
+
+  Args GetEmulatorArgs() {
+Args result;
+m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyEmulatorArgs,
+  result);
+return result;
+  }
 };
 
 static PluginProperties &GetGlobalProperties() {
@@ -112,8 +119,10 @@
 llvm::sys::fs::createUniquePath(socket_model, socket_path, false);
   } while (FileSystem::Instance().Exists(socket_path));
 
-  Args args(
-  {qemu, "-g", socket_path, launch_info.GetExecutableFile().GetPath()});
+  Args args({qemu, "-g", socket_path});
+  args.AppendArguments(GetGlobalProperties().GetEmulatorArgs());
+  args.AppendArgument("--");
+  args.AppendArgument(launch_info.GetExecutableFile().GetPath());
   for (size_t i = 1; i < launch_info.GetArguments().size(); ++i)
 args.AppendArgument(launch_info.GetArguments()[i].ref());
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D114722: [LLDB] Fix Python GIL-not-held issues

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: JDevlieghere.
labath added inline comments.



Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:101
+  } else {
+auto gstate = PyGILState_Ensure();
+Py_XDECREF(GetValue());

This usage of auto isn't very llvm-ish.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:277-285
+  if (_Py_IsFinalizing()) {
+// Leak m_py_obj rather than crashing the process.
+// https://docs.python.org/3/c-api/init.html#c.PyGILState_Ensure
+  } else {
+auto gstate = PyGILState_Ensure();
+Py_DECREF(m_py_obj);
+PyGILState_Release(gstate);

As I mentioned internally, I think this should be placed inside the 
ScriptInterpreterPythonImpl destructor (and elsewhere, if needed), as that's 
the level at which we do our normal locking. There it can become a regular 
`Locker` object, since that function will be called from SBDebugger::Terminate 
(or maybe even earlier, in either case it will be before any global destructors 
run).

The reason this can't be done with StructuredPythonObject, is because this one 
gets passed on to python code, which can delete it (or not) at pretty much 
arbitrary time. PythonObjects OTOH, are just C++  handles to python objects, 
and we should be able to keep track of their lifetimes reasonably well.

We can put an assert here to ensure the callers obey this contract.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1566
 
   {
 Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN,

Since the only reason this scope was introduced was to run the return statement 
in an unlocked state, I think it would be better to just remove it now. Same 
goes for other patterns like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114722

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

[Lldb-commits] [PATCH] D114746: [lldb] Generalize ParsedDWARFTypeAttributes

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 updated this revision to Diff 392069.

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

https://reviews.llvm.org/D114746

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
@@ -9,8 +9,11 @@
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFATTRIBUTE_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFATTRIBUTE_H
 
+#include "DWARFDIE.h"
 #include "DWARFDefines.h"
 #include "DWARFFormValue.h"
+#include "lldb/Core/Declaration.h"
+#include "lldb/Utility/ConstString.h"
 #include "llvm/ADT/SmallVector.h"
 #include 
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
@@ -10,6 +10,8 @@
 #include "DWARFUnit.h"
 #include "DWARFDebugInfo.h"
 
+using namespace lldb;
+
 DWARFAttributes::DWARFAttributes() : m_infos() {}
 
 DWARFAttributes::~DWARFAttributes() = default;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -252,39 +252,4 @@
   lldb_private::ClangASTImporter::LayoutInfo &layout_info);
 };
 
-/// Parsed form of all attributes that are relevant for type reconstruction.
-/// Some attributes are relevant for all kinds of types (declaration), while
-/// others are only meaningful to a specific type (is_virtual)
-struct ParsedDWARFTypeAttributes {
-  explicit ParsedDWARFTypeAttributes(const DWARFDIE &die);
-
-  lldb::AccessType accessibility = lldb::eAccessNone;
-  bool is_artificial = false;
-  bool is_complete_objc_class = false;
-  bool is_explicit = false;
-  bool is_forward_declaration = false;
-  bool is_inline = false;
-  bool is_scoped_enum = false;
-  bool is_vector = false;
-  bool is_virtual = false;
-  bool is_objc_direct_call = false;
-  bool exports_symbols = false;
-  clang::StorageClass storage = clang::SC_None;
-  const char *mangled_name = nullptr;
-  lldb_private::ConstString name;
-  lldb_private::Declaration decl;
-  DWARFDIE object_pointer;
-  DWARFFormValue abstract_origin;
-  DWARFFormValue containing_type;
-  DWARFFormValue signature;
-  DWARFFormValue specification;
-  DWARFFormValue type;
-  lldb::LanguageType class_language = lldb::eLanguageTypeUnknown;
-  llvm::Optional byte_size;
-  size_t calling_convention = llvm::dwarf::DW_CC_normal;
-  uint32_t bit_stride = 0;
-  uint32_t byte_stride = 0;
-  uint32_t encoding = 0;
-};
-
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFASTPARSERCLANG_H
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -286,135 +286,6 @@
   ForcefullyCompleteType(type);
 }
 
-ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const DWARFDIE &die) {
-  DWARFAttributes attributes;
-  size_t num_attributes = die.GetAttributes(attributes);
-  for (size_t i = 0; i < num_attributes; ++i) {
-dw_attr_t attr = attributes.AttributeAtIndex(i);
-DWARFFormValue form_value;
-if (!attributes.ExtractFormValueAtIndex(i, form_value))
-  continue;
-switch (attr) {
-case DW_AT_abstract_origin:
-  abstract_origin = form_value;
-  break;
-
-case DW_AT_accessibility:
-  accessibility = DWARFASTParser::GetAccessTypeFromDWARF(form_value.Unsigned());
-  break;
-
-case DW_AT_artificial:
-  is_artificial = form_value.Boolean();
-  break;
-
-case DW_AT_bit_stride:
-  bit_stride = form_value.Unsigned();
-  break;
-
-case DW_AT_byte_size:
-  byte_size = form_value.Unsigned();
-  break;
-
-case DW_AT_byte_stride:
-  byte_stride = form_value.Unsigned();
-  break;
-
-case DW_AT_calling_convention:
-  calling_convention = form_value.Unsigned();
-  break;
-
-case DW_AT_containing_type:
-  containing_type = form_value;
-  break;
-
-case DW_AT_decl_file:
-  // die.GetCU() can differ if DW_AT_specification uses DW_FORM_ref_addr.
-  decl.SetFile(
-  attributes.CompileUnitAtIndex(i)->GetFile(form_value.Unsigned()));
-  break;
-case DW_AT_decl_line:
-  decl.SetL

[Lldb-commits] [PATCH] D114746: [lldb] Generalize ParsedDWARFTypeAttributes

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 accepted this revision.
ljmf00 added a comment.
This revision is now accepted and ready to land.

In D114746#3160908 , @labath wrote:

> I don't think we should be putting this into the DWARFAttribute file. It is 
> substantially higher-level than the rest of the file, and the DWARFAttribute 
> file is destined to be merged with the llvm implementation at some point. Is 
> there a reason for not putting it into `DWARFASTParser` (like the other 
> patch)?

I changed to DWARFASTParser. There was no particular reason other than naming, 
but since I lack some knowledge about the overall infrastructure I was not sure.

> In D114746#3160331 , @ljmf00 wrote:
>
>> I'm not sure if this falls into NFC category since I'm changing how flags 
>> are stored.
>
> There's no formal definition of "NFC", so it does not really matter.

Oh ok. I read it on the Developers Policy glossary, so I was not sure. Thanks 
for the clarification.

>> This patch also reduces the memory footprint of the struct by using bit 
>> flags instead of individual booleans.
>
> The class was never meant to be stored anywhere else except the stack of the 
> function doing the parsing, so it's not really necessary to optimize it that 
> much. But since, you've already done it, I suppose it can stay...

Well, I didn't make any performance tests but it surely can improve memory 
usage. I guess the compiler can also be smart if the memory is only on the 
stack.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1140
   : containing_decl_ctx,
-GetOwningClangModule(die), name, clang_type, attrs.storage,
-attrs.is_inline);
+GetOwningClangModule(die), name, clang_type, attrs.is_external() ? 
clang::SC_Extern : clang::SC_None,
+attrs.is_inline());

shafik wrote:
> Is there a reason not to use an attribute `storage` like before? This feels 
> like we are leaking out of the abstraction where before we were not.
I think the abstraction of the external attribute is valid since `storage` is 
Clang specific StorageClass.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1493
 // and the byte size is zero.
-attrs.is_forward_declaration = true;
+attrs.attr_flags |= eDWARFAttributeIsForwardDecl;
   }

shafik wrote:
> if we are going to have getter abstraction why not have a 
> `setIsForwardDeclaration()` or something similar.
Yeah, I can see the improvement, I added setters too. I didn't add them since 
the set was only used as a statement rather than an expression.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h:86
+  /// Whether it is an artificially generated symbol.
+  eDWARFAttributeIsArtificial = (1u << 0),
+  eDWARFAttributeIsExplicit = (1u << 1),

ljmf00 wrote:
> I forgot to add the rest of the documentation. Leaving a note here for myself
Added documentation.


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

https://reviews.llvm.org/D114746

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

[Lldb-commits] [PATCH] D114746: [lldb] Generalize ParsedDWARFTypeAttributes

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 resigned from this revision.
ljmf00 added a comment.
This revision now requires review to proceed.

Ops, clicked on the wrong button


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

https://reviews.llvm.org/D114746

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

[Lldb-commits] [PATCH] D114746: [lldb] Generalize ParsedDWARFTypeAttributes

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 updated this revision to Diff 392087.

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

https://reviews.llvm.org/D114746

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
@@ -9,8 +9,11 @@
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFATTRIBUTE_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFATTRIBUTE_H
 
+#include "DWARFDIE.h"
 #include "DWARFDefines.h"
 #include "DWARFFormValue.h"
+#include "lldb/Core/Declaration.h"
+#include "lldb/Utility/ConstString.h"
 #include "llvm/ADT/SmallVector.h"
 #include 
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -252,39 +252,4 @@
   lldb_private::ClangASTImporter::LayoutInfo &layout_info);
 };
 
-/// Parsed form of all attributes that are relevant for type reconstruction.
-/// Some attributes are relevant for all kinds of types (declaration), while
-/// others are only meaningful to a specific type (is_virtual)
-struct ParsedDWARFTypeAttributes {
-  explicit ParsedDWARFTypeAttributes(const DWARFDIE &die);
-
-  lldb::AccessType accessibility = lldb::eAccessNone;
-  bool is_artificial = false;
-  bool is_complete_objc_class = false;
-  bool is_explicit = false;
-  bool is_forward_declaration = false;
-  bool is_inline = false;
-  bool is_scoped_enum = false;
-  bool is_vector = false;
-  bool is_virtual = false;
-  bool is_objc_direct_call = false;
-  bool exports_symbols = false;
-  clang::StorageClass storage = clang::SC_None;
-  const char *mangled_name = nullptr;
-  lldb_private::ConstString name;
-  lldb_private::Declaration decl;
-  DWARFDIE object_pointer;
-  DWARFFormValue abstract_origin;
-  DWARFFormValue containing_type;
-  DWARFFormValue signature;
-  DWARFFormValue specification;
-  DWARFFormValue type;
-  lldb::LanguageType class_language = lldb::eLanguageTypeUnknown;
-  llvm::Optional byte_size;
-  size_t calling_convention = llvm::dwarf::DW_CC_normal;
-  uint32_t bit_stride = 0;
-  uint32_t byte_stride = 0;
-  uint32_t encoding = 0;
-};
-
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFASTPARSERCLANG_H
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -286,135 +286,6 @@
   ForcefullyCompleteType(type);
 }
 
-ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const DWARFDIE &die) {
-  DWARFAttributes attributes;
-  size_t num_attributes = die.GetAttributes(attributes);
-  for (size_t i = 0; i < num_attributes; ++i) {
-dw_attr_t attr = attributes.AttributeAtIndex(i);
-DWARFFormValue form_value;
-if (!attributes.ExtractFormValueAtIndex(i, form_value))
-  continue;
-switch (attr) {
-case DW_AT_abstract_origin:
-  abstract_origin = form_value;
-  break;
-
-case DW_AT_accessibility:
-  accessibility = DWARFASTParser::GetAccessTypeFromDWARF(form_value.Unsigned());
-  break;
-
-case DW_AT_artificial:
-  is_artificial = form_value.Boolean();
-  break;
-
-case DW_AT_bit_stride:
-  bit_stride = form_value.Unsigned();
-  break;
-
-case DW_AT_byte_size:
-  byte_size = form_value.Unsigned();
-  break;
-
-case DW_AT_byte_stride:
-  byte_stride = form_value.Unsigned();
-  break;
-
-case DW_AT_calling_convention:
-  calling_convention = form_value.Unsigned();
-  break;
-
-case DW_AT_containing_type:
-  containing_type = form_value;
-  break;
-
-case DW_AT_decl_file:
-  // die.GetCU() can differ if DW_AT_specification uses DW_FORM_ref_addr.
-  decl.SetFile(
-  attributes.CompileUnitAtIndex(i)->GetFile(form_value.Unsigned()));
-  break;
-case DW_AT_decl_line:
-  decl.SetLine(form_value.Unsigned());
-  break;
-case DW_AT_decl_column:
-  decl.SetColumn(form_value.Unsigned());
-  break;
-
-case DW_AT_declaration:
-  is_forward_declaration = form_value.Boolean();
-  break;
-
-case DW_AT_encoding:
-  encoding = form_value.Unsigned();
-  break;
-
-case DW_AT_enum_class:
-  is_scoped_enum = form_value.Boolean();
-  break;
-
-case DW_AT_explicit:
-  is_explicit = form_value.Boolean();
-  break;
-
-case DW_AT_ex

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

[Neal via Phabricator via lldb-commits]

nealsid added a comment.

In D115126#3173236 , @labath wrote:

> All of the changes seem fine, but it's not clear to me what is the value of 
> the new test. To me, it just seems like a change-detector forcing the two 
> versions of the keybindings to stay in sync. The need for the friend 
> declarations and everything also enforces the feeling that this test is not 
> checking what it should be. Did you actually run into some kind of a 
> problem/bug here that you think this test would have prevented?
>
> Personally, I think it would be more useful if we verified the actual 
> behavior of our editline code. We have some code here, in 
> TestMultilineNavigation.py, and a couple of other places, but it would 
> definitely be useful to have more of those.

Thank you! My thought process on the tests was that the exact binding is user 
facing, and a stray extra character or accidental deletion in the strings that 
represent the key sequences should be tested against, as well as any change 
that caused the shortcut to be registered incorrectly.  If I used the same 
table in the dev & test code, it would not catch the first case, and a lost 
keyboard shortcut can be a visible bug, although, in this case, is easily 
fixable through the .editrc file.

The motivation is another change I have in progress that turns the Editline 
code into a table that stores the command, callback, key sequence, and help 
text in a table and loops over it to add them (which sounds suspiciously like 
this patch, I admit ;-)), and I wanted to make sure I didn't break any existing 
shortcuts when making that change, and some other changes I have planned, such 
as removing the conditional WCHAR support.

I also was on the fence about adding the test classes as friends.  I could have 
exposed the functionality to retrieve a shortcut through our Editline class, 
but the test would be the only client today.

Definitely +1 to having keyboard shortcut tests as part of the Python tests.  
Mind if I take that as a TODO?  I see how the coverage of both kinds of tests 
overlaps, but having the gtest tests can help find bugs earlier.  Thanks again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115126

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

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

[Neal via Phabricator via lldb-commits]

nealsid added a comment.

In D113650#3168622 , @clayborg wrote:

> I can't build on macOS now. I checked out the source code and tried to do:
>
>   cmake -G Ninja -DCMAKE_BUILD_TYPE:STRING=Debug 
> -DLLVM_ENABLE_ASSERTIONS:BOOL=TRUE 
> -DLLVM_ENABLE_PROJECTS='clang;libcxx;libcxxabi;lldb' 
> -DLLDB_BUILD_FRAMEWORK:BOOL=TRUE -DLLDB_USE_SYSTEM_DEBUGSERVER=ON 
> -DLLDB_EDITLINE_USE_WCHAR=0 -DLLDB_ENABLE_LIBEDIT:BOOL=TRUE 
> -DLLDB_ENABLE_CURSES:BOOL=TRUE -DLLDB_ENABLE_PYTHON:BOOL=TRUE 
> -DLLDB_ENABLE_LIBXML2:BOOL=TRUE -DLLDB_ENABLE_LUA:BOOL=FALSE 
> ../llvm-project/llvm

Random drive-by comment - any particular reason you compile without WCHAR 
support in libedit? I have a patch that removes the conditional support from 
our Editline class (actually it was committed and had to be reverted :P). But I 
was under the impression that nobody was using the non-WCHAR support anymore.  
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113650

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

[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

[Neal via Phabricator via lldb-commits]

nealsid added a comment.

Random unrelated question, and if I missed docs somewhere, I apologize.  Did I 
forget something in creating the revision that caused Harbormaster to not do 
arc lint or arc test?  https://reviews.llvm.org/B137600. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115126

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

[Lldb-commits] [lldb] 1f257ac - Speculatively fix the LLDB build bots from 6c75ab5f66b403f7ca67e86aeed3a58abe10570b

[Aaron Ballman via lldb-commits]

Author: Aaron Ballman
Date: 2021-12-06T13:30:15-05:00
New Revision: 1f257accd713a8e302d3fdd2cac615a303295c42

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

LOG: Speculatively fix the LLDB build bots from 
6c75ab5f66b403f7ca67e86aeed3a58abe10570b

It looks like some renames got missed.

Added: 


Modified: 
clang/lib/Sema/SemaTemplateDeduction.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index d527a379c9836..e9636d2b942e8 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -2144,7 +2144,7 @@ static Sema::TemplateDeductionResult 
DeduceTemplateArgumentsByTypeMatch(
 
   return Sema::TDK_NonDeducedMismatch;
 }
-case Type::DependentExtInt: {
+case Type::DependentBitInt: {
   const auto *IP = P->castAs();
 
   if (const auto *IA = A->getAs()) {

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index b1dbc382ff041..9b6327754f6c5 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4088,8 +4088,8 @@ 
TypeSystemClang::GetTypeClass(lldb::opaque_compiler_type_t type) {
 return lldb::eTypeClassVector;
   case clang::Type::Builtin:
   // Ext-Int is just an integer type.
-  case clang::Type::ExtInt:
-  case clang::Type::DependentExtInt:
+  case clang::Type::BitInt:
+  case clang::Type::DependentBitInt:
 return lldb::eTypeClassBuiltin;
   case clang::Type::ObjCObjectPointer:
 return lldb::eTypeClassObjCObjectPointer;
@@ -4744,8 +4744,8 @@ lldb::Encoding 
TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
 // TODO: Set this to more than one???
 break;
 
-  case clang::Type::ExtInt:
-  case clang::Type::DependentExtInt:
+  case clang::Type::BitInt:
+  case clang::Type::DependentBitInt:
 return qual_type->isUnsignedIntegerType() ? lldb::eEncodingUint
   : lldb::eEncodingSint;
 
@@ -5124,8 +5124,8 @@ lldb::Format 
TypeSystemClang::GetFormat(lldb::opaque_compiler_type_t type) {
   case clang::Type::Vector:
 break;
 
-  case clang::Type::ExtInt:
-  case clang::Type::DependentExtInt:
+  case clang::Type::BitInt:
+  case clang::Type::DependentBitInt:
 return qual_type->isUnsignedIntegerType() ? lldb::eFormatUnsigned
   : lldb::eFormatDecimal;
 



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

[Lldb-commits] [PATCH] D115062: [LLDB][Clang] add AccessSpecDecl for methods and fields in RecordType

[Zequan Wu via Phabricator via lldb-commits]

zequanwu updated this revision to Diff 392126.
zequanwu marked an inline comment as done.
zequanwu added a comment.

Erase the cxx_record_decl entry from map after completing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115062

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp
@@ -11,16 +11,23 @@
 struct Struct {
   // Test builtin types, which are represented by special CodeView type indices.
   boolB;
+private:
   charC;
+public:
   signed char SC;
+protected:
   unsigned char   UC;
   char16_tC16;
   char32_tC32;
+protected:
   wchar_t WC;
   short   S;
   unsigned short  US;
+public:
   int I;
+private:
   unsigned intUI;
+public:
   longL;
   unsigned long   UL;
   long long   LL;
@@ -32,15 +39,20 @@
 
 // Test class
 class Class {
-public:
   // Test pointers to builtin types, which are represented by different special
   // CodeView type indices.
   bool*PB;
+public:
   char*PC;
+private:
   signed char *PSC;
+protected:
   unsigned char   *PUC;
+private:
   char16_t*PC16;
+public:
   char32_t*PC32;
+private:
   wchar_t *PWC;
   short   *PS;
   unsigned short  *PUS;
@@ -155,16 +167,22 @@
 // CHECK-NEXT: (lldb) type lookup -- Struct
 // CHECK-NEXT: struct Struct {
 // CHECK-NEXT: bool B;
+// CHECK-NEXT: private:
 // CHECK-NEXT: char C;
+// CHECK-NEXT: public:
 // CHECK-NEXT: signed char SC;
+// CHECK-NEXT: protected:
 // CHECK-NEXT: unsigned char UC;
 // CHECK-NEXT: char16_t C16;
 // CHECK-NEXT: char32_t C32;
 // CHECK-NEXT: wchar_t WC;
 // CHECK-NEXT: short S;
 // CHECK-NEXT: unsigned short US;
+// CHECK-NEXT: public:
 // CHECK-NEXT: int I;
+// CHECK-NEXT: private:
 // CHECK-NEXT: unsigned int UI;
+// CHECK-NEXT: public:
 // CHECK-NEXT: long L;
 // CHECK-NEXT: unsigned long UL;
 // CHECK-NEXT: long long LL;
@@ -176,11 +194,17 @@
 // CHECK-NEXT: (lldb) type lookup -- Class
 // CHECK-NEXT: class Class {
 // CHECK-NEXT: bool *PB;
+// CHECK-NEXT: public:
 // CHECK-NEXT: char *PC;
+// CHECK-NEXT: private:
 // CHECK-NEXT: signed char *PSC;
+// CHECK-NEXT: protected:
 // CHECK-NEXT: unsigned char *PUC;
+// CHECK-NEXT: private:
 // CHECK-NEXT: char16_t *PC16;
+// CHECK-NEXT: public:
 // CHECK-NEXT: char32_t *PC32;
+// CHECK-NEXT: private:
 // CHECK-NEXT: wchar_t *PWC;
 // CHECK-NEXT: short *PS;
 // CHECK-NEXT: unsigned short *PUS;
@@ -217,7 +241,8 @@
 // CHECK-NEXT: }
 // CHECK-NEXT: (lldb) type lookup -- Derived
 // CHECK-NEXT: class Derived : public Class {
-// CHECK:  Derived &Reference;
+// CHECK-NEXT: public:
+// CHECK-NEXT: Derived &Reference;
 // CHECK-NEXT: OneMember Member;
 // CHECK-NEXT: const OneMember ConstMember;
 // CHECK-NEXT: volatile OneMember VolatileMember;
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -196,6 +196,11 @@
   ClangASTMetadata *GetMetadata(const clang::Decl *object);
   ClangASTMetadata *GetMetadata(const clang::Type *object);
 
+  void SetCXXRecordDeclAccess(const clang::CXXRecordDecl *object,
+  clang::AccessSpecifier access);
+  clang::AccessSpecifier
+  GetCXXRecordDeclAccess(const clang::CXXRecordDecl *object);
+
   // Basic Types
   CompilerType GetBuiltinTypeForEncodingAndBitSize(lldb::Encoding encoding,
size_t bit_size) override;
@@ -1080,6 +1085,12 @@
   /// Maps Types to their associated ClangASTMetadata.
   TypeMetadataMap m_type_metadata;
 
+  typedef llvm::DenseMap
+  CXXRecordDeclAccessMap;
+  /// Maps CXXRecordDecl to their most recent added method/field's
+  /// AccessSpecifier.
+  CXXRecordDeclAccessMap m_cxx_record_decl_access;
+
   /// The sema associated that is currently used to build this ASTContext.
   /// May be null if we are already done parsing this ASTContext or the
   /// ASTContext wasn't created by parsing source code.
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemCl

[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D115073#3173041 , @labath wrote:

> It seems to me that the DataEncoder class is a lot more complicated than what 
> is necessary for our existing use cases. All of them involve creating a new 
> buffer (either empty, or with some pre-existing content), modifying it, and 
> passing it on. A lot of the DataEncoder complexity (`IsDynamic`, 
> `UpdateMemberVariables`, ...) stems from the fact that it wants to support 
> writing into unowned buffers, functionality that we don't even use.

I disagree here. We //weren't// using,  but now we are using this 
functionality. If you look at the ELF test that I modified, not that it is 
using the new heap based buffer. Even though it is very easy to calculate a 
buffer size in advance for this, we shouldn't have to.

Another reason I believe we need to dynamically grow the buffer is: if I am 
about the encode an entire symbol table into a memory buffer, how would I 
determine the size of the buffer I need in advance? I shouldn't have to worry 
about this and it would make the DataEncoder class hard to use because of this. 
We are going to want to encode many more things for caching using DataEncoder 
in the future and we shouldn't have to pre-determine how much memory is needed 
in advance, or allocate way too much just in case, just to be able to encode 
the data. How large will our symbol table name index tables be? That would be 
hard to determine in advance. Same with indexing of debug info, which is 
something that I want to add soon after.

We currently need DataEncoder for two cases:

- fixing up data that isn't relocated in pre-determined buffers for variable 
addresses (DW_OP_addr from .o files in DWARF without dSYM cases for Apple)
- creating a new stream of data where we don't know the size in advance

In the latter case, we might also want to  first append a bunch of data, and 
then use the Put methods to do some fixups on data we dynamically appended to 
fixup offsets to data that was created later in the buffer.

So I strongly believe that we do need the functionality that I added to 
DataEncoder.

> If we removed that, we could make this code a lot simpler. See inline 
> comments for one way to achieve that.

Let me know if my comments above helped explain the APIs I added. I would like 
to avoid having to make two calls to a encode function to pre-determine the 
size of the buffers that are needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

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

[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:465
   std::unique_ptr head_data_up(
   new DataBufferHeap(m_data.GetDataStart(), m_data.GetByteSize()));
 

That would crash the program. The data here is coming from the DWARF from a 
mmap'ed read only file. We can't modify the data, and thus this is why we make 
a copy in the first place: so we can modify the DWARF data and fixup the 
address info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

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

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D113650#3173914 , @nealsid wrote:

> In D113650#3168622 , @clayborg 
> wrote:
>
>> I can't build on macOS now. I checked out the source code and tried to do:
>>
>>   cmake -G Ninja -DCMAKE_BUILD_TYPE:STRING=Debug 
>> -DLLVM_ENABLE_ASSERTIONS:BOOL=TRUE 
>> -DLLVM_ENABLE_PROJECTS='clang;libcxx;libcxxabi;lldb' 
>> -DLLDB_BUILD_FRAMEWORK:BOOL=TRUE -DLLDB_USE_SYSTEM_DEBUGSERVER=ON 
>> -DLLDB_EDITLINE_USE_WCHAR=0 -DLLDB_ENABLE_LIBEDIT:BOOL=TRUE 
>> -DLLDB_ENABLE_CURSES:BOOL=TRUE -DLLDB_ENABLE_PYTHON:BOOL=TRUE 
>> -DLLDB_ENABLE_LIBXML2:BOOL=TRUE -DLLDB_ENABLE_LUA:BOOL=FALSE 
>> ../llvm-project/llvm
>
> Random drive-by comment - any particular reason you compile without WCHAR 
> support in libedit? I have a patch that removes the conditional support from 
> our Editline class (actually it was committed and had to be reverted :P). But 
> I was under the impression that nobody was using the non-WCHAR support 
> anymore.  Thanks.

No real reason. I would be fine if this went away. Not sure if there are any 
platforms that have an older libedit shared library that need to avoid bugs in 
the wide character implementation, but I don't work on any. So feel free to 
remove this and see how things go!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113650

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

[Lldb-commits] [PATCH] D113930: [LLDB][NativePDB] Fix function decl creation for class methods

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added a comment.

In D113930#3173181 , @labath wrote:

> While I'm not very familiar with PDBs or the process of clang ast creation, 
> it sounds to me like there is something wrong here, and I want to make sure 
> we're not using the deduplication to cover up some other issue. Were you able 
> to understand (and ideally, explain) why we have the two parsing methods and 
> why they end up trying to add the same method twice?

In `ModuleParseAllDebugSymbols`, these two functions 
(`NativePDB::ParseFunctions` and `NativePDB::ParseTypes`) are called with 
NativePDB plugin. `NativePDB::ParseFunctions` parses pdb debug stream 
, which contains information for 
those methods existing in the binary. Functions defined in the source files but 
removed by optimizations will not show up in it. This is where those methods 
get created at first time. `NativePDB::ParseTypes` parses pdb type stream 
, which contains type information for 
methods even if they are removed by optimizations. All class members are in the 
class's FieldList type record. `ParseTypes` visits all classes' FieldList to 
complete their TagRecord. This is where those methods get created the second 
time. We can refer to a function's type record (from type stream) using its 
symbol record (from debug stream) but not the opposite, since it's common 
multiple functions have the same signature, which will be just one type record 
for them. If we can change the order of function calls so that `ParseFunctions` 
is after `ParseTypes`, we check if a method is already created or not, not sure 
if it's okay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113930

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

[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't think we actually disagree here. I'm aware of your use case (let's call 
it "dynamic mode") for appending to a buffer. I agree it's useful and I don't 
want to change that. What I want to remove is the other mode (non-dynamic). 
Presently, this is the only mode, but we're not actually making a good use of 
it. All of the existing use cases can be implemented with a "dynamic" buffer. 
And the code would be much simpler since there is only one mode to support -- 
one in which the data encoder owns the buffer it is writing to and can resize 
it at will.




Comment at: lldb/source/Expression/DWARFExpression.cpp:465
   std::unique_ptr head_data_up(
   new DataBufferHeap(m_data.GetDataStart(), m_data.GetByteSize()));
 

clayborg wrote:
> That would crash the program. The data here is coming from the DWARF from a 
> mmap'ed read only file. We can't modify the data, and thus this is why we 
> make a copy in the first place: so we can modify the DWARF data and fixup the 
> address info.
With the current implementation yes. But this was meant to be done in 
conjuction with the changes to the DataEncoder constructor. The idea is that 
the constructor itself would copy the data into the owned buffer such that it 
can be freely modified or appended to. (In here we wouldn't make use of the 
append functionality, but that doesn't matter.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

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

[Lldb-commits] [PATCH] D113930: [LLDB][NativePDB] Fix function decl creation for class methods

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for the explanation. This makes things a lot clearer (though it doesn't 
convince me that this is the right approach).

I still think that the piecemeal addition of member functions is a problem. 
While I don't think that changing the parsing order in 
`Module::ParseAllDebugSymbols` would be a problem (that function is only called 
from lldb-test), I think that would only hide the problem. Looking at what 
happens in the DWARF case, I /think/ (the code is fairly messy, so I could be 
wrong) that we don't create a clang ast representation for the function in 
`ParseFunctions`. We only create an lldb_private::Function (in 
`DWARFASTParserClang::ParseFunctionFromDWARF`). The ast entity only gets 
created in `SymbolFileDWARF::ParseTypes` 
(->ParseType->DWARFASTParserClang->ParseTypeFromDWARF). This kinda makes sense 
since we only need the ast representation of the method when we start dealing 
its class.

So I have a feeling that the real solution here is to avoid creating an ast 
entry for the function in the first phase. Then you should be able to create 
the full class declaration in the second phase, as you will have complete 
information there.

Does that make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113930

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

[Lldb-commits] [PATCH] D115137: [formatters] Add a pointer and reference tests for a list and forward_list formatters for libstdcpp and libcxx

[walter erquinigo via Phabricator via lldb-commits]

wallace accepted this revision.
wallace added a comment.
This revision is now accepted and ready to land.

thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115137

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

[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D115073#3174376 , @labath wrote:

> I don't think we actually disagree here. I'm aware of your use case (let's 
> call it "dynamic mode") for appending to a buffer. I agree it's useful and I 
> don't want to change that. What I want to remove is the other mode 
> (non-dynamic). Presently, this is the only mode, but we're not actually 
> making a good use of it. All of the existing use cases can be implemented 
> with a "dynamic" buffer. And the code would be much simpler since there is 
> only one mode to support -- one in which the data encoder owns the buffer it 
> is writing to and can resize it at will.

Ahh! Gotcha. That I can do. Thanks for the feedback, I will post an update 
shortly!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

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

[Lldb-commits] [PATCH] D115178: Unify libstdcpp and libcxx formatters for `std::optional`

[Alisamar Husain via Phabricator via lldb-commits]

zrthxn created this revision.
zrthxn added a reviewer: wallace.
zrthxn requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115178

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -913,11 +913,11 @@
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
   stl_deref_flags,
   "lldb.formatters.cpp.gnu_libstdcpp.StdMapLikeSynthProvider")));
-  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
-  RegularExpression("^std::optional<.+>(( )?&)?$"),
-  SyntheticChildrenSP(new ScriptedSyntheticChildren(
-  stl_synth_flags,
-  "lldb.formatters.cpp.gnu_libstdcpp.StdOptionalSynthProvider")));
+  // cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
+  // RegularExpression("^std::optional<.+>(( )?&)?$"),
+  // SyntheticChildrenSP(new ScriptedSyntheticChildren(
+  // stl_synth_flags,
+  // "lldb.formatters.cpp.gnu_libstdcpp.StdOptionalSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
   RegularExpression("^std::multiset<.+> >(( )?&)?$"),
   SyntheticChildrenSP(new ScriptedSyntheticChildren(


Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -913,11 +913,11 @@
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
   stl_deref_flags,
   "lldb.formatters.cpp.gnu_libstdcpp.StdMapLikeSynthProvider")));
-  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
-  RegularExpression("^std::optional<.+>(( )?&)?$"),
-  SyntheticChildrenSP(new ScriptedSyntheticChildren(
-  stl_synth_flags,
-  "lldb.formatters.cpp.gnu_libstdcpp.StdOptionalSynthProvider")));
+  // cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
+  // RegularExpression("^std::optional<.+>(( )?&)?$"),
+  // SyntheticChildrenSP(new ScriptedSyntheticChildren(
+  // stl_synth_flags,
+  // "lldb.formatters.cpp.gnu_libstdcpp.StdOptionalSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
   RegularExpression("^std::multiset<.+> >(( )?&)?$"),
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D115178: Unify libstdcpp and libcxx formatters for `std::optional`

[Alisamar Husain via Phabricator via lldb-commits]

zrthxn updated this revision to Diff 392154.
zrthxn added a comment.
Herald added a subscriber: mgorny.

Squash many commits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115178

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h

Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
@@ -45,6 +45,10 @@
 LibStdcppBitsetSyntheticFrontEndCreator(CXXSyntheticChildren *,
 lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibStdcppOptionalSyntheticFrontEndCreator(CXXSyntheticChildren *,
+lldb::ValueObjectSP);
+
 SyntheticChildrenFrontEnd *
 LibStdcppVectorIteratorSyntheticFrontEndCreator(CXXSyntheticChildren *,
 lldb::ValueObjectSP);
Index: lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
@@ -0,0 +1,106 @@
+//===-- GenericOptional.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 "LibCxx.h"
+#include "LibStdcpp.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+#include "lldb/Target/Target.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class GenericOptionalFrontend : public SyntheticChildrenFrontEnd {
+public:
+  enum class StdLib {
+LibCxx,
+LibStdcpp,
+  };
+
+  GenericOptionalFrontend(ValueObject &valobj, StdLib stdlib);
+
+  size_t GetIndexOfChildWithName(ConstString name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  size_t CalculateNumChildren() override { return m_has_value ? 1U : 0U; }
+
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+  bool Update() override;
+
+private:
+  bool m_has_value = false;
+  StdLib m_stdlib;
+};
+
+} // namespace
+
+GenericOptionalFrontend::GenericOptionalFrontend(ValueObject &valobj, StdLib stdlib)
+: SyntheticChildrenFrontEnd(valobj), m_stdlib(stdlib) {
+  if (auto target_sp = m_backend.GetTargetSP()) {
+Update();
+  }
+}
+
+
+bool GenericOptionalFrontend::Update() {
+  ValueObjectSP val_sp = m_backend.GetChildMemberWithName(ConstString("_M_payload"), true);
+  
+  if (!val_sp->GetChildMemberWithName(ConstString("_M_engaged"), true))
+return false;
+
+  // _M_engaged is a bool flag and is true if the optional contains a value.
+  // Converting it to unsigned gives us a size of 1 if it contains a value
+  // and 0 if not.
+  m_has_value = val_sp
+  ->GetChildMemberWithName(ConstString("_M_engaged"), true)
+  ->GetValueAsUnsigned(0) != 0;
+
+  return false;
+}
+
+ValueObjectSP GenericOptionalFrontend::GetChildAtIndex(size_t _idx) {
+  if (!m_has_value)
+return ValueObjectSP();
+
+  // _M_value contains the underlying value of an optional if it has one.
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("_M_payload"), true)
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("_M_payload"), true)
+  ->GetChildMemberWithName(ConstString("_M_value"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString("Value"));
+}
+
+SyntheticChildrenFrontEnd *formatters::LibStdcppOptionalSyntheticFrontEndCreator(
+CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new GenericOptionalFrontend(*valobj_sp,
+ GenericOptionalFrontend::StdLib::LibStdcpp);
+  return nullptr;
+}
+
+// SyntheticChildrenFrontEnd *formatters::LibcxxOptionalSyntheticFrontEndCreator(
+// CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
+//   if (valobj_sp)
+// return new GenericOptionalFrontend(*valobj_sp,
+//  GenericOptionalFrontend::StdLib::LibCxx);
+//   return nullptr;
+// }
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- 

Re: [Lldb-commits] [lldb] ee4b462 - [lldb] Fix a warning

[Shafik Yaghmour via lldb-commits]

I am wondering if you also need to check if token != LLDB_INVALID_ADDRESS

> On Dec 4, 2021, at 6:34 PM, Kazu Hirata via lldb-commits 
>  wrote:
> 
> 
> Author: Kazu Hirata
> Date: 2021-12-04T18:34:29-08:00
> New Revision: ee4b462693b1ffeccfe1b8fcf0a0c12896ac6e6a
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/ee4b462693b1ffeccfe1b8fcf0a0c12896ac6e6a
> DIFF: 
> https://github.com/llvm/llvm-project/commit/ee4b462693b1ffeccfe1b8fcf0a0c12896ac6e6a.diff
> 
> LOG: [lldb] Fix a warning
> 
> This patch fixes:
> 
>  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp:386:13:
>  error: comparison between NULL and non-pointer ('lldb::addr_t' (aka
>  'unsigned long') and NULL) [-Werror,-Wnull-arithmetic]
> 
> Added: 
> 
> 
> Modified: 
>lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp 
> b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
> index 0e25e9a8199bd..d41d422576a9f 100644
> --- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
> +++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
> @@ -383,7 +383,7 @@ uint32_t PlatformWindows::DoLoadImage(Process *process,
> return LLDB_INVALID_IMAGE_TOKEN;
>   }
> 
> -  if (token == NULL) {
> +  if (!token) {
> // XXX(compnerd) should we use the compiler to get the sizeof(unsigned)?
> uint64_t error_code =
> process->ReadUnsignedIntegerFromMemory(injected_result + 2 * 
> word_size + sizeof(unsigned),
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] [PATCH] D115182: [lldb] Remove some trivial scoped timers

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: JDevlieghere, aprantl.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

While profiling lldb (from swift/llvm-project), these timers were noticed to be 
short lived and high firing, and so they add noise more than value.

The data points I recorded are:

`FindTypes_Impl`: 49,646 calls, 812ns avg, 40.33ms total
`AppendSymbolIndexesWithName`: 36,229 calls, 913ns avg, 33.09ms total
`FindAllSymbolsWithNameAndType`: 36,229 calls, 1.93µs avg, 70.05ms total
`FindSymbolsWithNameAndType`: 23,263 calls, 3.09µs avg, 71.88ms total


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115182

Files:
  lldb/source/Core/Module.cpp
  lldb/source/Symbol/Symtab.cpp


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -663,7 +663,6 @@
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   if (symbol_name) {
 if (!m_name_indexes_computed)
   InitNameIndexes();
@@ -808,7 +807,6 @@
   std::vector &symbol_indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   // Initialize all of the lookup by name indexes before converting NAME to a
   // uniqued string NAME_STR below.
   if (!m_name_indexes_computed)
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -940,7 +940,6 @@
 size_t max_matches,
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {
-  LLDB_SCOPED_TIMER();
   if (SymbolFile *symbols = GetSymbolFile())
 symbols->FindTypes(name, parent_decl_ctx, max_matches,
searched_symbol_files, types);
@@ -1346,9 +1345,6 @@
   SymbolContextList &sc_list) {
   // No need to protect this call using m_mutex all other method calls are
   // already thread safe.
-  LLDB_SCOPED_TIMERF(
-  "Module::FindSymbolsWithNameAndType (name = %s, type = %i)",
-  name.AsCString(), symbol_type);
   if (Symtab *symtab = GetSymtab()) {
 std::vector symbol_indexes;
 symtab->FindAllSymbolsWithNameAndType(name, symbol_type, symbol_indexes);


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -663,7 +663,6 @@
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   if (symbol_name) {
 if (!m_name_indexes_computed)
   InitNameIndexes();
@@ -808,7 +807,6 @@
   std::vector &symbol_indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   // Initialize all of the lookup by name indexes before converting NAME to a
   // uniqued string NAME_STR below.
   if (!m_name_indexes_computed)
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -940,7 +940,6 @@
 size_t max_matches,
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {
-  LLDB_SCOPED_TIMER();
   if (SymbolFile *symbols = GetSymbolFile())
 symbols->FindTypes(name, parent_decl_ctx, max_matches,
searched_symbol_files, types);
@@ -1346,9 +1345,6 @@
   SymbolContextList &sc_list) {
   // No need to protect this call using m_mutex all other method calls are
   // already thread safe.
-  LLDB_SCOPED_TIMERF(
-  "Module::FindSymbolsWithNameAndType (name = %s, type = %i)",
-  name.AsCString(), symbol_type);
   if (Symtab *symtab = GetSymtab()) {
 std::vector symbol_indexes;
 symtab->FindAllSymbolsWithNameAndType(name, symbol_type, symbol_indexes);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D115182: [lldb] Remove some trivial scoped timers

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

These functions seem hot enough that the overhead of the timer outweighs the 
value it brings. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115182

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

[Lldb-commits] [PATCH] D115178: Unify libstdcpp and libcxx formatters for `std::optional`

[walter erquinigo via Phabricator via lldb-commits]

wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.

back to your queue until you get things working


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115178

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

[Lldb-commits] [lldb] 2ea3c8a - [formatters] Add a deque formatter for libstdcpp and fix the libcxx one

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-12-06T13:42:03-08:00
New Revision: 2ea3c8a50add5436cf939d59c3235408ca0255c1

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

LOG: [formatters] Add a deque formatter for libstdcpp and fix the libcxx one

This adds the formatters for libstdcpp's deque as a python
implementation. It adds comprehensive tests for the two different
storage strategies deque uses. Besides that, this fixes a couple of bugs
in the libcxx implementation. Finally, both implementation run against
the same tests.

This is a minor improvement on top of Danil Stefaniuc's formatter.

Added: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/deque/Makefile

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/deque/TestDataFormatterGenericDeque.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/deque/main.cpp

Modified: 
lldb/examples/synthetic/gnu_libstdcpp.py
lldb/examples/synthetic/libcxx.py
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

Removed: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/Makefile

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/main.cpp



diff  --git a/lldb/examples/synthetic/gnu_libstdcpp.py 
b/lldb/examples/synthetic/gnu_libstdcpp.py
index 87db880288265..022071322d058 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -684,3 +684,135 @@ def has_children(self):
 return True
 
 _list_uses_loop_detector = True
+
+class StdDequeSynthProvider:
+def __init__(self, valobj, d):
+self.valobj = valobj
+self.pointer_size = self.valobj.GetProcess().GetAddressByteSize()
+self.count = None
+self.block_size = -1
+self.element_size = -1
+self.find_block_size()
+
+
+def find_block_size(self):
+# in order to use the deque we must have the block size, or else
+# it's impossible to know what memory addresses are valid
+self.element_type = self.valobj.GetType().GetTemplateArgumentType(0)
+if not self.element_type.IsValid():
+return
+self.element_size = self.element_type.GetByteSize()
+# The block size (i.e. number of elements per subarray) is defined in
+# this piece of code, so we need to replicate it.
+#
+# #define _GLIBCXX_DEQUE_BUF_SIZE 512
+#
+# return (__size < _GLIBCXX_DEQUE_BUF_SIZE
+   #   ? size_t(_GLIBCXX_DEQUE_BUF_SIZE / __size) : size_t(1));
+if self.element_size < 512:
+self.block_size = 512 // self.element_size
+else:
+self.block_size = 1
+
+def num_children(self):
+if self.count is None:
+return 0
+return self.count
+
+def has_children(self):
+return True
+
+def get_child_index(self, name):
+try:
+return int(name.lstrip('[').rstrip(']'))
+except:
+return -1
+
+def get_child_at_index(self, index):
+if index < 0 or self.count is None:
+return None
+if index >= self.num_children():
+return None
+try:
+name = '[' + str(index) + ']'
+# We first look for the element in the first subarray,
+# which might be incomplete.
+if index < self.first_node_size:
+# The following statement is valid because self.first_elem is 
the pointer
+# to the first element
+return self.first_elem.CreateChildAtOffset(name, index * 
self.element_size, self.element_type)
+
+# Now the rest of the subarrays except for maybe the last one
+# are going to be complete, so the final expression is simpler
+i, j = divmod(index - self.first_node_size, self.block_size)
+
+# We first move to the beginning of the node/subarray were our 
element is
+node = self.start_node.CreateChildAtOffset(
+'',
+(1 + i) * self.valobj.GetProcess().GetAddressByteSize(),
+self.element_type.GetPointerType())
+return node.CreateChildAtOffset(name, j * self.element_size, 
self.element_type)
+
+except:
+return None
+
+def update(self):
+logger = lldb.formatters.Logger.Logger()
+self.count = 0
+try:
+# A deque is effectively a two-dim array, with fixed width.
+# However, only a subset of this memory contains valid data
+# since a deque may have some slack at

[Lldb-commits] [lldb] 6622c14 - [formatters] Add a pointer and reference tests for a list and forward_list formatters for libstdcpp and libcxx

[Walter Erquinigo via lldb-commits]

Author: Danil Stefaniuc
Date: 2021-12-06T13:42:03-08:00
New Revision: 6622c1411339488aa086704ac3fad3b040604dff

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

LOG: [formatters] Add a pointer and reference tests for a list and forward_list 
formatters for libstdcpp and libcxx

This adds extra tests for libstdcpp and libcxx list and forward_list formatters 
to check whether formatter behaves correctly when applied on pointer and 
reference values.

Reviewed By: wallace

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

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 589fb3190ac6a..55b3af8123073 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -936,7 +936,7 @@ static void 
LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
   RegularExpression("^std::(__cxx11::)?list<.+>(( )?&)?$"),
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
-  stl_synth_flags,
+  stl_deref_flags,
   "lldb.formatters.cpp.gnu_libstdcpp.StdListSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
   RegularExpression("^std::(__cxx11::)?forward_list<.+>(( )?&)?$"),

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
index c99807dd3cf0a..8870c4ce79c71 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
@@ -84,10 +84,100 @@ def do_test(self, stdlib_type):
  '...'
  ])
 
+def do_test_ptr_and_ref(self, stdlib_type):
+"""Test that ref and ptr to std::forward_list is displayed correctly"""
+self.build(dictionary={stdlib_type: "1"})
+
+(_, process, _, bkpt) = lldbutil.run_to_source_breakpoint(self,
+'Check ref and ptr',
+lldb.SBFileSpec("main.cpp", False))
+
+self.expect("frame variable ref",
+substrs=[
+ 'size=0',
+ '{}'])
+
+self.expect("frame variable *ptr",
+substrs=[
+ 'size=0',
+   '{}'])
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+self.expect("frame variable ref",
+substrs=[
+ '{',
+ '[0] = 47',
+ '}'])
+
+self.expect("frame variable *ptr",
+substrs=[
+ '{',
+ '[0] = 47',
+ '}'])
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+self.expect("frame variable ref",
+substrs=[
+ 'size=5',
+ '{',
+ '[0] = 1',
+ '[1] = 22',
+ '[2] = 333',
+ '[3] = ',
+ '[4] = 5',
+ '}'])
+
+self.expect("frame variable *ptr",
+substrs=[
+ 'size=5',
+ '{',
+ '[0] = 1',
+ '[1] = 22',
+ '[2] = 333',
+ '[3] = ',
+ '[4] = 5',
+ '}'])
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+self.runCmd(
+"settings set target.max-children-count 256",
+check=False)
+
+self.e

[Lldb-commits] [PATCH] D115137: [formatters] Add a pointer and reference tests for a list and forward_list formatters for libstdcpp and libcxx

[Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6622c1411339: [formatters] Add a pointer and reference tests 
for a list and forward_list… (authored by danilashtefan, committed by Walter 
Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115137

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/main.cpp
@@ -4,6 +4,12 @@
 typedef std::list int_list;
 typedef std::list string_list;
 
+
+template  void by_ref_and_ptr(T &ref, T *ptr) {
+  // Check ref and ptr
+  return;
+}
+
 int main()
 {
 int_list numbers_list;
@@ -21,13 +27,16 @@
 numbers_list.push_back(2);
 numbers_list.push_back(3);
 numbers_list.push_back(4);
+
+by_ref_and_ptr(numbers_list, &numbers_list);
 
 string_list text_list;
 text_list.push_back(std::string("goofy")); // Optional break point at this line.
 text_list.push_back(std::string("is"));
 text_list.push_back(std::string("smart"));
-
 text_list.push_back(std::string("!!!"));
+
+by_ref_and_ptr(text_list, &text_list);
 
 return 0; // Set final break point at this line.
 }
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
@@ -205,11 +205,58 @@
 self.assertTrue(
 self.frame().FindVariable("text_list").MightHaveChildren(),
 "text_list.MightHaveChildren() says False for non empty!")
+
+def do_test_ptr_and_ref(self, stdlib_type):
+"""Test that ref and ptr to std::list is displayed correctly"""
+self.build(dictionary={stdlib_type: "1"})
+
+(_, process, _, bkpt) = lldbutil.run_to_source_breakpoint(self,
+'Check ref and ptr',
+lldb.SBFileSpec("main.cpp", False))
+
+self.expect("frame variable ref",
+substrs=['size=4',
+ '[0] = ', '1',
+ '[1] = ', '2',
+ '[2] = ', '3',
+ '[3] = ', '4'])
+
+
+self.expect("frame variable *ptr",
+substrs=['size=4',
+ '[0] = ', '1',
+ '[1] = ', '2',
+ '[2] = ', '3',
+ '[3] = ', '4'])
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+self.expect("frame variable ref",
+substrs=['size=4',
+ '[0]', 'goofy',
+ '[1]', 'is',
+ '[2]', 'smart',
+ '[3]', '!!!'])
+
+self.expect("frame variable *ptr",
+substrs=['size=4',
+ '[0]', 'goofy',
+ '[1]', 'is',
+ '[2]', 'smart',
+ '[3]', '!!!'])
 
 @add_test_categories(["libstdcxx"])
 def test_with_run_command_libstdcpp(self):
 self.do_test_with_run_command(USE_LIBSTDCPP)
 
+@add_test_categories(["libstdcxx"])
+def test_ptr_and_ref_libstdcpp(self):
+self.do_test_ptr_and_ref(USE_LIBSTDCPP)
+
 @add_test_categories(["libc++"])
 def test_with_run_command_libcpp(self):
-self.do_test_with_run_command(USE_LIBCPP)
\ No newline at end of file
+self.do_test_with_run_command(USE_LIBCPP)
+
+@add_test_categories(["libc++"])
+def test_ptr_and_ref_libcpp(self):
+self.do_test_ptr_and_ref(USE_LIBCPP)
\ No newline at end of file
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp
+++ lldb/te

[Lldb-commits] [PATCH] D114746: [lldb] Generalize ParsedDWARFTypeAttributes

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

I am happy as long as Pavel is good.


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

https://reviews.llvm.org/D114746

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

[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 392177.
clayborg added a comment.

Updated to make DataEncoder always copy any data and own the buffer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

Files:
  lldb/include/lldb/Utility/DataEncoder.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/Platform/Android/AdbClient.cpp
  lldb/source/Utility/DataEncoder.cpp
  lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/DataEncoderTest.cpp

Index: lldb/unittests/Utility/DataEncoderTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/DataEncoderTest.cpp
@@ -0,0 +1,534 @@
+//===-- DataEncoderTest.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 "gtest/gtest.h"
+
+#include "lldb/Utility/DataEncoder.h"
+#include "llvm/ADT/ArrayRef.h"
+#include 
+using namespace lldb_private;
+using namespace llvm;
+
+TEST(DataEncoderTest, PutU8) {
+  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const uint32_t addr_size = 4;
+
+  uint32_t offset = 0;
+  DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderLittle,
+  addr_size);
+  offset = encoder.PutU8(offset, 11);
+  ASSERT_EQ(offset, 1U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 2, 3, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 12);
+  ASSERT_EQ(offset, 2U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 3, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 13);
+  ASSERT_EQ(offset, 3U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 14);
+  ASSERT_EQ(offset, 4U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 14, 5, 6, 7, 8}));
+  // Check that putting a number to an invalid offset doesn't work and returns
+  // an error offset and doesn't modify the buffer.
+  ASSERT_EQ(encoder.PutU8(init.size(), 15), UINT32_MAX);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 14, 5, 6, 7, 8}));
+}
+
+TEST(DataEncoderTest, AppendUnsignedLittle) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendU8(0x11);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11}));
+  encoder.AppendU16(0x2233);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x33, 0x22}));
+  encoder.AppendU32(0x44556677);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x33, 0x22, 0x77, 0x66, 0x55, 0x44}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x33, 0x22, 0x77, 0x66, 0x55, 0x44,
+   0xFF, 0xEE, 0xDD, 0xCC, 0xBB, 0xAA, 0x99, 0x88}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+}
+
+TEST(DataEncoderTest, AppendUnsignedBig) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderBig, addr_size);
+  encoder.AppendU8(0x11);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11}));
+  encoder.AppendU16(0x2233);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x22, 0x33}));
+  encoder.AppendU32(0x44556677);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+   0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}));
+}
+
+TEST(DataEncoderTest, AppendAddress4Little) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x44, 0x33, 0x22, 0x11}));
+  encoder.AppendAddress(0x55);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x44, 0x33, 0x22, 0x11, 0x55, 0x00, 0x00, 0x00}));
+}
+
+TEST(DataEncoderTest, AppendAddress4Big) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderBig, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x22, 0x33, 0x44}));
+  encoder.AppendAddress(0x55);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x00, 0x00, 0x00, 0x55}));
+}
+
+TEST(DataEncoderTest, AppendAddress8Little) {
+  const uint32_t addr_size = 8;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x44, 0x33, 0x22, 0x11, 0x00, 0x00, 0x0

[Lldb-commits] [PATCH] D114923: [lldb/plugins] Add arm64(e) support to ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 392178.
mib marked an inline comment as done.
mib edited the summary of this revision.
mib added a comment.

Add invalid Scripted Thread test, addressing @JDevlieghere comment.


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

https://reviews.llvm.org/D114923

Files:
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py

Index: lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
@@ -0,0 +1,84 @@
+import os,struct, signal
+
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class InvalidScriptedProcess(ScriptedProcess):
+def __init__(self, target: lldb.SBTarget, args : lldb.SBStructuredData):
+super().__init__(target, args)
+
+def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+return None
+
+def get_thread_with_id(self, tid: int):
+return {}
+
+def get_registers_for_thread(self, tid: int):
+return {}
+
+def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+return None
+
+def get_loaded_images(self):
+return self.loaded_images
+
+def get_process_id(self) -> int:
+return 666
+
+def should_stop(self) -> bool:
+return True
+
+def is_alive(self) -> bool:
+return True
+
+def get_scripted_thread_plugin(self):
+return InvalidScriptedThread.__module__ + "." + InvalidScriptedThread.__name__
+
+
+class InvalidScriptedThread(ScriptedThread):
+def __init__(self, process, args):
+super().__init__(process, args)
+
+def get_thread_id(self) -> int:
+return 0x19
+
+def get_name(self) -> str:
+return InvalidScriptedThread.__name__ + ".thread-1"
+
+def get_state(self) -> int:
+return lldb.eStateInvalid
+
+def get_stop_reason(self) -> Dict[str, Any]:
+return { "type": lldb.eStopReasonSignal, "data": {
+"signal": signal.SIGINT
+} }
+
+def get_stackframes(self):
+class ScriptedStackFrame:
+def __init__(idx, cfa, pc, symbol_ctx):
+self.idx = idx
+self.cfa = cfa
+self.pc = pc
+self.symbol_ctx = symbol_ctx
+
+
+symbol_ctx = lldb.SBSymbolContext()
+frame_zero = ScriptedStackFrame(0, 0x42424242, 0x500, symbol_ctx)
+self.frames.append(frame_zero)
+
+return self.frame_zero[0:0]
+
+def get_register_context(self) -> str:
+return None
+
+def __lldb_init_module(debugger, dict):
+if not 'SKIP_SCRIPTED_PROCESS_LAUNCH' in os.environ:
+debugger.HandleCommand(
+"process launch -C %s.%s" % (__name__,
+ InvalidScriptedProcess.__name__))
+else:
+print("Name of the class that will manage the scripted process: '%s.%s'"
+% (__name__, InvalidScriptedProcess.__name__))
\ No newline at end of file
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -41,6 +41,38 @@
 self.expect('script dir(ScriptedProcess)',
 substrs=["launch"])
 
+def test_invalid_scripted_register_context(self):
+"""Test that we can launch an lldb scripted process with an invalid
+Scripted Thread, with invalid register context."""
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target, VALID_TARGET)
+
+os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
+def cleanup():
+  del os.environ["SKIP_SCRIPTED_PROCESS_LAUNCH"]
+self.addTearDownHook(cleanup)
+
+scripted_process_example_relpath = 'invalid_scripted_process.py'
+self.runCmd("command script import " + os.path.join(self.getSourceDir(),
+scripted_process_example_relpath))
+
+launch_info = lldb.SBLaunchInfo(None)
+launch_info.SetProcessPluginName("ScriptedProcess")
+launch_info.SetScriptedProcessClassName("invalid_scripted_process.InvalidScriptedProcess")
+error = lldb.SBError()
+with tempfile.NamedTemporaryFile() as log_file:
+self.runCmd("log en

[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 392187.
clayborg added a comment.

Update the headerdoc and fixed some of the comments that were out of date or 
left over from the inital copy from DataExtractor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

Files:
  lldb/include/lldb/Utility/DataEncoder.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/Platform/Android/AdbClient.cpp
  lldb/source/Utility/DataEncoder.cpp
  lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/DataEncoderTest.cpp

Index: lldb/unittests/Utility/DataEncoderTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/DataEncoderTest.cpp
@@ -0,0 +1,534 @@
+//===-- DataEncoderTest.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 "gtest/gtest.h"
+
+#include "lldb/Utility/DataEncoder.h"
+#include "llvm/ADT/ArrayRef.h"
+#include 
+using namespace lldb_private;
+using namespace llvm;
+
+TEST(DataEncoderTest, PutU8) {
+  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const uint32_t addr_size = 4;
+
+  uint32_t offset = 0;
+  DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderLittle,
+  addr_size);
+  offset = encoder.PutU8(offset, 11);
+  ASSERT_EQ(offset, 1U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 2, 3, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 12);
+  ASSERT_EQ(offset, 2U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 3, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 13);
+  ASSERT_EQ(offset, 3U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 14);
+  ASSERT_EQ(offset, 4U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 14, 5, 6, 7, 8}));
+  // Check that putting a number to an invalid offset doesn't work and returns
+  // an error offset and doesn't modify the buffer.
+  ASSERT_EQ(encoder.PutU8(init.size(), 15), UINT32_MAX);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 14, 5, 6, 7, 8}));
+}
+
+TEST(DataEncoderTest, AppendUnsignedLittle) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendU8(0x11);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11}));
+  encoder.AppendU16(0x2233);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x33, 0x22}));
+  encoder.AppendU32(0x44556677);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x33, 0x22, 0x77, 0x66, 0x55, 0x44}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x33, 0x22, 0x77, 0x66, 0x55, 0x44,
+   0xFF, 0xEE, 0xDD, 0xCC, 0xBB, 0xAA, 0x99, 0x88}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+}
+
+TEST(DataEncoderTest, AppendUnsignedBig) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderBig, addr_size);
+  encoder.AppendU8(0x11);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11}));
+  encoder.AppendU16(0x2233);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x22, 0x33}));
+  encoder.AppendU32(0x44556677);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+   0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}));
+}
+
+TEST(DataEncoderTest, AppendAddress4Little) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x44, 0x33, 0x22, 0x11}));
+  encoder.AppendAddress(0x55);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x44, 0x33, 0x22, 0x11, 0x55, 0x00, 0x00, 0x00}));
+}
+
+TEST(DataEncoderTest, AppendAddress4Big) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderBig, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x22, 0x33, 0x44}));
+  encoder.AppendAddress(0x55);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x00, 0x00, 0x00, 0x55}));
+}
+
+TEST(DataEncoderTest, AppendAddress8Little) {
+  const uint32_t addr_size = 8;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(),
+ 

[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 392191.
clayborg added a comment.

More documentation fixed for DataEncoder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

Files:
  lldb/include/lldb/Utility/DataEncoder.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/Platform/Android/AdbClient.cpp
  lldb/source/Utility/DataEncoder.cpp
  lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/DataEncoderTest.cpp

Index: lldb/unittests/Utility/DataEncoderTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/DataEncoderTest.cpp
@@ -0,0 +1,534 @@
+//===-- DataEncoderTest.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 "gtest/gtest.h"
+
+#include "lldb/Utility/DataEncoder.h"
+#include "llvm/ADT/ArrayRef.h"
+#include 
+using namespace lldb_private;
+using namespace llvm;
+
+TEST(DataEncoderTest, PutU8) {
+  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const uint32_t addr_size = 4;
+
+  uint32_t offset = 0;
+  DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderLittle,
+  addr_size);
+  offset = encoder.PutU8(offset, 11);
+  ASSERT_EQ(offset, 1U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 2, 3, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 12);
+  ASSERT_EQ(offset, 2U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 3, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 13);
+  ASSERT_EQ(offset, 3U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 14);
+  ASSERT_EQ(offset, 4U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 14, 5, 6, 7, 8}));
+  // Check that putting a number to an invalid offset doesn't work and returns
+  // an error offset and doesn't modify the buffer.
+  ASSERT_EQ(encoder.PutU8(init.size(), 15), UINT32_MAX);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 14, 5, 6, 7, 8}));
+}
+
+TEST(DataEncoderTest, AppendUnsignedLittle) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendU8(0x11);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11}));
+  encoder.AppendU16(0x2233);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x33, 0x22}));
+  encoder.AppendU32(0x44556677);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x33, 0x22, 0x77, 0x66, 0x55, 0x44}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x33, 0x22, 0x77, 0x66, 0x55, 0x44,
+   0xFF, 0xEE, 0xDD, 0xCC, 0xBB, 0xAA, 0x99, 0x88}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+}
+
+TEST(DataEncoderTest, AppendUnsignedBig) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderBig, addr_size);
+  encoder.AppendU8(0x11);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11}));
+  encoder.AppendU16(0x2233);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x22, 0x33}));
+  encoder.AppendU32(0x44556677);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+   0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}));
+}
+
+TEST(DataEncoderTest, AppendAddress4Little) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x44, 0x33, 0x22, 0x11}));
+  encoder.AppendAddress(0x55);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x44, 0x33, 0x22, 0x11, 0x55, 0x00, 0x00, 0x00}));
+}
+
+TEST(DataEncoderTest, AppendAddress4Big) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderBig, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x22, 0x33, 0x44}));
+  encoder.AppendAddress(0x55);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x00, 0x00, 0x00, 0x55}));
+}
+
+TEST(DataEncoderTest, AppendAddress8Little) {
+  const uint32_t addr_size = 8;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x44, 0x33, 0x22, 0x11, 0x00, 0x00, 0x00, 0x00}));
+  encoder.Appe

[Lldb-commits] [PATCH] D115178: Unify libstdcpp and libcxx formatters for `std::optional`

[Alisamar Husain via Phabricator via lldb-commits]

zrthxn updated this revision to Diff 392197.
zrthxn added a comment.

Added libcxx support


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115178

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/Generic.h
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h

Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
@@ -45,6 +45,10 @@
 LibStdcppBitsetSyntheticFrontEndCreator(CXXSyntheticChildren *,
 lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibStdcppOptionalSyntheticFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP);
+
 SyntheticChildrenFrontEnd *
 LibStdcppVectorIteratorSyntheticFrontEndCreator(CXXSyntheticChildren *,
 lldb::ValueObjectSP);
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -75,10 +75,10 @@
   return val_sp->Clone(ConstString("Value"));
 }
 
-SyntheticChildrenFrontEnd *
-formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
-  lldb::ValueObjectSP valobj_sp) {
-  if (valobj_sp)
-return new OptionalFrontEnd(*valobj_sp);
-  return nullptr;
-}
+// SyntheticChildrenFrontEnd *
+// formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+//   lldb::ValueObjectSP valobj_sp) {
+//   if (valobj_sp)
+// return new OptionalFrontEnd(*valobj_sp);
+//   return nullptr;
+// }
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -170,8 +170,8 @@
   lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *
-LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
-  lldb::ValueObjectSP valobj_sp);
+LibcxxOptionalSyntheticFrontEndCreator(CXXSyntheticChildren *,
+   lldb::ValueObjectSP valobj_sp);
 
 SyntheticChildrenFrontEnd *
 LibcxxVariantFrontEndCreator(CXXSyntheticChildren *,
Index: lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
@@ -0,0 +1,136 @@
+//===-- GenericOptional.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 "Generic.h"
+#include "LibCxx.h"
+#include "LibStdcpp.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+#include "lldb/Target/Target.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class GenericOptionalFrontend : public SyntheticChildrenFrontEnd {
+public:
+  enum class StdLib {
+LibCxx,
+LibStdcpp,
+  };
+
+  GenericOptionalFrontend(ValueObject &valobj, StdLib stdlib);
+
+  size_t GetIndexOfChildWithName(ConstString name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  size_t CalculateNumChildren() override { return m_has_value ? 1U : 0U; }
+
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+  bool Update() override;
+
+private:
+  bool m_has_value = false;
+  StdLib m_stdlib;
+};
+
+} // namespace
+
+GenericOptionalFrontend::GenericOptionalFrontend(ValueObject &valobj,
+ StdLib stdlib)
+: SyntheticChildrenFrontEnd(valobj), m_stdlib(stdlib) {
+  if (auto target_sp = m_backend.GetTargetSP()) {
+Update();
+  }
+}
+
+bool GenericOptionalFrontend::Update() {
+  ValueObjectSP engaged_sp;
+
+  if (m_stdlib == StdLib::LibCxx)
+engaged_sp =
+m_backend.GetChildMemberWithName(ConstString("__engaged_"), true);
+  else if (m_stdlib == StdLib::LibStdcpp)
+engaged_sp =
+m_backe

[Lldb-commits] [PATCH] D115178: Unify libstdcpp and libcxx formatters for `std::optional`

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

good job! Only some cosmetic changes are needed and you also need to delete the 
python code




Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:775-779
+  // AddCXXSummary(cpp_category_sp,
+  //   lldb_private::formatters::LibcxxOptionalSummaryProvider,
+  //   "libc++ std::optional summary provider",
+  //   ConstString("^std::__[[:alnum:]]+::optional<.+>(( )?&)?$"),
+  //   stl_summary_flags, true);

remove this



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:922-926
+  // cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
+  // RegularExpression("^std::optional<.+>(( )?&)?$"),
+  // SyntheticChildrenSP(new ScriptedSyntheticChildren(
+  // stl_synth_flags,
+  // "lldb.formatters.cpp.gnu_libstdcpp.StdOptionalSynthProvider")));

same here



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:950-954
+  // cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
+  // RegularExpression("^std::optional<.+>(( )?&)?$"),
+  // TypeSummaryImplSP(new ScriptSummaryFormat(
+  // stl_summary_flags,
+  // "lldb.formatters.cpp.gnu_libstdcpp.StdOptionalSummaryProvider")));

same



Comment at: lldb/source/Plugins/Language/CPlusPlus/Generic.h:1-2
+//===-- LibCxx.h ---*- C++
+//-*-===//
+//

make this one line (=80 chars)



Comment at: lldb/source/Plugins/Language/CPlusPlus/Generic.h:10-11
+
+// #ifndef LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_LIBCXX_H
+// #define LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_LIBCXX_H
+

this shouldn't be a comment, and use
  LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_GENERIC_H



Comment at: lldb/source/Plugins/Language/CPlusPlus/Generic.h:26
+
+// #endif // LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_LIBCXX_H

LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_GENERIC_H



Comment at: lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp:1-2
+//===-- GenericOptional.cpp
+---===//
+//

80 chars



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp:78-84
+// SyntheticChildrenFrontEnd *
+// formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+//   lldb::ValueObjectSP valobj_sp) {
+//   if (valobj_sp)
+// return new OptionalFrontEnd(*valobj_sp);
+//   return nullptr;
+// }

delete this entire file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115178

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

[Lldb-commits] [lldb] 13278ef - [lldb] Remove some trivial scoped timers

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2021-12-06T15:22:28-08:00
New Revision: 13278efd0c95a8b589bbd7dbbf17680a03b6eb77

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

LOG: [lldb] Remove some trivial scoped timers

While profiling lldb (from swift/llvm-project), these timers were noticed to be 
short lived and high firing, and so they add noise more than value.

The data points I recorded are:

`FindTypes_Impl`: 49,646 calls, 812ns avg, 40.33ms total
`AppendSymbolIndexesWithName`: 36,229 calls, 913ns avg, 33.09ms total
`FindAllSymbolsWithNameAndType`: 36,229 calls, 1.93µs avg, 70.05ms total
`FindSymbolsWithNameAndType`: 23,263 calls, 3.09µs avg, 71.88ms total

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

Added: 


Modified: 
lldb/source/Core/Module.cpp
lldb/source/Symbol/Symtab.cpp

Removed: 




diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index cbecbb9aa5fed..dbec8250223cf 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -940,7 +940,6 @@ void Module::FindTypes_Impl(
 size_t max_matches,
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {
-  LLDB_SCOPED_TIMER();
   if (SymbolFile *symbols = GetSymbolFile())
 symbols->FindTypes(name, parent_decl_ctx, max_matches,
searched_symbol_files, types);
@@ -1346,9 +1345,6 @@ void Module::FindSymbolsWithNameAndType(ConstString name,
   SymbolContextList &sc_list) {
   // No need to protect this call using m_mutex all other method calls are
   // already thread safe.
-  LLDB_SCOPED_TIMERF(
-  "Module::FindSymbolsWithNameAndType (name = %s, type = %i)",
-  name.AsCString(), symbol_type);
   if (Symtab *symtab = GetSymtab()) {
 std::vector symbol_indexes;
 symtab->FindAllSymbolsWithNameAndType(name, symbol_type, symbol_indexes);

diff  --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index c67955523bfb1..0057b9e6c8fb2 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -663,7 +663,6 @@ uint32_t Symtab::AppendSymbolIndexesWithName(ConstString 
symbol_name,
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   if (symbol_name) {
 if (!m_name_indexes_computed)
   InitNameIndexes();
@@ -808,7 +807,6 @@ Symtab::FindAllSymbolsWithNameAndType(ConstString name,
   std::vector &symbol_indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   // Initialize all of the lookup by name indexes before converting NAME to a
   // uniqued string NAME_STR below.
   if (!m_name_indexes_computed)



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

[Lldb-commits] [PATCH] D115182: [lldb] Remove some trivial scoped timers

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG13278efd0c95: [lldb] Remove some trivial scoped timers 
(authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115182

Files:
  lldb/source/Core/Module.cpp
  lldb/source/Symbol/Symtab.cpp


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -663,7 +663,6 @@
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   if (symbol_name) {
 if (!m_name_indexes_computed)
   InitNameIndexes();
@@ -808,7 +807,6 @@
   std::vector &symbol_indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   // Initialize all of the lookup by name indexes before converting NAME to a
   // uniqued string NAME_STR below.
   if (!m_name_indexes_computed)
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -940,7 +940,6 @@
 size_t max_matches,
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {
-  LLDB_SCOPED_TIMER();
   if (SymbolFile *symbols = GetSymbolFile())
 symbols->FindTypes(name, parent_decl_ctx, max_matches,
searched_symbol_files, types);
@@ -1346,9 +1345,6 @@
   SymbolContextList &sc_list) {
   // No need to protect this call using m_mutex all other method calls are
   // already thread safe.
-  LLDB_SCOPED_TIMERF(
-  "Module::FindSymbolsWithNameAndType (name = %s, type = %i)",
-  name.AsCString(), symbol_type);
   if (Symtab *symtab = GetSymtab()) {
 std::vector symbol_indexes;
 symtab->FindAllSymbolsWithNameAndType(name, symbol_type, symbol_indexes);


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -663,7 +663,6 @@
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   if (symbol_name) {
 if (!m_name_indexes_computed)
   InitNameIndexes();
@@ -808,7 +807,6 @@
   std::vector &symbol_indexes) {
   std::lock_guard guard(m_mutex);
 
-  LLDB_SCOPED_TIMER();
   // Initialize all of the lookup by name indexes before converting NAME to a
   // uniqued string NAME_STR below.
   if (!m_name_indexes_computed)
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -940,7 +940,6 @@
 size_t max_matches,
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {
-  LLDB_SCOPED_TIMER();
   if (SymbolFile *symbols = GetSymbolFile())
 symbols->FindTypes(name, parent_decl_ctx, max_matches,
searched_symbol_files, types);
@@ -1346,9 +1345,6 @@
   SymbolContextList &sc_list) {
   // No need to protect this call using m_mutex all other method calls are
   // already thread safe.
-  LLDB_SCOPED_TIMERF(
-  "Module::FindSymbolsWithNameAndType (name = %s, type = %i)",
-  name.AsCString(), symbol_type);
   if (Symtab *symtab = GetSymtab()) {
 std::vector symbol_indexes;
 symtab->FindAllSymbolsWithNameAndType(name, symbol_type, symbol_indexes);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D114923: [lldb/plugins] Add arm64(e) support to ScriptedProcess

[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


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

https://reviews.llvm.org/D114923

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

[Lldb-commits] [PATCH] D114923: [lldb/plugins] Add arm64(e) support to ScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcaea440a11e4: [lldb/plugins] Add arm64(e) support to 
ScriptedProcess (authored by mib).

Changed prior to commit:
  https://reviews.llvm.org/D114923?vs=392178&id=392218#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114923

Files:
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py

Index: lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
@@ -0,0 +1,84 @@
+import os,struct, signal
+
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class InvalidScriptedProcess(ScriptedProcess):
+def __init__(self, target: lldb.SBTarget, args : lldb.SBStructuredData):
+super().__init__(target, args)
+
+def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+return None
+
+def get_thread_with_id(self, tid: int):
+return {}
+
+def get_registers_for_thread(self, tid: int):
+return {}
+
+def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+return None
+
+def get_loaded_images(self):
+return self.loaded_images
+
+def get_process_id(self) -> int:
+return 666
+
+def should_stop(self) -> bool:
+return True
+
+def is_alive(self) -> bool:
+return True
+
+def get_scripted_thread_plugin(self):
+return InvalidScriptedThread.__module__ + "." + InvalidScriptedThread.__name__
+
+
+class InvalidScriptedThread(ScriptedThread):
+def __init__(self, process, args):
+super().__init__(process, args)
+
+def get_thread_id(self) -> int:
+return 0x19
+
+def get_name(self) -> str:
+return InvalidScriptedThread.__name__ + ".thread-1"
+
+def get_state(self) -> int:
+return lldb.eStateInvalid
+
+def get_stop_reason(self) -> Dict[str, Any]:
+return { "type": lldb.eStopReasonSignal, "data": {
+"signal": signal.SIGINT
+} }
+
+def get_stackframes(self):
+class ScriptedStackFrame:
+def __init__(idx, cfa, pc, symbol_ctx):
+self.idx = idx
+self.cfa = cfa
+self.pc = pc
+self.symbol_ctx = symbol_ctx
+
+
+symbol_ctx = lldb.SBSymbolContext()
+frame_zero = ScriptedStackFrame(0, 0x42424242, 0x500, symbol_ctx)
+self.frames.append(frame_zero)
+
+return self.frame_zero[0:0]
+
+def get_register_context(self) -> str:
+return None
+
+def __lldb_init_module(debugger, dict):
+if not 'SKIP_SCRIPTED_PROCESS_LAUNCH' in os.environ:
+debugger.HandleCommand(
+"process launch -C %s.%s" % (__name__,
+ InvalidScriptedProcess.__name__))
+else:
+print("Name of the class that will manage the scripted process: '%s.%s'"
+% (__name__, InvalidScriptedProcess.__name__))
\ No newline at end of file
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -41,6 +41,38 @@
 self.expect('script dir(ScriptedProcess)',
 substrs=["launch"])
 
+def test_invalid_scripted_register_context(self):
+"""Test that we can launch an lldb scripted process with an invalid
+Scripted Thread, with invalid register context."""
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target, VALID_TARGET)
+
+os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
+def cleanup():
+  del os.environ["SKIP_SCRIPTED_PROCESS_LAUNCH"]
+self.addTearDownHook(cleanup)
+
+scripted_process_example_relpath = 'invalid_scripted_process.py'
+self.runCmd("command script import " + os.path.join(self.getSourceDir(),
+scripted_process_example_relpath))
+
+launch_info = lldb.SBLaunchInfo(None)
+launch_info.SetProcessPluginName("ScriptedProcess")
+launch_info.SetScriptedProcessClassName("invalid_scripted_proc

[Lldb-commits] [lldb] caea440 - [lldb/plugins] Add arm64(e) support to ScriptedProcess

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-12-06T16:11:59-08:00
New Revision: caea440a11e47bf86b0e43feb28a9287e4fbe3f8

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

LOG: [lldb/plugins] Add arm64(e) support to ScriptedProcess

This patch adds support for arm64(e) targets to ScriptedProcess, by
providing the `DynamicRegisterInfo` to the base `lldb.ScriptedThread` class.
This allows create and debugging ScriptedProcess on Apple Silicon
hardware as well as Apple mobile devices.

It also replace the C++ asserts on `ScriptedThread::GetDynamicRegisterInfo`
by some error logging, re-enables `TestScriptedProcess` for arm64
Darwin platforms and adds a new invalid Scripted Thread test.

rdar://85892451

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

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py

Modified: 
lldb/examples/python/scripted_process/scripted_process.py
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

Removed: 




diff  --git a/lldb/examples/python/scripted_process/scripted_process.py 
b/lldb/examples/python/scripted_process/scripted_process.py
index ebb48523805d..16dcc7274853 100644
--- a/lldb/examples/python/scripted_process/scripted_process.py
+++ b/lldb/examples/python/scripted_process/scripted_process.py
@@ -298,14 +298,14 @@ def get_register_info(self):
 self.register_info['registers'] = [
 {'name': 'rax', 'bitsize': 64, 'offset': 0, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 0, 'dwarf': 0},
 {'name': 'rbx', 'bitsize': 64, 'offset': 8, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 3, 'dwarf': 3},
-{'name': 'rcx', 'bitsize': 64, 'offset': 16, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 2, 'dwarf': 2, 'generic': 
'arg4', 'alt-name': 'arg4', },
-{'name': 'rdx', 'bitsize': 64, 'offset': 24, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 1, 'dwarf': 1, 'generic': 
'arg3', 'alt-name': 'arg3', },
-{'name': 'rdi', 'bitsize': 64, 'offset': 32, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 5, 'dwarf': 5, 'generic': 
'arg1', 'alt-name': 'arg1', },
-{'name': 'rsi', 'bitsize': 64, 'offset': 40, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 4, 'dwarf': 4, 'generic': 
'arg2', 'alt-name': 'arg2', },
-{'name': 'rbp', 'bitsize': 64, 'offset': 48, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 6, 'dwarf': 6, 'generic': 
'fp', 'alt-name': 'fp', },
-{'name': 'rsp', 'bitsize': 64, 'offset': 56, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 7, 'dwarf': 7, 'generic': 
'sp', 'alt-name': 'sp', },
-{'name': 'r8', 'bitsize': 64, 'offset': 64, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 8, 'dwarf': 8, 'generic': 
'arg5', 'alt-name': 'arg5', },
-{'name': 'r9', 'bitsize': 64, 'offset': 72, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 9, 'dwarf': 9, 'generic': 
'arg6', 'alt-name': 'arg6', },
+{'name': 'rcx', 'bitsize': 64, 'offset': 16, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 2, 'dwarf': 2, 'generic': 
'arg4', 'alt-name': 'arg4'},
+{'name': 'rdx', 'bitsize': 64, 'offset': 24, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 1, 'dwarf': 1, 'generic': 
'arg3', 'alt-name': 'arg3'},
+{'name': 'rdi', 'bitsize': 64, 'offset': 32, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 5, 'dwarf': 5, 'generic': 
'arg1', 'alt-name': 'arg1'},
+{'name': 'rsi', 'bitsize': 64, 'offset': 40, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 4, 'dwarf': 4, 'generic': 
'arg2', 'alt-name': 'arg2'},
+{'name': 'rbp', 'bitsize': 64, 'offset': 48, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 6, 'dwarf': 6, 'generic': 
'fp', 'alt-name': 'fp'},
+{'name': 'rsp', 'bitsize': 64, 'offset': 56, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 7, 'dwarf': 7, 'generic': 
'sp', 'alt-name': 'sp'},
+{'name': 'r8', 'bitsize': 64, 'offset': 64, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 8, 'dwarf': 8, 'generic': 
'arg5', 'alt-name': 'arg5'},
+{'name': 'r9', 'bitsize': 64, 'offset': 72, 
'encoding': 'uint', 'format': 'hex', 'set': 0, 'gcc': 9, 'dwarf': 9, 'generic': 
'arg6', 'alt-name': 'arg6'},
 {'n

[Lldb-commits] [PATCH] D113930: [LLDB][NativePDB] Fix function decl creation for class methods

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added a comment.

In D113930#3174406 , @labath wrote:

> I still think that the piecemeal addition of member functions is a problem. 
> While I don't think that changing the parsing order in 
> `Module::ParseAllDebugSymbols` would be a problem (that function is only 
> called from lldb-test), I think that would only hide the problem. Looking at 
> what happens in the DWARF case, I /think/ (the code is fairly messy, so I 
> could be wrong) that we don't create a clang ast representation for the 
> function in `ParseFunctions`. We only create an lldb_private::Function (in 
> `DWARFASTParserClang::ParseFunctionFromDWARF`). The ast entity only gets 
> created in `SymbolFileDWARF::ParseTypes` 
> (->ParseType->DWARFASTParserClang->ParseTypeFromDWARF). This kinda makes 
> sense since we only need the ast representation of the method when we start 
> dealing its class.

From what I tested on dwarf plugin, `lldb-test symbols -dump-ast ` doesn't 
print any ast decl, except a skeleton class decl. After adding some logs at 
(https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L1026),
 it doesn't get executed.

  $ lldb-test symbols -dump-ast ast-methods.exe
  Module: ast-methods.exe
  struct Struct;

Nevertheless I think it works in dwarf because it creates the decls if die is 
`DW_TAG_subprogram`/`DW_TAG_inlined_subroutine `/`DW_TAG_subroutine_type ` 
(https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L522).
 We can associate a decl with a die. But we can't associate a type record with 
a decl when parsing type stream in PDB.

> So I have a feeling that the real solution here is to avoid creating an ast 
> entry for the function in the first phase. Then you should be able to create 
> the full class declaration in the second phase, as you will have complete 
> information there.

If we don't create ast entries at `ParseFunctions`, non-class-member functions 
will not have ast decls. If we just create ast decls for non-class-member 
functions in `ParseFunctions`, we are expecting that `ParseTypes` is always 
called after `ParseFunctions` to complete the decl creation for member 
functions, which is not right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113930

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

[Lldb-commits] [PATCH] D115201: [lldb] Generalize UpdateSymbolContextScopeForType

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 created this revision.
Herald added a reviewer: shafik.
ljmf00 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115201

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -75,6 +75,7 @@
   friend class DebugMapModule;
   friend class DWARFCompileUnit;
   friend class DWARFDIE;
+  friend class DWARFASTParser;
   friend class DWARFASTParserClang;
 
   // Static Functions
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -153,14 +153,6 @@
 
   void LinkDeclToDIE(clang::Decl *decl, const DWARFDIE &die);
 
-  /// If \p type_sp is valid, calculate and set its symbol context scope, and
-  /// update the type list for its backing symbol file.
-  ///
-  /// Returns \p type_sp.
-  lldb::TypeSP
-  UpdateSymbolContextScopeForType(const lldb_private::SymbolContext &sc,
-  const DWARFDIE &die, lldb::TypeSP type_sp);
-
   /// Follow Clang Module Skeleton CU references to find a type definition.
   lldb::TypeSP ParseTypeFromClangModule(const lldb_private::SymbolContext &sc,
 const DWARFDIE &die,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1382,40 +1382,6 @@
   }
 }
 
-TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType(
-const SymbolContext &sc, const DWARFDIE &die, TypeSP type_sp) {
-  if (!type_sp)
-return type_sp;
-
-  SymbolFileDWARF *dwarf = die.GetDWARF();
-  TypeList &type_list = dwarf->GetTypeList();
-  DWARFDIE sc_parent_die = SymbolFileDWARF::GetParentSymbolContextDIE(die);
-  dw_tag_t sc_parent_tag = sc_parent_die.Tag();
-
-  SymbolContextScope *symbol_context_scope = nullptr;
-  if (sc_parent_tag == DW_TAG_compile_unit ||
-  sc_parent_tag == DW_TAG_partial_unit) {
-symbol_context_scope = sc.comp_unit;
-  } else if (sc.function != nullptr && sc_parent_die) {
-symbol_context_scope =
-sc.function->GetBlock(true).FindBlockByID(sc_parent_die.GetID());
-if (symbol_context_scope == nullptr)
-  symbol_context_scope = sc.function;
-  } else {
-symbol_context_scope = sc.module_sp.get();
-  }
-
-  if (symbol_context_scope != nullptr)
-type_sp->SetSymbolContextScope(symbol_context_scope);
-
-  // We are ready to put this type into the uniqued list up at the module
-  // level.
-  type_list.Insert(type_sp);
-
-  dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
-  return type_sp;
-}
-
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
const DWARFDIE &die,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -61,6 +61,14 @@
   const lldb_private::ExecutionContext *exe_ctx = nullptr);
 
   static lldb::AccessType GetAccessTypeFromDWARF(uint32_t dwarf_accessibility);
+
+  /// If \p type_sp is valid, calculate and set its symbol context scope, and
+  /// update the type list for its backing symbol file.
+  ///
+  /// Returns \p type_sp.
+  lldb::TypeSP
+  UpdateSymbolContextScopeForType(const lldb_private::SymbolContext &sc,
+  const DWARFDIE &die, lldb::TypeSP type_sp);
 };
 
 /// Parsed form of all attributes that are relevant for type reconstruction.
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
@@ -11,7 +11,9 @@
 #include "DWARFDIE.h"
 #include "DWARFUnit.h"
 
+#include "lldb/Core/Module.h"
 #include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Target/StackFrame.h"
 
@@ -251,3 +253,37 @@
 }
   }
 }
+
+TypeSP DWARFAST

[Lldb-commits] [lldb] 9c144b3 - [lldb/test] Fix InvalidScriptedThread windows test failure

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-12-06T17:59:29-08:00
New Revision: 9c144b3b0d6a995c2a1eecb53db06f3896e878ff

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

LOG: [lldb/test] Fix InvalidScriptedThread windows test failure

This patch should fix a Windows test failure for the
InvalidScriptedThread test:

https://lab.llvm.org/buildbot/#/builders/83/builds/12571

This refactors the test to stop using python `tempfile` since it's not
supported on Windows and creates a logfile at runtime in the test folder.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py 
b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
index 87ea860f436d..603dc7fa6c12 100644
--- a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -47,6 +47,9 @@ def test_invalid_scripted_register_context(self):
 self.build()
 target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
 self.assertTrue(target, VALID_TARGET)
+log_file = self.getBuildArtifact('thread.log')
+self.runCmd("log enable lldb thread -f " + log_file)
+self.assertTrue(os.path.isfile(log_file))
 
 os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
 def cleanup():
@@ -61,17 +64,18 @@ def cleanup():
 launch_info.SetProcessPluginName("ScriptedProcess")
 
launch_info.SetScriptedProcessClassName("invalid_scripted_process.InvalidScriptedProcess")
 error = lldb.SBError()
-with tempfile.NamedTemporaryFile() as log_file:
-self.runCmd("log enable lldb thread -f " + log_file.name)
-process = target.Launch(launch_info, error)
 
-self.assertTrue(error.Success(), error.GetCString())
-self.assertTrue(process, PROCESS_IS_VALID)
-self.assertEqual(process.GetProcessID(), 666)
-self.assertEqual(process.GetNumThreads(), 0)
+process = target.Launch(launch_info, error)
+
+self.assertTrue(error.Success(), error.GetCString())
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), 666)
+self.assertEqual(process.GetNumThreads(), 0)
+
+with open(log_file, 'r') as f:
+log = f.read()
 
-self.assertIn("Failed to get scripted thread registers 
data.".encode(),
-  log_file.read())
+self.assertIn("Failed to get scripted thread registers data.", log)
 
 @skipIf(archs=no_match(['x86_64']))
 def test_scripted_process_and_scripted_thread(self):



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

[Lldb-commits] [PATCH] D115211: [lldb] Make the LLDB version a first class citizen

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, bulbazord.
Herald added subscribers: tstellar, mgorny.
JDevlieghere requested review of this revision.

Instead of trying to squeeze `GetVersion` into `lldb-private.h`, give it its 
own header and implementation file in its own directory. I called it `Basic`, 
partially because the library was already called `lldbBase` (?) but mostly to 
align with `clang/Basic/Version.h` (and `swift/Basic/Version.h`) which might 
explain the current library name. but I'm pretty flexible as far as naming 
goes, I think `Version/Version.h` would work pretty well too.

Additional benefits of this patch include that we can get rid of the quoting 
macros and that other place of LLDB can read the version number from 
`Version.inc`. A downstream change requiring the latter was the main motivation 
for this patch. I considered adding a few other functions similar to what clang 
offers but decided against it since there would be no existing uses for them.


https://reviews.llvm.org/D115211

Files:
  lldb/include/lldb/Basic/Version.h
  lldb/include/lldb/Basic/Version.inc.in
  lldb/include/lldb/lldb-private.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/Basic/CMakeLists.txt
  lldb/source/Basic/Version.cpp
  lldb/source/CMakeLists.txt
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectVersion.cpp
  lldb/source/Initialization/SystemInitializerCommon.cpp
  lldb/source/lldb.cpp
  lldb/tools/lldb-server/CMakeLists.txt
  lldb/tools/lldb-server/lldb-server.cpp
  lldb/tools/lldb-test/CMakeLists.txt

Index: lldb/tools/lldb-test/CMakeLists.txt
===
--- lldb/tools/lldb-test/CMakeLists.txt
+++ lldb/tools/lldb-test/CMakeLists.txt
@@ -6,7 +6,7 @@
   SystemInitializerTest.cpp
 
   LINK_LIBS
-lldbBase
+lldbBasic
 lldbBreakpoint
 lldbCore
 lldbDataFormatters
Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -7,8 +7,8 @@
 //===--===//
 
 #include "SystemInitializerLLGS.h"
+#include "lldb/Basic/Version.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
-#include "lldb/lldb-private.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
Index: lldb/tools/lldb-server/CMakeLists.txt
===
--- lldb/tools/lldb-server/CMakeLists.txt
+++ lldb/tools/lldb-server/CMakeLists.txt
@@ -46,7 +46,7 @@
 SystemInitializerLLGS.cpp
 
 LINK_LIBS
-  lldbBase
+  lldbBasic
   lldbHost
   lldbInitialization
   ${LLDB_PLUGINS}
Index: lldb/source/Initialization/SystemInitializerCommon.cpp
===
--- lldb/source/Initialization/SystemInitializerCommon.cpp
+++ lldb/source/Initialization/SystemInitializerCommon.cpp
@@ -9,13 +9,13 @@
 #include "lldb/Initialization/SystemInitializerCommon.h"
 
 #include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h"
+#include "lldb/Basic/Version.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/Socket.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/ReproducerProvider.h"
 #include "lldb/Utility/Timer.h"
-#include "lldb/lldb-private.h"
 
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__NetBSD__)
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
Index: lldb/source/Commands/CommandObjectVersion.cpp
===
--- lldb/source/Commands/CommandObjectVersion.cpp
+++ lldb/source/Commands/CommandObjectVersion.cpp
@@ -8,8 +8,8 @@
 
 #include "CommandObjectVersion.h"
 
+#include "lldb/Basic/Version.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
-#include "lldb/lldb-private.h"
 
 using namespace lldb;
 using namespace lldb_private;
Index: lldb/source/Commands/CMakeLists.txt
===
--- lldb/source/Commands/CMakeLists.txt
+++ lldb/source/Commands/CMakeLists.txt
@@ -41,7 +41,7 @@
   CommandOptionsProcessLaunch.cpp
 
   LINK_LIBS
-lldbBase
+lldbBasic
 lldbBreakpoint
 lldbCore
 lldbDataFormatters
Index: lldb/source/CMakeLists.txt
===
--- lldb/source/CMakeLists.txt
+++ lldb/source/CMakeLists.txt
@@ -1,42 +1,6 @@
 include_directories(.)
 
-set(lldbBase_SOURCES
-lldb.cpp
-  )
-
-
-find_first_existing_vc_file("${LLDB_SOURCE_DIR}" lldb_vc)
-
-set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSVersion.inc")
-set(generate_vcs_version_script "${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake")
-
-if(lldb_vc AND LLVM_APPEND_VC_REV)
-  set(lldb_source_dir ${LLDB_SOURCE_DIR})
-e

[Lldb-commits] [PATCH] D115211: [lldb] Make the LLDB version a first class citizen

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I am definitely not sad to see lldbBase go away, but I am wondering if we 
really need a separate library for this functionality (function).

The only reason I can think of is that this function forces a dependency on 
clang. If we put it in say Utility, then all of lldb would depend on a (very 
tiny) piece of clang. I don't think that most of lldb would care about that, 
but this isn't technically correct for lldb-server. It doesn't use clang, and 
only depends on it because of the general tangledness of lldb dependencies. In 
an ideal world one would even be able to build lldb-server without having clang 
checked out. However, given that:

- we are pretty far from an ideal world, and if we ever reach it, we could make 
this small dependency conditional on the existence of clang
- it's really not clear (to me) what else should belong in the Basic library

I am thinking that we could just put this stuff into Utility. It's true that 
the clang version stuff lives in a library called "Basic", but that's because 
this is the name clang chose for its lowest level library. In lldb the lowest 
level library is Utility.

WDYT?


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

https://reviews.llvm.org/D115211

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