[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

2021-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lldb/source/Core/Mangled.cpp:87
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 

rZhBoYao wrote:
> Should we remove `Mangled::operator!` altogether? I find it less confusing 
> without. 
Does `!mangled` work without it? Would it be sufficient to implement the `!` 
operator in terms of bool conversion function?


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

https://reviews.llvm.org/D116217

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

[Lldb-commits] [PATCH] D115662: [lldb][DWARF] Remove duplicate DIE type assignment

2021-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.

I don't have any issues with this, if everything still works. I hoped there 
would be more of a discussion on this (I was hoping I'd maybe learn something 
from it). That obviously didn't happen, but I don't think it needs to hold this 
back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115662

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

[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

2021-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

I think I've managed to figure out what the code is doing, but it wouldn't hurt 
to include a comment giving a high-level overview of what that code is doing 
and/or splitting it into smaller functions with helpful names.

Can we expect to have a test for this?




Comment at: 
lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp:203
+ReadCStringFromMemory(td + offset_td_name, thread_name,
+  fbsd_maxcomlen + 1, error);
+

sizeof(thread_name)



Comment at: 
lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp:221
+ThreadSP thread_sp(new ThreadFreeBSDKernel(
+*this, tid, pcb_addr,
+llvm::formatv("(pid {0}) {1}/{2}", pid, comm, thread_name)));

is this going to be unique (for the entire system)?



Comment at: lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp:29
+ std::string thread_name)
+: Thread(process, tid), m_thread_name(thread_name), m_pcb_addr(pcb_addr) {}
 

std::move


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

https://reviews.llvm.org/D116255

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

[Lldb-commits] [PATCH] D116211: With firmware debug sess, if gdb stub knows the UUID/address of binary, try to load it

2021-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

What happened to the test case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116211

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

[Lldb-commits] [PATCH] D116162: [lldb/python] Fix dangling Event and CommandReturnObject references

2021-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

This isn't a feature I would want advertise too broadly, or add special APIs to 
support it. The way I see it, if a user needs to check if an object has been 
cleared, something has gone wrong already. So I'd rather do something to 
discourage this use case instead of providing more support for it. Crashing is 
definitely discouraging, but maybe a bit too extreme. If there was a simple way 
to throw an exception in this case, then I think that would be a good 
compromise.

The reason I started looking into all of this is because of a bug report where 
the user wanted to stash a debugger object and access it later (which is a 
semi-reasonable thing to do, I'd say). I haven't heard of anybody trying to 
store SBCommandReturnObjects, nor I intend to start encouraging that. The only 
reason I wrote this patch is to tie up loose ends.

> Having the object reseted by SWIG if it goes out-of-scope could collide with 
> this approach. What do you think ?

I'm not sure what you meant by that, but I don't consider this behavior as set 
in stone. If we come up with a different/better way to handle this, then we can 
just change this code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116162

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

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

2021-12-28 Thread PoYao Chang via Phabricator via lldb-commits 
rZhBoYao added inline comments.



Comment at: lldb/source/Core/Mangled.cpp:87
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 

labath wrote:
> rZhBoYao wrote:
> > Should we remove `Mangled::operator!` altogether? I find it less confusing 
> > without. 
> Does `!mangled` work without it? Would it be sufficient to implement the `!` 
> operator in terms of bool conversion function?
Yes, I tried removing it and the unit test I added (`TEST(MangledTest, 
LogicalNotOperator)`) passed. Keeping it this way is fine tho. 


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

https://reviews.llvm.org/D116217

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

[Lldb-commits] [PATCH] D116195: [LLDB] Allows overwriting the existing line entry at given file address when inserting

2021-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.

In D116195#3208476 , @zequanwu wrote:

> In D116195#3207672 , @labath wrote:
>
>> How are you planning to make use of this functionality?
>>
>> I'm asking because I'm wondering if it wouldn't be better to do this kind of 
>> processing in the PDB code, and then hand this class a finished list of line 
>> entries. Inserting entries into the middle of a vector is expensive, which 
>> is why our dwarf code no longer uses this function (it uses the 
>> vector constructor instead). If we could get pdb to do 
>> something similar, then we could get rid of this function altogether.
>
> My plan was to insert entries when parsing inlined call site(`S_INLINESITE `) 
> in ParseBlocksRecursive, which usually happens after ParseLineTable. In PDB, 
> line entries in inlined call site often have same file addresses as line 
> entries in line table, and the former is better since it describes lines 
> inside inlined function rather than lines in caller. And we could have nested 
> inlined call sites. The line entries from inner call sites would always 
> overwrite the line entries from the outer call sites if they have the same 
> file address.

So, I'm afraid that won't work. You shouldn't be modifying the line tables 
after ParseLineTable returns. Lldb (currently) considers the line tables to be 
either fully parsed or not parsed at all. There's no way to "partially" parse a 
line table. Attempting to do so will result in various strange effects, 
including file+line breakpoints resolving differently depending on which blocks 
have been parsed.

I think you'll need to decide whether you want the inline info to be reflected 
in the line table or not. If yes, then you'll need to parse it from within 
ParseLineTable, at which point, you can probably do the deduplication outside 
of the LineTable class -- we can talk about how to make that easier.

We can also try to come up with a completely different way to represent line 
tables in lldb. The currently implementation is clearly dwarf-centric, and 
probably not best suited for the pdb use case. But (due to the breakpoint use 
case) I don't think we can avoid having a mode which does a complete parse of 
whatever we consider to be a line table. The only thing we can do is to avoid 
parsing everything for the case where that is not necessary.

> Maybe it's better to use a set to store the line entries, ordering just by 
> the file address so that insertion is cheaper? Currently, it compares other 
> fields if two lines have same file address 
> (https://github.com/llvm/llvm-project/blob/main/lldb/source/Symbol/LineTable.cpp#L150).
>  Is it necessary? I think line entries in line table always have distinct 
> file address.

Given the above, I think that a sorted vector is an optimal data structure for 
the current state of the world. It's possible we don't need to sort by the 
other fields, but I don't think that's the biggest issue right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116195

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

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

2021-12-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lldb/source/Core/Mangled.cpp:87
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 

rZhBoYao wrote:
> labath wrote:
> > rZhBoYao wrote:
> > > Should we remove `Mangled::operator!` altogether? I find it less 
> > > confusing without. 
> > Does `!mangled` work without it? Would it be sufficient to implement the 
> > `!` operator in terms of bool conversion function?
> Yes, I tried removing it and the unit test I added (`TEST(MangledTest, 
> LogicalNotOperator)`) passed. Keeping it this way is fine tho. 
In that case, you should definitely remove it. We don't want to have two 
implementations that need to be kept in sync.


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

https://reviews.llvm.org/D116217

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

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing bool conversion of Mangled

2021-12-28 Thread PoYao Chang via Phabricator via lldb-commits 
rZhBoYao updated this revision to Diff 396395.
rZhBoYao retitled this revision from "[lldb] Fix PR52702 by fixing 
Mangled::operator!" to "[lldb] Fix PR52702 by fixing bool conversion of 
Mangled".
rZhBoYao edited the summary of this revision.

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

https://reviews.llvm.org/D116217

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/unittests/Core/MangledTest.cpp

Index: lldb/unittests/Core/MangledTest.cpp
===
--- lldb/unittests/Core/MangledTest.cpp
+++ lldb/unittests/Core/MangledTest.cpp
@@ -89,6 +89,25 @@
   EXPECT_STREQ("", the_demangled.GetCString());
 }
 
+TEST(MangledTest, BoolConversionOperator) {
+  {
+ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+Mangled TheMangled(MangledName);
+EXPECT_EQ(true, bool(TheMangled));
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+ConstString UnmangledName("puts");
+Mangled TheMangled(UnmangledName);
+EXPECT_EQ(true, bool(TheMangled));
+EXPECT_EQ(false, !TheMangled);
+  }
+  {
+Mangled TheMangled{};
+EXPECT_EQ(false, bool(TheMangled));
+EXPECT_EQ(true, !TheMangled);
+  }
+}
 
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2140,7 +2140,8 @@
 
   llvm::StringRef basename;
   llvm::StringRef context;
-  bool name_is_mangled = (bool)Mangled(name);
+  bool name_is_mangled = Mangled::GetManglingScheme(name.GetStringRef()) !=
+ Mangled::eManglingSchemeNone;
 
   if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
   context, basename))
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -70,23 +70,13 @@
 SetValue(ConstString(name));
 }
 
