[Lldb-commits] [PATCH] D108768: [lldb] DynamicRegisterInfo: fix wrong regnos in Dump()

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

mgorny updated this revision to Diff 369042.
mgorny retitled this revision from "[lldb] [DynamicRegisterInfo] Fix mistaken 
reg type when calculating offsets" to "[lldb] DynamicRegisterInfo: fix wrong 
regnos in Dump()".
mgorny edited the summary of this revision.
mgorny added a comment.

So I've given it some thought and found a simple fix.


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

https://reviews.llvm.org/D108768

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


Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -775,7 +775,12 @@
 if (m_regs[i].value_regs) {
   s.Printf(", value_regs = [ ");
   for (size_t j = 0; m_regs[i].value_regs[j] != LLDB_INVALID_REGNUM; ++j) {
-s.Printf("%s ", m_regs[m_regs[i].value_regs[j]].name);
+const RegisterInfo *value_reg = GetRegisterInfo(
+eRegisterKindProcessPlugin, m_regs[i].value_regs[j]);
+if (value_reg)
+  s.Printf("%s ", value_reg->name);
+else
+  s.Printf("(%d) ", m_regs[i].value_regs[j]);
   }
   s.Printf("]");
 }
@@ -783,7 +788,12 @@
   s.Printf(", invalidate_regs = [ ");
   for (size_t j = 0; m_regs[i].invalidate_regs[j] != LLDB_INVALID_REGNUM;
++j) {
-s.Printf("%s ", m_regs[m_regs[i].invalidate_regs[j]].name);
+const RegisterInfo *invalidate_reg = GetRegisterInfo(
+eRegisterKindProcessPlugin, m_regs[i].invalidate_regs[j]);
+if (invalidate_reg)
+  s.Printf("%s ", invalidate_reg->name);
+else
+  s.Printf("(%d) ", m_regs[i].invalidate_regs[j]);
   }
   s.Printf("]");
 }


Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -775,7 +775,12 @@
 if (m_regs[i].value_regs) {
   s.Printf(", value_regs = [ ");
   for (size_t j = 0; m_regs[i].value_regs[j] != LLDB_INVALID_REGNUM; ++j) {
-s.Printf("%s ", m_regs[m_regs[i].value_regs[j]].name);
+const RegisterInfo *value_reg = GetRegisterInfo(
+eRegisterKindProcessPlugin, m_regs[i].value_regs[j]);
+if (value_reg)
+  s.Printf("%s ", value_reg->name);
+else
+  s.Printf("(%d) ", m_regs[i].value_regs[j]);
   }
   s.Printf("]");
 }
@@ -783,7 +788,12 @@
   s.Printf(", invalidate_regs = [ ");
   for (size_t j = 0; m_regs[i].invalidate_regs[j] != LLDB_INVALID_REGNUM;
++j) {
-s.Printf("%s ", m_regs[m_regs[i].invalidate_regs[j]].name);
+const RegisterInfo *invalidate_reg = GetRegisterInfo(
+eRegisterKindProcessPlugin, m_regs[i].invalidate_regs[j]);
+if (invalidate_reg)
+  s.Printf("%s ", invalidate_reg->name);
+else
+  s.Printf("(%d) ", m_regs[i].invalidate_regs[j]);
   }
   s.Printf("]");
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108768: [lldb] DynamicRegisterInfo: fix wrong regnos in Dump()

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

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

Also updated the comments in `struct RegisterInfo` to indicate what kind of 
numbers go in these arrays.


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

https://reviews.llvm.org/D108768

Files:
  lldb/include/lldb/lldb-private-types.h
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp


Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -775,7 +775,12 @@
 if (m_regs[i].value_regs) {
   s.Printf(", value_regs = [ ");
   for (size_t j = 0; m_regs[i].value_regs[j] != LLDB_INVALID_REGNUM; ++j) {
-s.Printf("%s ", m_regs[m_regs[i].value_regs[j]].name);
+const RegisterInfo *value_reg = GetRegisterInfo(
+eRegisterKindProcessPlugin, m_regs[i].value_regs[j]);
+if (value_reg)
+  s.Printf("%s ", value_reg->name);
+else
+  s.Printf("(%d) ", m_regs[i].value_regs[j]);
   }
   s.Printf("]");
 }
@@ -783,7 +788,12 @@
   s.Printf(", invalidate_regs = [ ");
   for (size_t j = 0; m_regs[i].invalidate_regs[j] != LLDB_INVALID_REGNUM;
++j) {
-s.Printf("%s ", m_regs[m_regs[i].invalidate_regs[j]].name);
+const RegisterInfo *invalidate_reg = GetRegisterInfo(
+eRegisterKindProcessPlugin, m_regs[i].invalidate_regs[j]);
+if (invalidate_reg)
+  s.Printf("%s ", invalidate_reg->name);
+else
+  s.Printf("(%d) ", m_regs[i].invalidate_regs[j]);
   }
   s.Printf("]");
 }
Index: lldb/include/lldb/lldb-private-types.h
===
--- lldb/include/lldb/lldb-private-types.h
+++ lldb/include/lldb/lldb-private-types.h
@@ -51,12 +51,13 @@
   /// List of registers (terminated with LLDB_INVALID_REGNUM). If this value is
   /// not null, all registers in this list will be read first, at which point
   /// the value for this register will be valid. For example, the value list
-  /// for ah would be eax (x86) or rax (x64).
-  uint32_t *value_regs; //
+  /// for ah would be eax (x86) or rax (x64). Register numbers are
+  /// of eRegisterKindProcessPlugin.
+  uint32_t *value_regs;
   /// List of registers (terminated with LLDB_INVALID_REGNUM). If this value is
   /// not null, all registers in this list will be invalidated when the value 
of
   /// this register changes. For example, the invalidate list for eax would be
-  /// rax ax, ah, and al.
+  /// rax ax, ah, and al. Register numbers are of eRegisterKindProcessPlugin.
   uint32_t *invalidate_regs;
   /// A DWARF expression that when evaluated gives the byte size of this
   /// register.


Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -775,7 +775,12 @@
 if (m_regs[i].value_regs) {
   s.Printf(", value_regs = [ ");
   for (size_t j = 0; m_regs[i].value_regs[j] != LLDB_INVALID_REGNUM; ++j) {
-s.Printf("%s ", m_regs[m_regs[i].value_regs[j]].name);
+const RegisterInfo *value_reg = GetRegisterInfo(
+eRegisterKindProcessPlugin, m_regs[i].value_regs[j]);
+if (value_reg)
+  s.Printf("%s ", value_reg->name);
+else
+  s.Printf("(%d) ", m_regs[i].value_regs[j]);
   }
   s.Printf("]");
 }
@@ -783,7 +788,12 @@
   s.Printf(", invalidate_regs = [ ");
   for (size_t j = 0; m_regs[i].invalidate_regs[j] != LLDB_INVALID_REGNUM;
++j) {
-s.Printf("%s ", m_regs[m_regs[i].invalidate_regs[j]].name);
+const RegisterInfo *invalidate_reg = GetRegisterInfo(
+eRegisterKindProcessPlugin, m_regs[i].invalidate_regs[j]);
+if (invalidate_reg)
+  s.Printf("%s ", invalidate_reg->name);
+else
+  s.Printf("(%d) ", m_regs[i].invalidate_regs[j]);
   }
   s.Printf("]");
 }
Index: lldb/include/lldb/lldb-private-types.h
===
--- lldb/include/lldb/lldb-private-types.h
+++ lldb/include/lldb/lldb-private-types.h
@@ -51,12 +51,13 @@
   /// List of registers (terminated with LLDB_INVALID_REGNUM). If this value is
   /// not null, all registers in this list will be read first, at which point
   /// the value for this register will be valid. For example, the value list