-// Convert to pointer operator. This allows code to check any Mangled objects
+// Convert to bool operator. This allows code to check any Mangled objects
 // to see if they contain anything valid using code such as:
 //
 //  Mangled mangled(...);
 //  if (mangled)
 //  { ...
-Mangled::operator void *() const {
-  return (m_mangled) ? const_cast(this) : nullptr;
-}
-
-// Logical NOT operator. This allows code to check any Mangled objects to see
-// if they are invalid using code such as:
-//
-//  Mangled mangled(...);
-//  if (!file_spec)
-//  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+Mangled::operator bool() const { return m_mangled || m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {
Index: lldb/include/lldb/Core/Mangled.h
===
--- lldb/include/lldb/Core/Mangled.h
+++ lldb/include/lldb/Core/Mangled.h
@@ -72,10 +72,10 @@
 return !(*this == rhs);
   }
 
-  /// Convert to pointer operator.
+  /// Convert to bool operator.
   ///
-  /// This allows code to check a Mangled object to see if it contains a valid
-  /// mangled name using code such as:
+  /// This allows code to check any Mangled objects to see if they contain
+  /// anything valid using code such as:
   ///
   /// \code
   /// Mangled mangled(...);
@@ -84,25 +84,9 @@
   /// \endcode
   ///
   /// \return
-  /// A pointer to this object if either the mangled or unmangled
-  /// name is set, NULL otherwise.
-  operator void *() const;
-
-  /// Logical NOT operator.
-  ///
-  /// This allows code to check a Mangled object to see if it contains an
-  /// empty mangled name using code such as:
-  ///
-  /// \code
-  /// Mangled mangled(...);
-  /// if (!mangled)
-  /// { ...
-  /// \endcode
-  ///
-  /// \return
-  /// Returns \b true if the object has an empty mangled and
-  /// unmangled name, \b false otherwise.
-  bool operator!() const;
+  /// Returns \b true if either the mangled or unmangled name is set,
+  /// \b false if the object has an empty mangled and unmangled name.
+  explicit operator bool() const;
 
   /// Clear the mangled and demangled values.
   void Clear();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

2021-12-28 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp:221
+ThreadSP thread_sp(new ThreadFreeBSDKernel(
+*this, tid, pcb_addr,
+llvm::formatv("(pid {0}) {1}/{2}", pid, comm, thread_name)));

labath wrote:
> is this going to be unique (for the entire system)?
Yes, I have specifically verified that.


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

https://reviews.llvm.org/D116255

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

[Lldb-commits] [lldb] a2154b1 - Cache the manual DWARF index out to the LLDB cache directory when the LLDB index cache is enabled.

2021-12-28 Thread Greg Clayton via lldb-commits 

Author: Greg Clayton
Date: 2021-12-28T11:00:28-08:00
New Revision: a2154b19515304f42000160bed820630c3780db8

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

LOG: Cache the manual DWARF index out to the LLDB cache directory when the LLDB 
index cache is enabled.