-  /// for ah would be eax (x86) or rax (x64).
-  uint32_t *value_regs; //
+  /// for ah would be eax (x86) or rax (x64). Register numbers are
+  /// of eRegisterKindProcessPlugin.
+  uint32_t *value_regs;
   /// List of registers (terminated with LLDB_INVALID_REGNUM). If this value is
   /// not 

[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files

[Andrej Korman via Phabricator via lldb-commits]

Aj0SK added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp) {
+  Status error;

Aj0SK wrote:
> clayborg wrote:
> > This should take in the CoreStyle and only emit what was asked for.
> > 
> > If style is set to "eSaveCoreStackOnly", then grab only the memory regions 
> > for each thread stack pointer and save only those. 
> > 
> > We can't tell from LLDB APIs if a page is dirty, so if we get 
> > "eSaveCoreDirtyOnly", then we will need to save all memory regions that 
> > have write permissions.
> > 
> > If it is either "eSaveCoreFull"  or "eSaveCoreUnspecified" then save 
> > everything.
> I agree that this code should take care of a CoreStyle but it poses a 
> significant problem to this implementation as it was mainly designed to save 
> smaller Minidumps. This solution stores all of the information in memory 
> before dumping them (this eases an implementation quite a bit because of the 
> way how Minidump pointers (offsets) are working). This implementation, on my 
> machine, exhausted memory before the minidump could be written in case of a 
> full memory dump. At this time, we don't plan to reimplement this solution in 
> the way it would allow to store all the data on disc at the time of minidump 
> creation so there are two possible solutions that I see:
> 
> 1. If the type of the CoreStyle is full or dirty, this plugin will return 
> false from the SaveCore function. Then maybe the default CoreStyle for this 
> plugin should be changed to "eSaveCoreStackOnly".
> 
> 2. To the best of my knowledge, it should be theoretically possible to 
> reimplement Dump() method to sort of take special care of a MemoryListStream, 
> dumping also the memory information at the end of the minidump file as I 
> think the memory dump is the most stressful for the memory and otherwise 
> there is no problem with this.
To state my preference, I would rather stick to the first option and landed a 
minimum viable product. The only reason for this is that I have this version 
way better tested and also I am not entirely sure about how minidump deals with 
the whole memory dumps as it can be a little problematic for a big memory 
chunks... Then, I would rather add the necessary changes in the next patch...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108233

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

[Lldb-commits] [PATCH] D108816: [lldb] Return all line entries matchign a line if no column is specified

[Kim-Anh Tran via Phabricator via lldb-commits]

kimanh created this revision.
kimanh added reviewers: mib, JDevlieghere.
kimanh published this revision for review.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Previously, if no column was specified, ResolveSymbolContext would take
the first match returned by FindLineEntryIndexByFileIndex, and reuse it
to find subsequent exact matches. With the introduction of columns, columns
are now considered when matching the line entries.

This leads to a problem if one wants to get all existing line entries
that match that line, since now the column is also used for the exact match.
This way, all line entries are filtered out that have a different
column number, but the same line number.

This patch changes that by ignoring the column information of the first match
if the original request of ResolveSymbolContext was also ignoring it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108816

Files:
  lldb/source/Symbol/CompileUnit.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp

Index: lldb/unittests/Symbol/TestLineEntry.cpp
===
--- lldb/unittests/Symbol/TestLineEntry.cpp
+++ lldb/unittests/Symbol/TestLineEntry.cpp
@@ -38,7 +38,8 @@
   void SetUp() override;
 
 protected:
-  llvm::Expected GetLineEntryForLine(uint32_t line);
+  llvm::Expected
+  GetLineEntriesForLine(uint32_t line, llvm::Optional column);
   llvm::Optional m_file;
   ModuleSP m_module_sp;
 };
@@ -50,8 +51,9 @@
   m_module_sp = std::make_shared(m_file->moduleSpec());
 }
 
-llvm::Expected LineEntryTest::GetLineEntryForLine(uint32_t line) {
   // TODO: Handle SourceLocationSpec column information
+llvm::Expected LineEntryTest::GetLineEntriesForLine(
+uint32_t line, llvm::Optional column = llvm::None) {
   SymbolContextList sc_comp_units;
   SymbolContextList sc_line_entries;
   FileSpec file_spec("inlined-functions.cpp");
@@ -62,7 +64,7 @@
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No comp unit found on the test object.");
 
-  SourceLocationSpec location_spec(file_spec, line, /*column=*/llvm::None,
+  SourceLocationSpec location_spec(file_spec, line, column,
/*check_inlines=*/true,
/*exact_match=*/true);
 
@@ -71,33 +73,53 @@
   if (sc_line_entries.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No line entry found on the test object.");
-  return sc_line_entries[0].line_entry;
+  return sc_line_entries;
+}
+
+// This tests if we can get all line entries that match the passed line, if
+// no column is specified.
+TEST_F(LineEntryTest, GetAllExactLineMatchesWithoutColumn) {
+  auto sc_line_entries = GetLineEntriesForLine(12);
+  ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded());
+  ASSERT_EQ(sc_line_entries->NumLineEntriesWithLine(12), 6u);
+}
+
+// This tests if we can get exact line and column matches.
+TEST_F(LineEntryTest, GetAllExactLineColumnMatches) {
+  auto sc_line_entries = GetLineEntriesForLine(12, 39);
+  ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded());
+  ASSERT_EQ(sc_line_entries->NumLineEntriesWithLine(12), 1u);
+  auto line_entry = sc_line_entries.get()[0].line_entry;
+  ASSERT_EQ(line_entry.column, 39);
 }
 
 TEST_F(LineEntryTest, GetSameLineContiguousAddressRangeNoInlines) {
-  auto line_entry = GetLineEntryForLine(18);
-  ASSERT_THAT_EXPECTED(line_entry, llvm::Succeeded());
+  auto sc_line_entries = GetLineEntriesForLine(18);
+  ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded());
+  auto line_entry = sc_line_entries.get()[0].line_entry;
   bool include_inlined_functions = false;
   auto range =
-  line_entry->GetSameLineContiguousAddressRange(include_inlined_functions);
+  line_entry.GetSameLineContiguousAddressRange(include_inlined_functions);
   ASSERT_EQ(range.GetByteSize(), (uint64_t)0x24);
 }
 
 TEST_F(LineEntryTest, GetSameLineContiguousAddressRangeOneInline) {
-  auto line_entry = GetLineEntryForLine(18);
-  ASSERT_THAT_EXPECTED(line_entry, llvm::Succeeded());
+  auto sc_line_entries = GetLineEntriesForLine(18);
+  ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded());
+  auto line_entry = sc_line_entries.get()[0].line_entry;
   bool include_inlined_functions = true;
   auto range =
-  line_entry->GetSameLineContiguousAddressRange(include_inlined_functions);
+  line_entry.GetSameLineContiguousAddressRange(include_inlined_functions);
   ASSERT_EQ(range.GetByteSize(), (uint64_t)0x49);
 }
 
 TEST_F(LineEntryTest, GetSameLineContiguousAddressRangeNestedInline) {
-  auto line_entry = GetLineEntryForLine(12);
-  ASSERT_THAT_EXPECTED(line_entry, llvm::Succeeded());
+  auto sc_line_entries = GetLineEntriesForLine(12);
+  ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded());
+  auto line_entry = sc_line_entries.get()[0].line_entry;
   bool include_inlined_functions = true;
   auto 

[Lldb-commits] [PATCH] D107456: [lldb] Support .debug_rnglists.dwo sections in dwp file

[Kim-Anh Tran via Phabricator via lldb-commits]

kimanh added a comment.

Friendly ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107456

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

[Lldb-commits] [PATCH] D108817: [LLDB] Fix 'std::out_of_range' crashing bug when file name completion with using file path.

[Hiroki Imai via Phabricator via lldb-commits]

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

When I run a lldb command  that uses filename completion, if I enter a string 
that is not only a filename but also a string with a non-file name string 
added, such as a relative path, it will crash as soon as I press the [Tab] key.
For example, debugging an executable named hello that is compiled from a file 
named hello.c, and I’ll put a breakpoint on line 3 of hello.c.

  $ lldb ./hello
  (lldb) breakpoint set --file hello.c --line 3

This is not a problem, but if I set  "--file ./hello."  and then press [Tab] 
key to complete file name, lldb crashes.

  $ lldb ./hello
  (lldb) breakpoint set --file ./hello.terminate called after throwing an 
instance of 'std::out_of_range'
what():  basic_string::substr: __pos (which is 8) > this->size() (which is 
7)
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace.
  Stack dump:
  0.Program arguments: lldb-12 ./hello
  Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH 
or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
  
/lib/x86_64-linux-gnu/libLLVM-12.so.1(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamEi+0x23)[0x7f172281de53]
  
/lib/x86_64-linux-gnu/libLLVM-12.so.1(_ZN4llvm3sys17RunSignalHandlersEv+0x50)[0x7f172281c170]
  /lib/x86_64-linux-gnu/libLLVM-12.so.1(+0xbd94bf)[0x7f172281e4bf]
  /lib/x86_64-linux-gnu/libpthread.so.0(+0x153c0)[0x7f172b08a3c0]
  /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xcb)[0x7f172174b18b]
  /lib/x86_64-linux-gnu/libc.so.6(abort+0x12b)[0x7f172172a859]
  /lib/x86_64-linux-gnu/libstdc++.so.6(+0x9e911)[0x7f1721b01911]
  /lib/x86_64-linux-gnu/libstdc++.so.6(+0xaa38c)[0x7f1721b0d38c]
  /lib/x86_64-linux-gnu/libstdc++.so.6(+0xaa3f7)[0x7f1721b0d3f7]
  /lib/x86_64-linux-gnu/libstdc++.so.6(+0xaa6a9)[0x7f1721b0d6a9]
  /lib/x86_64-linux-gnu/libstdc++.so.6(+0xa13ab)[0x7f1721b043ab]
  /lib/x86_64-linux-gnu/liblldb-12.so.1(+0x63cbb3)[0x7f172a67bbb3]
  /lib/x86_64-linux-gnu/liblldb-12.so.1(+0x63fa59)[0x7f172a67ea59]
  /lib/x86_64-linux-gnu/libedit.so.2(el_wgets+0x102)[0x7f1721112d42]
  /lib/x86_64-linux-gnu/liblldb-12.so.1(+0x63ee36)[0x7f172a67de36]
  /lib/x86_64-linux-gnu/liblldb-12.so.1(+0x5b9a5b)[0x7f172a5f8a5b]
  /lib/x86_64-linux-gnu/liblldb-12.so.1(+0x5babfe)[0x7f172a5f9bfe]
  /lib/x86_64-linux-gnu/liblldb-12.so.1(+0x59f254)[0x7f172a5de254]
  /lib/x86_64-linux-gnu/liblldb-12.so.1(+0x66446d)[0x7f172a6a346d]
  
/lib/x86_64-linux-gnu/liblldb-12.so.1(_ZN4lldb10SBDebugger21RunCommandInterpreterEbb+0xe9)[0x7f172a2be949]
  lldb-12[0x406e5a]
  lldb-12[0x408826]
  /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0x7f172172c0b3]
  lldb-12[0x40435e]
  Aborted (core dumped)

The crash was caused because substr() (in lldb/source/Host/common/Editline.cpp) 
cut out string which size is user's input string from the completed string.

I modified the code that erase the user's intput string from current line then 
add the completion string.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108817

Files:
  lldb/source/Host/common/Editline.cpp