This patch add the ability to cache the manual DWARF indexing results to disk 
for faster subsequent debug sessions. Manual DWARF indexing is time consuming 
and causes all DWARF to be fully parsed and indexed each time you debug a 
binary that doesn't have an acceptable accelerator table. Acceptable 
accelerator tables include .debug_names in DWARF5 or Apple accelerator tables.

This patch breaks up testing by testing all of the encoding and decoding of 
required C++ objects in a gtest unit test, and then has a test to verify the 
debug info cache is generated correctly.

This patch also adds the ability to track when a symbol table or DWARF index is 
loaded or saved to the cache in the "statistics dump" command. This is 
essential to know in statistics as it can help explain why a debug session was 
slower or faster than expected.

Reviewed By: labath, wallace

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

Added: 

lldb/test/API/functionalities/module_cache/debug_index/TestDebugIndexCache.py
lldb/test/API/functionalities/module_cache/debug_index/exe.yaml
lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp

Modified: 
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Symbol/Symtab.h
lldb/include/lldb/Target/Statistics.h
lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp
lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
lldb/source/Symbol/Symtab.cpp
lldb/source/Target/Statistics.cpp
lldb/test/API/commands/statistics/basic/TestStats.py

lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
lldb/unittests/SymbolFile/DWARF/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/Symbol/SymbolFile.h 
b/lldb/include/lldb/Symbol/SymbolFile.h
index 7c0365483c123..288576b978a78 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -67,8 +67,7 @@ class SymbolFile : public PluginInterface {
 
   // Constructors and Destructors
   SymbolFile(lldb::ObjectFileSP objfile_sp)
-  : m_objfile_sp(std::move(objfile_sp)), m_abilities(0),
-m_calculated_abilities(false) {}
+  : m_objfile_sp(std::move(objfile_sp)) {}
 
   ~SymbolFile() override = default;
 
@@ -326,6 +325,29 @@ class SymbolFile : public PluginInterface {
   /// hasn't been indexed yet, or a valid duration if it has.
   virtual StatsDuration GetDebugInfoIndexTime() { return StatsDuration(0.0); }
 
+  /// Accessors for the bool that indicates if the debug info index was loaded
+  /// from, or saved to the module index cache.
+  ///
+  /// In statistics it is handy to know if a module's debug info was loaded 
from
+  /// or saved to the cache. When the debug info index is loaded from the cache
+  /// startup times can be faster. When the cache is enabled and the debug info
+  /// index is saved to the cache, debug sessions can be slower. These 
accessors
+  /// can be accessed by the statistics and emitted to help track these costs.
+  /// \{
+  bool GetDebugInfoIndexWasLoadedFromCache() const {
+return m_index_was_loaded_from_cache;
+  }
+  void SetDebugInfoIndexWasLoadedFromCache() {
+m_index_was_loaded_from_cache = true;
+  }
+  bool GetDebugInfoIndexWasSavedToCache() const {
+return m_index_was_saved_to_cache;
+  }
+  void SetDebugInfoIndexWasSavedToCache() {
+m_index_was_saved_to_cache = true;
+  }
+  /// \}
+
 protected:
   void AssertModuleLock();
   virtual uint32_t CalculateNumCompileUnits() = 0;
@@ -341,8 +363,10 @@ class SymbolFile : public PluginInterface {
   llvm::Optional> m_compile_units;
   TypeList m_type_list;
   Symtab *m_symtab = nullptr;
-  uint32_t m_abilities;
-  bool m_calculated_abilities;
+  uint32_t m_abilities = 0;
+  bool m_calculated_abilities = false;
+  bool m_index_was_loaded_from_cache = false;
+  bool m_index_was_saved_to_cache = false;
 
 private:
   SymbolFile(const SymbolFile &) = delete;

diff  --git a/lldb/include/lldb/Symbol/Symtab.h 
b/lldb/include/lldb/Symbol/Symtab.h
index fe0a82306c4f5..504b49c026742 100644
--- a/lldb/include/lldb/Symbol/Symtab.h
+++ b/lldb/include/lldb/Symbol/Symtab.h
@@ -212,6 +212,30 @@ class Symtab {
   ///   false if the symbol table wasn't cached or was out of date.
   bool LoadFromCache();
 
+
+  /// Accessors for the bo

[Lldb-commits] [PATCH] D115951: Cache the manual DWARF index out to the LLDB cache directory when the LLDB index cache is enabled.

2021-12-28 Thread Greg Clayton via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa2154b195153: Cache the manual DWARF index out to the LLDB 
cache directory when the LLDB… (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115951

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/Symtab.h
  lldb/include/lldb/Target/Statistics.h
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
  lldb/source/Symbol/Symtab.cpp
  lldb/source/Target/Statistics.cpp
  lldb/test/API/commands/statistics/basic/TestStats.py
  lldb/test/API/functionalities/module_cache/debug_index/TestDebugIndexCache.py
  lldb/test/API/functionalities/module_cache/debug_index/exe.yaml
  lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
  lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
  lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
===
--- /dev/null
+++ lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
@@ -0,0 +1,198 @@
+//===-- DWARFIndexCachingTest.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 "Plugins/SymbolFile/DWARF/DIERef.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDIE.h"
+#include "Plugins/SymbolFile/DWARF/ManualDWARFIndex.h"
+#include "Plugins/SymbolFile/DWARF/NameToDIE.h"
+#include "TestingSupport/Symbol/YAMLModuleTester.h"
+#include "lldb/Core/DataFileCache.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Utility/DataEncoder.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "llvm/ADT/STLExtras.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static void EncodeDecode(const DIERef &object, ByteOrder byte_order) {
+  const uint8_t addr_size = 8;
+  DataEncoder encoder(byte_order, addr_size);
+  object.Encode(encoder);
+  llvm::ArrayRef bytes = encoder.GetData();
+  DataExtractor data(bytes.data(), bytes.size(), byte_order, addr_size);
+  offset_t data_offset = 0;
+  EXPECT_EQ(object, DIERef::Decode(data, &data_offset));
+}
+
+static void EncodeDecode(const DIERef &object) {
+  EncodeDecode(object, eByteOrderLittle);
+  EncodeDecode(object, eByteOrderBig);
+}
+
+TEST(DWARFIndexCachingTest, DIERefEncodeDecode) {
+  // Tests DIERef::Encode(...) and DIERef::Decode(...)
+  EncodeDecode(DIERef(llvm::None, DIERef::Section::DebugInfo, 0x11223344));
+  EncodeDecode(DIERef(llvm::None, DIERef::Section::DebugTypes, 0x11223344));
+  EncodeDecode(DIERef(100, DIERef::Section::DebugInfo, 0x11223344));
+  EncodeDecode(DIERef(200, DIERef::Section::DebugTypes, 0x11223344));
+}
+
+static void EncodeDecode(const NameToDIE &object, ByteOrder byte_order) {
+  const uint8_t addr_size = 8;
+  DataEncoder encoder(byte_order, addr_size);
+  DataEncoder strtab_encoder(byte_order, addr_size);
+  ConstStringTable const_strtab;
+
+  object.Encode(encoder, const_strtab);
+
+  llvm::ArrayRef bytes = encoder.GetData();
+  DataExtractor data(bytes.data(), bytes.size(), byte_order, addr_size);
+
+  const_strtab.Encode(strtab_encoder);
+  llvm::ArrayRef strtab_bytes = strtab_encoder.GetData();
+  DataExtractor strtab_data(strtab_bytes.data(), strtab_bytes.size(),
+byte_order, addr_size);
+  StringTableReader strtab_reader;
+  offset_t strtab_data_offset = 0;
+  ASSERT_EQ(strtab_reader.Decode(strtab_data, &strtab_data_offset), true);
+
+  NameToDIE decoded_object;
+  offset_t data_offset = 0;
+  decoded_object.Decode(data, &data_offset, strtab_reader);
+  EXPECT_TRUE(object == decoded_object);
+}
+
+static void EncodeDecode(const NameToDIE &object) {
+  EncodeDecode(object, eByteOrderLittle);
+  EncodeDecode(object, eByteOrderBig);
+}
+
+TEST(DWARFIndexCachingTest, NameToDIEEncodeDecode) {
+  NameToDIE map;
+  // Make sure an empty NameToDIE map encodes and decodes correctly.
+  EncodeDecode(map);
+  map.Insert(ConstString("hello"),
+ DIERef(llvm::None, DIERef::Section::DebugInfo, 0x11223344));
+  map.Insert(ConstString("workd"),
+ DIERef(100, DIERef::Section::DebugInfo, 0x11223344));
+  // Make sure a valid NameToDIE map encodes and decodes correctly.
+  EncodeDecode(map);
+}
+
+static void EncodeDecode(const ManualDWARFIndex::IndexSet &object,
+ By

[Lldb-commits] [PATCH] D116012: Fix "settings set -g" so it works again.

2021-12-28 Thread Greg Clayton via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG48207b2559c6: Fix "settings set -g" so it works 
again. (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116012

Files:
  lldb/source/Commands/Options.td
  lldb/test/Shell/Settings/TestSettingsSet.test


Index: lldb/test/Shell/Settings/TestSettingsSet.test
===
--- lldb/test/Shell/Settings/TestSettingsSet.test
+++ lldb/test/Shell/Settings/TestSettingsSet.test
@@ -3,6 +3,11 @@
 # Check that setting an empty value with -f(orce) clears the value.
 # RUN: not %lldb -b -s %s 2>&1 | FileCheck %s
 
+# Make sure that "settings set -g" no longer requires a bogus filename.
+settings set -g target.skip-prologue false
+settings show target.skip-prologue
+# CHECK: target.skip-prologue (boolean) = false
+
 settings set tab-size 16
 settings show tab-size
 # CHECK: tab-size (unsigned) = 16
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -18,8 +18,7 @@
 }
 
 let Command = "settings set" in {
-  def setset_global : Option<"global", "g">, Arg<"Filename">,
-Completion<"DiskFile">,
+  def setset_global : Option<"global", "g">,
 Desc<"Apply the new value to the global default value.">;
   def setset_force : Option<"force", "f">,
 Desc<"Force an empty value to be accepted as the default.">;


Index: lldb/test/Shell/Settings/TestSettingsSet.test
===
--- lldb/test/Shell/Settings/TestSettingsSet.test
+++ lldb/test/Shell/Settings/TestSettingsSet.test
@@ -3,6 +3,11 @@
 # Check that setting an empty value with -f(orce) clears the value.
 # RUN: not %lldb -b -s %s 2>&1 | FileCheck %s
 
+# Make sure that "settings set -g" no longer requires a bogus filename.
+settings set -g target.skip-prologue false
+settings show target.skip-prologue
+# CHECK: target.skip-prologue (boolean) = false
+
 settings set tab-size 16
 settings show tab-size
 # CHECK: tab-size (unsigned) = 16
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -18,8 +18,7 @@
 }
 
 let Command = "settings set" in {
-  def setset_global : Option<"global", "g">, Arg<"Filename">,
-Completion<"DiskFile">,
+  def setset_global : Option<"global", "g">,
 Desc<"Apply the new value to the global default value.">;
   def setset_force : Option<"force", "f">,
 Desc<"Force an empty value to be accepted as the default.">;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 48207b2 - Fix "settings set -g" so it works again.

2021-12-28 Thread Greg Clayton via lldb-commits 

Author: Greg Clayton
Date: 2021-12-28T11:03:09-08:00
New Revision: 48207b2559c6b012ce167f2e76acea39e9d405cf

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

LOG: Fix "settings set -g" so it works again.

When we switched options over to use the Options.td file, a bug was introduced 
that caused the "-g" option for "settings set" to require a filename arguemnt. 
This patch fixes this issue and adds a test so this doesn't regress.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/Commands/Options.td
lldb/test/Shell/Settings/TestSettingsSet.test

Removed: 




diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 3e89eb0f6bda1..7fbf0ab039953 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -18,8 +18,7 @@ let Command = "help" in {
 }
 
 let Command = "settings set" in {
-  def setset_global : Option<"global", "g">, Arg<"Filename">,
-Completion<"DiskFile">,
+  def setset_global : Option<"global", "g">,
 Desc<"Apply the new value to the global default value.">;
   def setset_force : Option<"force", "f">,
 Desc<"Force an empty value to be accepted as the default.">;

diff  --git a/lldb/test/Shell/Settings/TestSettingsSet.test 
b/lldb/test/Shell/Settings/TestSettingsSet.test
index 3006a694a16b2..8e90c00c77c2a 100644
--- a/lldb/test/Shell/Settings/TestSettingsSet.test
+++ b/lldb/test/Shell/Settings/TestSettingsSet.test
@@ -3,6 +3,11 @@
 # Check that setting an empty value with -f(orce) clears the value.
 # RUN: not %lldb -b -s %s 2>&1 | FileCheck %s
 
+# Make sure that "settings set -g" no longer requires a bogus filename.
+settings set -g target.skip-prologue false
+settings show target.skip-prologue
+# CHECK: target.skip-prologue (boolean) = false
+
 settings set tab-size 16
 settings show tab-size
 # CHECK: tab-size (unsigned) = 16



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

[Lldb-commits] [PATCH] D116195: [LLDB] Allows overwriting the existing line entry at given file address when inserting

2021-12-28 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu added a comment.

In D116195#3211346 , @labath wrote:

> In D116195#3208476 , @zequanwu 
> wrote:
>
>> In D116195#3207672 , @labath wrote:
>>
>>> How are you planning to make use of this functionality?
>>>
>>> I'm asking because I'm wondering if it wouldn't be better to do this kind 
>>> of processing in the PDB code, and then hand this class a finished list of 
>>> line entries. Inserting entries into the middle of a vector is expensive, 
>>> which is why our dwarf code no longer uses this function (it uses the 
>>> vector constructor instead). If we could get pdb to do 
>>> something similar, then we could get rid of this function altogether.
>>
>> My plan was to insert entries when parsing inlined call site(`S_INLINESITE 
>> `) in ParseBlocksRecursive, which usually happens after ParseLineTable. In 
>> PDB, line entries in inlined call site often have same file addresses as 
>> line entries in line table, and the former is better since it describes 
>> lines inside inlined function rather than lines in caller. And we could have 
>> nested inlined call sites. The line entries from inner call sites would 
>> always overwrite the line entries from the outer call sites if they have the 
>> same file address.
>
> So, I'm afraid that won't work. You shouldn't be modifying the line tables 
> after ParseLineTable returns. Lldb (currently) considers the line tables to 
> be either fully parsed or not parsed at all. There's no way to "partially" 
> parse a line table. Attempting to do so will result in various strange 
> effects, including file+line breakpoints resolving differently depending on 
> which blocks have been parsed.
>
> I think you'll need to decide whether you want the inline info to be 
> reflected in the line table or not. If yes, then you'll need to parse it from 
> within ParseLineTable, at which point, you can probably do the deduplication 
> outside of the LineTable class -- we can talk about how to make that easier.

I think the inline info should be reflected in the line table. Otherwise `image 
lookup -a ...` cannot correctly show the file and line info for inlined 
functions. My plan is to keep a sorted (by address) set of line entries before 
adding into line sequence. Every time we add a new line entry into the set 
while parsing inline info, replace the existing entry with new entry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116195

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