Index: lldb/source/Host/common/Editline.cpp
===
--- lldb/source/Host/common/Editline.cpp
+++ lldb/source/Host/common/Editline.cpp
@@ -1006,11 +1006,11 @@
 switch (completion.GetMode()) {
 case CompletionMode::Normal: {
   std::string to_add = completion.GetCompletion();
-  to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
   // Terminate the current argument with a quote if it started with a 
quote.
   if (!request.GetParsedLine().empty() && 
request.GetParsedArg().IsQuoted())
 to_add.push_back(request.GetParsedArg().GetQuoteChar());
   to_add.push_back(' ');
+ el_deletestr(m_editline, request.GetCursorArgumentPrefix().size());
   el_insertstr(m_editline, to_add.c_str());
   // Clear all the autosuggestion parts if the only single space can be 
completed.
   if (to_add == " ")


Index: lldb/source/Host/common/Editline.cpp
===
--- lldb/source/Host/common/Editline.cpp
+++ lldb/source/Host/common/Editline.cpp
@@ -1006,11 +1006,11 @@
 switch (completion.GetMode()) {
 case CompletionMode::Normal: {
   std::string to_add = completion.GetCompletion();
-  to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
   // Terminate the current argument with a quote if it started with a quote.
   if (!request.GetParsedLine().empty() && request.GetParsedArg().IsQuoted())
 to_add.push_back(request.GetParsedArg().GetQuoteChar());
   to_add.push_back(' ');
+	  el_deletestr(m_editline, request.GetCursorArgumentPrefix().size());
   el_insertstr(m_editline, to_add.c_str());
   // Clear all the autosu

[Lldb-commits] [PATCH] D108807: [LLDB][Docs] Convert links.md & lldb-for-gdb-users.txt file to respective .rst files

[Shivam Gupta via Phabricator via lldb-commits]

xgupta created this revision.
xgupta updated this revision to Diff 369063.
xgupta added a comment.
xgupta updated this revision to Diff 369099.
xgupta retitled this revision from "[LLDB][Docs] Convert some .txt files to 
.rst" to "[LLDB][Docs] Convert links.md & lldb-for-gdb-users.txt file to 
respective .rst files".
xgupta edited the summary of this revision.
xgupta published this revision for review.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Convert lldb-for-gdb-users.txt


xgupta added a comment.

move lldb-for-gdb-users.rst to use/ folder


Update links.md & lldb-for-gdb-users.txt file to respective .rst  fles for 
consistency as most of the documentation is written in reStructuredText format.

Signed-off-by: Shivam Gupta 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108807

Files:
  lldb/docs/lldb-for-gdb-users.txt
  lldb/docs/use/links.md
  lldb/docs/use/links.rst
  lldb/docs/use/lldb-for-gdb-users.rst

Index: lldb/docs/use/lldb-for-gdb-users.rst
===
--- lldb/docs/use/lldb-for-gdb-users.rst
+++ lldb/docs/use/lldb-for-gdb-users.rst
@@ -1,8 +1,14 @@
+LLDB for GDB users
+==
+
 Here's a short precis of how to run lldb if you are familiar with the
 gdb command set:
 
+.. contents::
+   :local:
 
-1) LLDB Command Structure:
+LLDB Command Structure:
+---
 
 First some details on lldb command structure to help orient you...
 
@@ -10,6 +16,8 @@
 the lldb command syntax fairly structured.  The commands are all of the
 form
 
+::
+
   [-options [option-value]] [argument [argument...]]
 
 The command line parsing is done before command execution, so it is
@@ -27,7 +35,9 @@
 arguments are the arguments you are passing to the program.  So if you wanted
 to pass an argument that contained a "-" you would have to do:
 
-(lldb) process launch -- -program_arg value
+::
+
+> (lldb) process launch -- -program_arg value
 
 We also tried to reduce the number of special purpose argument
 parsers, which sometimes forces the user to be a little more explicit
@@ -35,11 +45,15 @@
 this is the breakpoint command.  In gdb, to set a breakpoint, you
 would just say:
 
-(gdb) break foo.c:12
+::
+
+> (gdb) break foo.c:12
 
 or
 
-(gdb) break foo
+::
+
+> (gdb) break foo
 
 if foo is a function.  As time went on, the parser that tells foo.c:12
 from foo from foo.c::foo (which means the function foo in the file
@@ -48,38 +62,52 @@
 you want to break on.  The lldb commands are more verbose but also precise.  
 So you say:
 
-(lldb) breakpoint set -f foo.c -l 12
+::
+
+> (lldb) breakpoint set -f foo.c -l 12
 
 to set a file & line breakpoint.  To set a breakpoint on a function
 by name, you do:
 
-(lldb) breakpoint set -n foo
+::
+
+> (lldb) breakpoint set -n foo
 
 This can allow us to be more expressive, so you can say:
 
-(lldb) breakpoint set -M foo
+::
+
+> (lldb) breakpoint set -M foo
 
 to break on all C++ methods named foo, or:
 
-(lldb) breakpoint set -S alignLeftEdges:
+::
+
+> (lldb) breakpoint set -S alignLeftEdges:
 
 to set a breakpoint on all ObjC selectors called alignLeftEdges:.  It
 also makes it easy to compose specifications, like:
 
-(lldb) breakpoint set -s foo.dylib -n foo
+::
+
+> (lldb) breakpoint set -s foo.dylib -n foo
 
 for all functions called foo in the shared library foo.dylib.  Suggestions
 on more interesting primitives of this sort are also very welcome.
 
 So for instance:
 
-(lldb) breakpoint set -n "-[SKTGraphicView alignLeftEdges:]"
+::
+
+> (lldb) breakpoint set -n "-[SKTGraphicView alignLeftEdges:]"
 
 Just like gdb, the lldb command interpreter does a shortest unique
 string match on command names, so the previous command can also be
 typed:
 
-(lldb) b s -n "-[SKTGraphicView alignLeftEdges:]"
+::
+
+> (lldb) b s -n "-[SKTGraphicView alignLeftEdges:]"
 
 lldb also supports command completion for source file names, symbol
 names, file names, etc. Completion is initiated by a hitting a .
@@ -97,12 +125,16 @@
 Finally, there is a mechanism to construct aliases for commonly used
 commands.  So for instance if you get annoyed typing
 
-(lldb) b s -f foo.c -l 12
+::
+
+> (lldb) b s -f foo.c -l 12
 
 you can do:
 
-(lldb) command alias bfl breakpoint set -f %1 -l %2
-(lldb) bfl foo.c 12
+::
+
+> (lldb) command alias bfl breakpoint set -f %1 -l %2
+> (lldb) bfl foo.c 12
 
 We have added a few aliases for commonly used commands (e.g. "step",
 "next" and "continue") but we haven't tried to be exhaustive because
@@ -126,40 +158,48 @@
 with the "script" command.  
 
 
-
-2) A typical session:
-
+A typical session:
+--
 
 a) Setting the program to debug:
+
 
 
 As with gdb, you can start lldb and specify the file you wish to debug
 on the command line:
 
-$ lldb /Projects/Sketch/build/Debug/Sketch.app
-Current executable set to '/Projects/Sketch/build/Debug/Sketch.app' (x86_64).
+::
+
+> lldb /Projects/Sketch/bu

[Lldb-commits] [PATCH] D108812: [LLDB][Docs] Renew best-practices.txt

[Shivam Gupta via Phabricator via lldb-commits]

xgupta created this revision.
xgupta updated this revision to Diff 369103.
xgupta added a comment.
xgupta published this revision for review.
xgupta added reviewers: teemperor, mgorny, JDevlieghere.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Add ToC


This file contain some old reference to files those are now either renamed or 
replaced.
Also this .txt file didn't generate to html during the sphnix documentation 
build so I reformat it to .rst file.

Signed-off-by: Shivam Gupta 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108812

Files:
  lldb/docs/testsuite/best-practices.rst
  lldb/docs/testsuite/best-practices.txt

Index: lldb/docs/testsuite/best-practices.txt
===
--- lldb/docs/testsuite/best-practices.txt
+++ /dev/null
@@ -1,93 +0,0 @@
-This document attempts to point out some best practices that prove to be helpful
-when building new test cases in the tot/test directory.  Everyone is welcomed to
-add/modify contents into this file.
-
-o Do not use hard-coded line numbers in your test case.  Instead, try to tag the
-  line with some distinguishing pattern, and use the function line_number()
-  defined in lldbtest.py which takes filename and string_to_match as arguments
-  and returns the line number.
-
-As an example, take a look at test/breakpoint_conditions/main.c which has these
-two lines:
-
-return c(val); // Find the line number of c's parent call here.
-
-and
-
-return val + 3; // Find the line number of function "c" here.
-
-The Python test case TestBreakpointConditions.py uses the comment strings to
-find the line numbers during setUp(self) and use them later on to verify that
-the correct breakpoint is being stopped on and that its parent frame also has
-the correct line number as intended through the breakpoint condition.
-
-o Take advantage of the unittest framework's decorator features to properly
-  mark your test class or method for platform-specific tests.
-
-As an example, take a look at test/forward/TestForwardDeclaration.py which has
-these lines:
-
-@unittest2.skipUnless(sys.platform.startswith("darwin"), "requires Darwin")
-def test_with_dsym_and_run_command(self):
-"""Display *bar_ptr when stopped on a function with forward declaration of struct bar."""
-self.buildDsym()
-self.forward_declaration()
-
-This tells the test harness that unless we are running "darwin", the test should
-be skipped.  This is because we are asking to build the binaries with dsym debug
-info, which is only available on the darwin platforms.
-
-o Cleanup after yourself.  A classic example of this can be found in test/types/
-  TestFloatTypes.py:
-
-def test_float_types_with_dsym(self):
-"""Test that float-type variables are displayed correctly."""
-d = {'CXX_SOURCES': 'float.cpp'}
-self.buildDsym(dictionary=d)
-self.setTearDownCleanup(dictionary=d)
-self.float_type()
-
-...
-
-def test_double_type_with_dsym(self):
-"""Test that double-type variables are displayed correctly."""
-d = {'CXX_SOURCES': 'double.cpp'}
-self.buildDsym(dictionary=d)
-self.setTearDownCleanup(dictionary=d)
-self.double_type()
-
-This tests different data structures composed of float types to verify that what
-the debugger prints out matches what the compiler does for different variables
-of these types.  We're using a dictionary to pass the build parameters to the
-build system.  After a particular test instance is done, it is a good idea to
-clean up the files built.  This eliminates the chance that some leftover files
-can interfere with the build phase for the next test instance and render it
-invalid.
-
-TestBase.setTearDownCleanup(self, dictionary) defined in lldbtest.py is created
-to cope with this use case by taking the same build parameters in order to do
-the cleanup when we are finished with a test instance, during
-TestBase.tearDown(self).
-
-o Class-wise cleanup after yourself.
-
-TestBase.tearDownClass(cls) provides a mechanism to invoke the platform-specific
-cleanup after finishing with a test class. A test class can have more than one
-test methods, so the tearDownClass(cls) method gets run after all the test
-methods have been executed by the test harness.
-
-The default cleanup action performed by the plugins/darwin.py module invokes the
-"make clean" os command.
-
-If this default cleanup is not enough, individual class can provide an extra
-cleanup hook with a class method named classCleanup , for example,
-in test/breakpoint_command/TestBreakpointCommand.py:
-
-@classmethod
-def classCleanup(cls):
-system(["/bin/sh", "-c", "rm -f output.txt"])
-
-The 'output.txt' file gets generated during the test run, so it makes sense to
-explicitly spell out the action in the same TestBreakpointCommand.py file to do
-the cleanup instead of artificially adding i

[Lldb-commits] [PATCH] D108812: [LLDB][Docs] Renew best-practices.txt

[Shivam Gupta via Phabricator via lldb-commits]

xgupta added a comment.

I later realize that we already have https://lldb.llvm.org/resources/test.html 
which contains some similar instructions to write testcases. So not sure we 
want to have this document.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108812

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

[Lldb-commits] [PATCH] D108816: [lldb] Return all line entries matchign a line if no column is specified

[Med Ismail Bennani via Phabricator via lldb-commits]

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

LGTM. Thanks for fixing that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108816

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

[Lldb-commits] [lldb] 602497d - [trace] [intel pt] Create a "process trace save" command

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-08-27T09:34:01-07:00
New Revision: 602497d672ca66f3f0e59adca35f00199f266810

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

LOG: [trace] [intel pt] Create a "process trace save" command

added new command "process trace save -d ".
-it saves a JSON file as /trace.json, with the main properties of 
the trace session.
-it saves binary Intel-pt trace as /thread_id.trace; each file saves 
each thread.
-it saves modules to the directory /modules .
-it only works for live process and it only support Intel-pt right now.

Example:
```
b main
run
process trace start
n
process trace save -d /tmp/mytrace
```
A file named trace.json and xxx.trace should be generated in /tmp/mytrace. To 
load the trace that was just saved:
```
trace load /tmp/mytrace
thread trace dump instructions
```
You should see the instructions of the trace got printed.

To run a test:
```
cd ~/llvm-sand/build/Release/fbcode-x86_64/toolchain
ninja lldb-dotest
./bin/lldb-dotest -p TestTraceSave
```

Reviewed By: wallace

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

Added: 
lldb/source/Plugins/Trace/common/TraceJSONStructs.cpp
lldb/source/Plugins/Trace/common/TraceJSONStructs.h
lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp
lldb/source/Plugins/Trace/common/TraceSessionSaver.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.h
lldb/test/API/commands/trace/TestTraceSave.py

Modified: 
lldb/include/lldb/Target/Trace.h
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/Options.td
lldb/source/Plugins/Trace/common/CMakeLists.txt
lldb/source/Plugins/Trace/common/TraceSessionFileParser.cpp
lldb/source/Plugins/Trace/common/TraceSessionFileParser.h
lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h

Removed: 




diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index f5654988b201..643b761cdb89 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -20,6 +20,7 @@
 #include "lldb/Utility/TraceGDBRemotePackets.h"
 #include "lldb/Utility/UnimplementedError.h"
 #include "lldb/lldb-private.h"
+#include "lldb/lldb-types.h"
 
 namespace lldb_private {
 
@@ -55,6 +56,22 @@ class Trace : public PluginInterface,
   /// A stream object to dump the information to.
   virtual void Dump(Stream *s) const = 0;
 
+  /// Save the trace of a live process to the specified directory, which
+  /// will be created if needed.
+  /// This will also create a a file \a /trace.json with the main
+  /// properties of the trace session, along with others files which contain
+  /// the actual trace data. The trace.json file can be used later as input
+  /// for the "trace load" command to load the trace in LLDB.
+  /// The process being trace is not a live process, return an error.
+  ///
+  /// \param[in] directory
+  ///   The directory where the trace files will be saved.
+  ///
+  /// \return
+  ///   \a llvm::success if the operation was successful, or an \a llvm::Error
+  ///   otherwise.
+  virtual llvm::Error SaveLiveTraceToDisk(FileSpec directory) = 0;
+
   /// Find a trace plug-in using JSON data.
   ///
   /// When loading trace data from disk, the information for the trace data
@@ -156,12 +173,12 @@ class Trace : public PluginInterface,
 
   /// Check if a thread is currently traced by this object.
   ///
-  /// \param[in] thread
-  /// The thread in question.
+  /// \param[in] tid
+  /// The id of the thread in question.
   ///
   /// \return
   /// \b true if the thread is traced by this instance, \b false otherwise.
-  virtual bool IsTraced(const Thread &thread) = 0;
+  virtual bool IsTraced(lldb::tid_t tid) = 0;
 
   /// \return
   /// A description of the parameters to use for the \a Trace::Start 
method.

diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp 
b/lldb/source/Commands/CommandObjectProcess.cpp
index c65b661f9cfc..bb6220a53d4e 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -1644,6 +1644,80 @@ class CommandObjectProcessTraceStart : public 
CommandObjectTraceProxy {
   }
 };
 
+// CommandObjectProcessTraceSave
+#define LLDB_OPTIONS_process_trace_s

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

[Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG602497d672ca: [trace] [intel pt] Create a "process 
trace save" command (authored by Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107669

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/common/CMakeLists.txt
  lldb/source/Plugins/Trace/common/TraceJSONStructs.cpp
  lldb/source/Plugins/Trace/common/TraceJSONStructs.h
  lldb/source/Plugins/Trace/common/TraceSessionFileParser.cpp
  lldb/source/Plugins/Trace/common/TraceSessionFileParser.h
  lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp
  lldb/source/Plugins/Trace/common/TraceSessionSaver.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.h
  lldb/test/API/commands/trace/TestTraceSave.py

Index: lldb/test/API/commands/trace/TestTraceSave.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceSave.py
@@ -0,0 +1,97 @@
+import lldb
+from intelpt_testcase import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceSave(TraceIntelPTTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def testErrorMessages(self):
+# We first check the output when there are no targets
+self.expect("process trace save",
+substrs=["error: invalid target, create a target using the 'target create' command"],
+error=True)
+
+# We now check the output when there's a non-running target
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+
+self.expect("process trace save",
+substrs=["error: invalid process"],
+error=True)
+
+# Now we check the output when there's a running target without a trace
+self.expect("b main")
+self.expect("run")
+
+self.expect("process trace save",
+substrs=["error: Process is not being traced"],
+error=True)
+
+def testSaveToInvalidDir(self):
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+self.expect("r")
+self.expect("thread trace start")
+self.expect("n")
+
+# Check the output when saving without providing the directory argument
+self.expect("process trace save -d",
+substrs=["error: last option requires an argument"],
+error=True)
+
+# Check the output when saving to an invalid directory
+self.expect("process trace save -d /",
+substrs=["error: couldn't write to the file"],
+error=True)
+
+def testSaveWhenNotLiveTrace(self):
+self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+substrs=["intel-pt"])
+
+# Check the output when not doing live tracing
+self.expect("process trace save -d " +
+os.path.join(self.getBuildDir(), "intelpt-trace", "trace_not_live_dir"),
+substrs=["error: Saving a trace requires a live process."],
+error=True)
+
+
+def testSaveTrace(self):
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+self.expect("r")
+self.expect("thread trace start")
+self.expect("b 7")
+
+ci = self.dbg.GetCommandInterpreter()
+res = lldb.SBCommandReturnObject()
+
+ci.HandleCommand("thread trace dump instructions -c 10 --forwards", res)
+self.assertEqual(res.Succeeded(), True)
+first_ten_instructions = res.GetOutput()
+
+ci.HandleCommand("thread trace dump instructions -c 10", res)
+self.assertEqual(res.Succeeded(), True)
+last_ten_instructions = res.GetOutput()
+
+# Now, save the trace to 
+self.expect("process trace save -d " +
+os.path.join(

[Lldb-commits] [PATCH] D108831: [lldb] [gdb-remote] Add x86_64 pseudo-registers when using gdbserver

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

mgorny created this revision.
mgorny added reviewers: krytarowski, jasonmolenda, JDevlieghere, labath, emaste.
Herald added a subscriber: pengfei.
mgorny requested review of this revision.

Add a pre- and post-finalize hooks to GDBRemoteRegisterContext that add
missing pseudo-register on x86_64.  This is necessary since -- unlike
lldb-server -- gdbserver does not include pseudo-registers in its target
definitions.  Both hooks are implemented in a separate file for x86_64,
and can be easily defined for other architectures.

The pre-finalize hook checks whether appropriate x86_64 pseudo-registers
are defined (e.g. the EAX, AX, AH, AL, MMi registers, etc.).  If they
are not, it defines them referring to the full registers via value_regs
field.

Then, the already-present logic in the finalize function sets
appropriate offsets based on the value_regs linking.  The exception to
that are the AH..DH registers which carry the offsets of AL..DL at this
point due to the limitation of value_regs logic.

Finally, the post-finalize hook takes care of adjusting the offsets
of AH..DH registers.  While technically this could be resolved via some
generic logic in RegisterInfo, given it applies only to a 4 very
specific registers, it seemed cleaner not to require global LLDB changes
and instead perform the fixup in a hook.


https://reviews.llvm.org/D108831

Files:
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86_64.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -9,8 +9,8 @@
 
 @skipIfXmlSupportMissing
 @skipIfRemote
-def test_x86_64_vec_regs(self):
-"""Test rendering of x86_64 vector registers from gdbserver."""
+def test_x86_64_regs(self):
+"""Test grabbing various x86_64 registers from gdbserver."""
 class MyResponder(MockGDBServerResponder):
 def qXferRead(self, obj, annex, offset, length):
 if annex == "target.xml":
@@ -112,8 +112,6 @@
["rdi = 0x3837363534333231"])
 self.match("register read fp",
["rbp = 0x4847464544434241"])
-self.match("register read sp",
-   ["rsp = 0x5857565554535251"])
 self.match("register read arg5",
["r8 = 0x6867666564636261"])
 self.match("register read arg6",
@@ -123,6 +121,26 @@
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read ecx",
+   ["ecx = 0x04030201"])
+self.match("register read cx",
+   ["cx = 0x0201"])
+self.match("register read ch",
+   ["ch = 0x02"])
+self.match("register read cl",
+   ["cl = 0x01"])
+self.match("register read r8d",
+   ["r8d = 0x64636261"])
+self.match("register read r8w",
+   ["r8w = 0x6261"])
+self.match("register read r8l",
+   ["r8l = 0x61"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86_64.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86_64.cpp
@@ -0,0 +1,195 @@
+//===-- GDBRemoteRegisterContext.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 "GDBRemoteRegisterContext.h"
+
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::process_gdb_remote;
+
+enum PartialGPRRegKind {
+  eRegKind32,
+  eRegKind16,
+  eRegKind8h,
+  eRegKind8l,
+
+  eRegKindCount
+};
+
+struct PartialGPRReg {
+  const char *name;
+
+  // extra storage used for pointers
+  uint32_t value_regs[2];
+  uint32_t invalidate

[Lldb-commits] [PATCH] D108831: [lldb] [gdb-remote] Add x86_64 pseudo-registers when using gdbserver

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

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

Remove unnecessary include.


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

https://reviews.llvm.org/D108831

Files:
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86_64.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -9,8 +9,8 @@
 
 @skipIfXmlSupportMissing
 @skipIfRemote
-def test_x86_64_vec_regs(self):
-"""Test rendering of x86_64 vector registers from gdbserver."""
+def test_x86_64_regs(self):
+"""Test grabbing various x86_64 registers from gdbserver."""
 class MyResponder(MockGDBServerResponder):
 def qXferRead(self, obj, annex, offset, length):
 if annex == "target.xml":
@@ -112,8 +112,6 @@
["rdi = 0x3837363534333231"])
 self.match("register read fp",
["rbp = 0x4847464544434241"])
-self.match("register read sp",
-   ["rsp = 0x5857565554535251"])
 self.match("register read arg5",
["r8 = 0x6867666564636261"])
 self.match("register read arg6",
@@ -123,6 +121,26 @@
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read ecx",
+   ["ecx = 0x04030201"])
+self.match("register read cx",
+   ["cx = 0x0201"])
+self.match("register read ch",
+   ["ch = 0x02"])
+self.match("register read cl",
+   ["cl = 0x01"])
+self.match("register read r8d",
+   ["r8d = 0x64636261"])
+self.match("register read r8w",
+   ["r8w = 0x6261"])
+self.match("register read r8l",
+   ["r8l = 0x61"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86_64.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86_64.cpp
@@ -0,0 +1,193 @@
+//===-- GDBRemoteRegisterContext.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 "GDBRemoteRegisterContext.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::process_gdb_remote;
+
+enum PartialGPRRegKind {
+  eRegKind32,
+  eRegKind16,
+  eRegKind8h,
+  eRegKind8l,
+
+  eRegKindCount
+};
+
+struct PartialGPRReg {
+  const char *name;
+
+  // extra storage used for pointers
+  uint32_t value_regs[2];
+  uint32_t invalidate_regs[eRegKindCount];
+};
+
+struct PartialGPRRegSet {
+  const char *reg64;
+  PartialGPRReg r[eRegKindCount];
+};
+
+static std::array partial_gpr_regs = {{
+{"rax", {{"eax"}, {"ax"}, {"ah"}, {"al"}}},
+{"rbx", {{"ebx"}, {"bx"}, {"bh"}, {"bl"}}},
+{"rcx", {{"ecx"}, {"cx"}, {"ch"}, {"cl"}}},
+{"rdx", {{"edx"}, {"dx"}, {"dh"}, {"dl"}}},
+{"rdi", {{"edi"}, {"di"}, {nullptr}, {"dil"}}},
+{"rsi", {{"esi"}, {"si"}, {nullptr}, {"sil"}}},
+{"rbp", {{"ebp"}, {"bp"}, {nullptr}, {"bpl"}}},
+{"rsp", {{"esp"}, {"sp"}, {nullptr}, {"spl"}}},
+{"r8", {{"r8d"}, {"r8w"}, {nullptr}, {"r8l"}}},
+{"r9", {{"r9d"}, {"r9w"}, {nullptr}, {"r9l"}}},
+{"r10", {{"r10d"}, {"r10w"}, {nullptr}, {"r10l"}}},
+{"r11", {{"r11d"}, {"r11w"}, {nullptr}, {"r11l"}}},
+{"r12", {{"r12d"}, {"r12w"}, {nullptr}, {"r12l"}}},
+{"r13", {{"r13d"}, {"r13w"}, {nullptr}, {"r13l"}}},
+{"r14", {{"r14d"}, {"r14w"}, {nullptr}, {"r14l"}}},
+{"r15", {{"r15d"}, {"r15w"}, {nullptr}, {"r15l"}}},
+}};
+
+static uint32_t partial_gpr_sizes[eRegKindCount] = {4, 2, 1, 1};
+
+struct MMReg {
+  const char *streg;
+  const char *name;
+
+  // extra storage used for pointers
+  uint32_t value_regs[2];
+};
+
+st

[Lldb-commits] [PATCH] D100206: [lldb] [llgs client] Support minimal fork/vfork handling

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

mgorny added a comment.

@JDevlieghere, ping.


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

https://reviews.llvm.org/D100206

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

[Lldb-commits] [PATCH] D107664: [lldb] [Commands] Remove 'append' from 'platform file open' mode

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

mgorny added a comment.

@jasonmolenda, ping.


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

https://reviews.llvm.org/D107664

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

[Lldb-commits] [PATCH] D107780: [lldb] [gdb-remote] Implement fallback to vFile:stat for GetFileSize()

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

mgorny added a comment.

@jasonmolenda, ping.


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

https://reviews.llvm.org/D107780

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

[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files

[Greg Clayton via Phabricator via lldb-commits]

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

No worries on only saving out the minimal for now, but just make sure that you 
error out if CoreStyle is anything but "eSaveCoreUnspecified" or 
"eSaveCoreStackOnly".




Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp) {
+  Status error;

Aj0SK wrote:
> Aj0SK wrote:
> > clayborg wrote:
> > > This should take in the CoreStyle and only emit what was asked for.
> > > 
> > > If style is set to "eSaveCoreStackOnly", then grab only the memory 
> > > regions for each thread stack pointer and save only those. 
> > > 
> > > We can't tell from LLDB APIs if a page is dirty, so if we get 
> > > "eSaveCoreDirtyOnly", then we will need to save all memory regions that 
> > > have write permissions.
> > > 
> > > If it is either "eSaveCoreFull"  or "eSaveCoreUnspecified" then save 
> > > everything.
> > I agree that this code should take care of a CoreStyle but it poses a 
> > significant problem to this implementation as it was mainly designed to 
> > save smaller Minidumps. This solution stores all of the information in 
> > memory before dumping them (this eases an implementation quite a bit 
> > because of the way how Minidump pointers (offsets) are working). This 
> > implementation, on my machine, exhausted memory before the minidump could 
> > be written in case of a full memory dump. At this time, we don't plan to 
> > reimplement this solution in the way it would allow to store all the data 
> > on disc at the time of minidump creation so there are two possible 
> > solutions that I see:
> > 
> > 1. If the type of the CoreStyle is full or dirty, this plugin will return 
> > false from the SaveCore function. Then maybe the default CoreStyle for this 
> > plugin should be changed to "eSaveCoreStackOnly".
> > 
> > 2. To the best of my knowledge, it should be theoretically possible to 
> > reimplement Dump() method to sort of take special care of a 
> > MemoryListStream, dumping also the memory information at the end of the 
> > minidump file as I think the memory dump is the most stressful for the 
> > memory and otherwise there is no problem with this.
> To state my preference, I would rather stick to the first option and landed a 
> minimum viable product. The only reason for this is that I have this version 
> way better tested and also I am not entirely sure about how minidump deals 
> with the whole memory dumps as it can be a little problematic for a big 
> memory chunks... Then, I would rather add the necessary changes in the next 
> patch...
That is fine. I was just looking at the implementation below and I think I 
might know why this was taking up too much space. I will comment below, and 
maybe this will fix the issues of storing way too much info in the miniump



Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:522
+  MemoryRegionInfo range_info;
+  error = process_sp->GetMemoryRegionInfo(0, range_info);
+

FYI: memory region for address zero often has no permissions, so there is no 
need to actually try and save this memory. For any memory region be sure the 
check if it has permissions using:

```
if (region_info.GetLLDBPermissions() != 0)
```
before trying to read or save this off. I will comment below



Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:530
+  // Get interesting addresses
+  std::vector interesting_addresses;
+  auto thread_list = process_sp->GetThreadList();

Should we have a std::set to store the base of any regions that 
have already been saved? Many PC values could be in the same regions (like when 
many threads are waiting in syscalls).



Comment at: 
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:544-546
+  while (range_info.GetRange().GetRangeBase() != LLDB_INVALID_ADDRESS) {
+const addr_t addr = range_info.GetRange().GetRangeBase();
+const addr_t size = range_info.GetRange().GetByteSize();

Why are we checking the region_info for address zero here? The first time 
through this loop, it will contain information for address zero which will be a 
region that has no permissions. I suggest a new main loop in the comment below 
which simplifies this entire logic and also makes sure we don't emit the same 
region more than once.



Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:549
+bool is_interesting = false;
+for (size_t interesting_address : interesting_addresses)
+  if (interesting_address >= addr && interesting_address < addr + size) {

Shouldn't this be the main loop that would replace the "while 
(range_info.GetRange().GetRangeBase() != LLDB_INVALID_ADDRESS)" 

[Lldb-commits] [PATCH] D101406: Rename human-readable name for DW_LANG_Mips_Assembler

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

> It doesn't make sense to warn about that. :)

Why not? LLDB doesn't have support for assembler as a source language, no 
assembler expression evaluator, etc..

I don't have much experience debugging assembler sources with LLDB, so I"m not 
sure about the expectations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101406

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

[Lldb-commits] [PATCH] D107079: [lldb] Add an option for emitting LLDB logs as JSON

[Adrian Prantl via Phabricator via lldb-commits]

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

I think this is a great overall addition, might be good to have at least one 
other reviewer sign off though. (Summoning 
@jingham!)
 Should we add a python script to pretty-print a JSON log file?


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

https://reviews.llvm.org/D107079

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

[Lldb-commits] [PATCH] D107456: [lldb] Support .debug_rnglists.dwo sections in dwp file

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil accepted this revision.
jankratochvil added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:556
+  " (ranges list base: 0x%" PRIx64 "): %s",
+  offset, m_ranges_base, toString(table_or_error.takeError()).c_str());
   }

kimanh wrote:
> jankratochvil wrote:
> > One such reason can be missing DWP absolute offset for the error report. 
> > That could be returned from `GetRnglistData()`.
> > 
> If I understand your comment correctly, you are suggesting to incorporate the 
> contribution offset into the error message of `GetRnglistData`, is that 
> correct? However, `GetRnglistData` will only return an error message if the 
> contribution offset cannot be read, so the contribution offset cannot be 
> specified in the error message (and thus the absolute offset cannot be 
> inferred) . Or maybe I'm misunderstanding your comment here?
Not so correct. The callers of `GetRnglistData` need to know the contribution 
offset to properly report it during parsing errors in the callers. 
`GetRnglistTable` can report error if `extractHeaderAndOffsets` finds invalid 
header data. In such case LLDB will report (with 
https://www.jankratochvil.net/t/debug_rnglists-dwp.s.patch):
```
error: debug_rnglists-dwp.s.tmp Failed to extract range list table at offset 
0xc: parsing .debug_rnglists table at offset 0x0: unsupported reserved unit 
length of value 0xfff0
```
And one has no idea which DWO in that DWP file was invalid, that `0xc` is 
relative to start of contribution, not to start of the `.debug_rnglists.dwo` 
section. `GetRnglistData` knows the contribution offset but its caller 
`DWARFDebugRnglistTable` does not.
I am not sure how perfect the error messages should be so giving an approval 
(if I am authorized to give one) but it would be nice to improve the error 
reporting.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107456

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

[Lldb-commits] [PATCH] D107079: [lldb] Add an option for emitting LLDB logs as JSON

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I have a few little grammatical nits.

The suggestion to use EnumArgs rather than a fresh argument type you can do or 
not as you see fit.

It really seems to me that SBDebugger.EnableLog should provide a way to set the 
OutputFormat.  For humans, the plain text is probably preferable, but for 
programs that run lldb and want to control logging output, JSON would seem more 
natural.  And programs that run lldb are more likely to want to use EnableLog 
than HandleCommand, since we've provided it...

But if you do that the OutputFormat should be a public enum, it would be dopey 
to have an API that takes a char * of "plain" or "json".

There should be a setting for the preferred OutputFormat.




Comment at: lldb/docs/design/logging.rst:23
+
+LLDB allows configuring format that log messages are emitted in and what
+metainformation should be attached to each log message. These options can

You at least need "the format", but maybe

configuring the format with which log messages are emitted and the 
metainformation which should...

is a little easier to parse.



Comment at: lldb/docs/design/logging.rst:25
+metainformation should be attached to each log message. These options can
+be set for each logging channel (but not separately for each logging category).
+The output of each channel can also be directed to write to a log file,

These options are applied to log channels, and apply to all enabled categories 
in that channel.






Comment at: lldb/docs/design/logging.rst:47
+
+The current JSON log format puts each log message in its own JSON object. Each
+object also contains a trailing comma character (``,``). The intended way of

This makes it not proper JSON by itself, right?  I'm guessing you do it this 
way so you don't have to track and last log emission?  I can see that that 
would be more trouble than it's worth, so I'm not objecting.  It is a little 
weird to call this JSON format when it is just JSON-adjacent, but that's 
probably too picky...



Comment at: lldb/docs/design/logging.rst:65
+set.
+
+.. list-table:: JSON message fields

We always know which channel/category pair produces the message.  I think it 
would be handy to also always include the channel/category pairs.  That seems 
like it would be useful information.



Comment at: lldb/docs/design/logging.rst:76
+ - The log message itself.
+   * - ``sequence-id``
+ - Number

The sequence ID is useful when the output is unstructured, but do we really 
need it in the structured form?  This is just the index of the message in the 
JSON vector.



Comment at: lldb/docs/design/logging.rst:82
+ - The number of seconds since the host systems epoch (usually UNIX time).
+   * - ``pid``
+ - Number

Seems like the pid & tid are always ganged together.  Maybe indicate that here?

But it does seem a shame that to get the thread info, I have to pay for "pid = 
" in every output message.

Seems like the pid should go in some sort of header, it won't change.  You 
didn't add this behavior, but putting it in the JSON makes the pid emission 
cost about twice what it does in text form.



Comment at: lldb/include/lldb/Utility/Log.h:60
+  /// The format used when emitting log messages.
+  enum class OutputFormat {
+/// Log messages are printed as plain text.

I was a little surprised to see this enum in Log.h and not in the lldb public 
enums.  Don't you need that so you can make an SB API to set the log output 
format?  And don't you need that API?



Comment at: lldb/source/Commands/Options.td:446
 Desc<"Prepend the names of files and function that generate the logs.">;
+  def log_format : Option<"format", "O">, Group<1>, Arg<"LogFormat">,
+Desc<"Specify the used log format for the enabled channels.">;

You could also do this with the EnumArg descriptor.  That way the help will 
show the options.  I'm pretty sure that also auto-gen's the completion for the 
option as well. 



Comment at: lldb/unittests/Utility/LogTest.cpp:275
+
+TEST_F(LogChannelEnabledTest, PlainTextFormat) {
+  std::string error;

Since you allow the Output format to be changed while the channel stays 
enabled, you should add a test for that.  You code should handle that 
gracefully, from what I can see.  But still...


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

https://reviews.llvm.org/D107079

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

[Lldb-commits] [PATCH] D107521: [lldb/Plugins] Introduce Scripted Interface Factory

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I just started reading this but I have to go now.  Had one comment about errors 
from Dispatch.  I'll look more later.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:80
 
-  return static_cast(*should_stop);
-}
-
-Status ScriptedProcessPythonInterface::Stop() {
-  return GetStatusFromMethod("stop");
-}
-
-Status ScriptedProcessPythonInterface::GetStatusFromMethod(
-llvm::StringRef method_name) {
-  Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
- Locker::FreeLock);
-
-  if (!m_object_instance_sp)
-return Status("Python object ill-formed.");
-
-  if (!m_object_instance_sp)
-return Status("Cannot convert Python object to StructuredData::Generic.");
-  PythonObject implementor(PyRefType::Borrowed,
-   (PyObject *)m_object_instance_sp->GetValue());
-
-  if (!implementor.IsAllocated())
-return Status("Python implementor not allocated.");
-
-  PythonObject pmeth(
-  PyRefType::Owned,
-  PyObject_GetAttrString(implementor.get(), method_name.str().c_str()));
-
-  if (PyErr_Occurred())
-PyErr_Clear();
-
-  if (!pmeth.IsAllocated())
-return Status("Python method not allocated.");
-
-  if (PyCallable_Check(pmeth.get()) == 0) {
-if (PyErr_Occurred())
-  PyErr_Clear();
-return Status("Python method not callable.");
-  }
-
-  if (PyErr_Occurred())
-PyErr_Clear();
-
-  PythonObject py_return(PyRefType::Owned,
- PyObject_CallMethod(implementor.get(),
- method_name.str().c_str(),
- nullptr));
-
-  if (PyErr_Occurred()) {
-PyErr_Print();
-PyErr_Clear();
-return Status("Python method could not be called.");
-  }
-
-  if (PyObject *py_ret_ptr = py_return.get()) {
-lldb::SBError *sb_error =
-(lldb::SBError *)LLDBSWIGPython_CastPyObjectToSBError(py_ret_ptr);
-
-if (!sb_error)
-  return Status("Couldn't cast lldb::SBError to lldb::Status.");
-
-Status status = m_interpreter.GetStatusFromSBError(*sb_error);
-
-if (status.Fail())
-  return Status("error: %s", status.AsCString());
-
-return status;
+  if (!obj || !obj->IsValid() || error.Fail()) {
+return error_with_message(llvm::Twine("Null or invalid object (" +

It seems like we also do this bit every time we do Dispatch.  Maybe Dispatch 
could check the outgoing obj & obj->IsValid and if they aren't then it can put 
an appropriate message in the "error" object.  Then you would just have to 
check error.Fail().

That would also simplify the logging.   You could always dump the error's 
GetCString to your error_with_message.  Right now, you don't log the error's 
GetCString in the case where the object is valid, but error.Fail is true, which 
might end up dropping some useful info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107521

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

[Lldb-commits] [PATCH] D108817: [LLDB] Fix 'std::out_of_range' crashing bug when file name completion with using file path.

[Hiroki Imai via Phabricator via lldb-commits]

HirokiImai updated this revision to Diff 369220.
HirokiImai added a comment.

Apply clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108817

Files:
  lldb/source/Host/common/Editline.cpp


Index: lldb/source/Host/common/Editline.cpp
===
--- lldb/source/Host/common/Editline.cpp
+++ lldb/source/Host/common/Editline.cpp
@@ -1006,11 +1006,11 @@
 switch (completion.GetMode()) {
 case CompletionMode::Normal: {
   std::string to_add = completion.GetCompletion();
-  to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
   // Terminate the current argument with a quote if it started with a 
quote.
   if (!request.GetParsedLine().empty() && 
request.GetParsedArg().IsQuoted())
 to_add.push_back(request.GetParsedArg().GetQuoteChar());
   to_add.push_back(' ');
+  el_deletestr(m_editline, request.GetCursorArgumentPrefix().size());
   el_insertstr(m_editline, to_add.c_str());
   // Clear all the autosuggestion parts if the only single space can be 
completed.
   if (to_add == " ")


Index: lldb/source/Host/common/Editline.cpp
===
--- lldb/source/Host/common/Editline.cpp
+++ lldb/source/Host/common/Editline.cpp
@@ -1006,11 +1006,11 @@
 switch (completion.GetMode()) {
 case CompletionMode::Normal: {
   std::string to_add = completion.GetCompletion();
-  to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
   // Terminate the current argument with a quote if it started with a quote.
   if (!request.GetParsedLine().empty() && request.GetParsedArg().IsQuoted())
 to_add.push_back(request.GetParsedArg().GetQuoteChar());
   to_add.push_back(' ');
+  el_deletestr(m_editline, request.GetCursorArgumentPrefix().size());
   el_insertstr(m_editline, to_add.c_str());
   // Clear all the autosuggestion parts if the only single space can be completed.
   if (to_add == " ")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits