[Lldb-commits] [PATCH] D117299: [lldb] Ignore non-address bits in "memory find" arguments

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 401904.
DavidSpickett added a comment.

Fix "to to" in comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117299

Files:
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/test/API/linux/aarch64/tagged_memory_access/Makefile
  
lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py
  lldb/test/API/linux/aarch64/tagged_memory_access/main.c
  lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
  
lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py
  lldb/test/API/linux/aarch64/tagged_memory_read/main.c

Index: lldb/test/API/linux/aarch64/tagged_memory_access/main.c
===
--- lldb/test/API/linux/aarch64/tagged_memory_access/main.c
+++ lldb/test/API/linux/aarch64/tagged_memory_access/main.c
@@ -5,11 +5,15 @@
   return (char *)((size_t)ptr | (tag << 56));
 }
 
-int main(int argc, char const *argv[]) {
-  char buf[32];
+// Global to zero init
+char buf[32];
 
+int main(int argc, char const *argv[]) {
   char *ptr1 = set_non_address_bits(buf, 0x34);
   char *ptr2 = set_non_address_bits(buf, 0x56);
 
+  // Target value for "memory find"
+  buf[15] = '?';
+
   return 0; // Set break point at this line.
 }
Index: lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py
===
--- lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py
+++ lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py
@@ -1,6 +1,9 @@
 """
-Test that "memory read" removes non address bits from
-memory read arguments.
+Test that "memory read" and "memory find" remove non address bits from
+address arguments.
+
+These tests use the top byte ignore feature of AArch64. Which Linux
+always enables.
 """
 
 
@@ -17,10 +20,7 @@
 
 NO_DEBUG_INFO_TESTCASE = True
 
-# AArch64 Linux always enables top byte ignore
-@skipUnlessArch("aarch64")
-@skipUnlessPlatform(["linux"])
-def test_tagged_memory_read(self):
+def setup_test(self):
 self.build()
 self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
@@ -37,6 +37,11 @@
 substrs=['stopped',
  'stop reason = breakpoint'])
 
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_tagged_memory_read(self):
+self.setup_test()
+
 # If we do not remove non address bits, this can fail in two ways.
 # 1. We attempt to read much more than 16 bytes, probably more than
 #the default 1024 byte read size. Which will error.
@@ -53,3 +58,26 @@
 # Would fail if we don't remove non address bits because 0x56... > 0x34...
 self.expect("memory read ptr2 ptr1+16", patterns=[tagged_addr_pattern], matching=False)
 self.expect("memory read", patterns=[tagged_addr_pattern], matching=False)
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_tagged_memory_find(self):
+self.setup_test()
+
+# If memory find doesn't remove non-address bits one of two
+# things happen.
+# 1. It tries to search a gigantic amount of memory.
+#We're not going to test for this because a failure
+#would take a very long time and perhaps even find the
+#target value randomly.
+# 2. It thinks high address <= low address, which we check below.
+
+self.runCmd("memory find -s '?' ptr2 ptr1+32")
+
+self.assertTrue(self.res.Succeeded())
+out = self.res.GetOutput()
+# memory find does not fail when it doesn't find the data.
+# First check we actually got something.
+self.assertRegexpMatches(out, "data found at location: 0x[0-9A-Fa-f]+")
+# Then that the location found does not display the tag bits.
+self.assertNotRegexpMatches(out, "data found at location: 0x(34|56)[0-9A-Fa-f]+")
Index: lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
@@ -1,3 +0,0 @@
-C_SOURCES := main.c
-
-include Makefile.rules
Index: lldb/source/Commands/CommandObjectMemory.cpp
===
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -1035,6 +1035,12 @@
   return false;
 }
 
+ABISP abi = m_exe_ctx.GetProcessPtr()->GetABI();
+if (abi) {
+  low_addr = abi->FixDataAddress(low_addr);
+  high_addr = abi->FixDataAddress(high_addr);
+}
+
 if (high_addr <= low_addr) {
   result.AppendError(
   "starting address must be smaller than ending add

[Lldb-commits] [PATCH] D117299: [lldb] Ignore non-address bits in "memory find" arguments

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.



Comment at: 
lldb/test/API/linux/aarch64/tagged_memory_access/TestAArch64LinuxTaggedMemoryAccess.py:83
+# Then that the location found does not display the tag bits.
+self.assertNotRegexpMatches(out, "data found at location: 
0x(34|56)[0-9A-Fa-f]+")

omjavaid wrote:
> so I am thinking do we actually need to omit tags from location address. 
> Internally we want LLDB to ignore tags thats fine but user must be seeing the 
> tagged address of buf so why omit tags in output of memory find?
> 
TLDR: The logic here is that the tag (or more generally non-address) bits of 
the pointer are a property of the pointer, not the memory pointed to.

More detail, since I've thought about this a lot. Please challenge if it 
doesn't make sense.

You could argue that you're showing the user a "view" of memory through that 
pointer. My problem there is that pointer auth and memory tagging break that 
argument because you cannot assume they stay the same as the memory location 
increments. Since you can't just assume that the signature for `ptr` and 
`ptr+16` would be the same, or the memory tag for those locations would be the 
same.

The only minor benefit there is the user can copy a tagged pointer to a later 
location, but those tags may not actually let the program access the memory. 
(though the debugger can always but it didn't need tags to do that in any case)

This is what the full output looks like. If you wanted to display the tags in 
the output which tag would you use? Well, simple choice is the start address 
tag. However, it's really an arbitrary choice that I think doesn't help much.

```
(lldb) p ptr1
(char *) $2 = 0x34411029 ""
(lldb) p ptr2
(char *) $3 = 0x56411029 ""
(lldb) memory find -s '?' ptr2 ptr1+16
data found at location: 0x411038
0x00411038: 3f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ?...
0x00411048: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
```

I agree there is an initial "why did my pointer get changed" reaction but think 
I not showing non-address bits when showing actual memory is going to be 
clearer in the long run. Granted this is mostly intuition, we don't have a lot 
of user feedback about this at this stage.

Actual "memory tags" do have a tag that is attached to the memory location in 
hardware, so you may want to show that. That's something I've been addressing 
for "memory read" and will probably port that over to memory find as well. 
It'll look something like:

```
(lldb) memory find -s '?' ptr2 ptr1+16
data found at location: 0x411038
0x00411038: 3f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ?... 
(tag: 0x1)
0x00411048: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   
(tag: 0x2)
```

Crucially here we're showing the real tag for each line, instead of assuming 
that the one in the pointer can be applied to them all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117299

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

[Lldb-commits] [PATCH] D117490: [lldb] Log prototype

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 401909.
labath added a comment.
Herald added a subscriber: emaste.

Do this for real


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117490

Files:
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/include/lldb/Utility/Log.h
  lldb/include/lldb/Utility/Logging.h
  lldb/source/Plugins/Process/POSIX/ProcessPOSIXLog.cpp
  lldb/source/Plugins/Process/POSIX/ProcessPOSIXLog.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteLog.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
  lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.h
  lldb/source/Utility/Log.cpp
  lldb/source/Utility/Logging.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -18,13 +18,24 @@
 using namespace lldb;
 using namespace lldb_private;
 
-enum { FOO = 1, BAR = 2 };
+enum class TestChannel : Log::MaskType {
+  FOO = Log::ChannelFlag<0>,
+  BAR = Log::ChannelFlag<1>,
+  LLVM_MARK_AS_BITMASK_ENUM(BAR),
+};
+
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
 static constexpr Log::Category test_categories[] = {
-{{"foo"}, {"log foo"}, FOO}, {{"bar"}, {"log bar"}, BAR},
+{{"foo"}, {"log foo"}, TestChannel::FOO},
+{{"bar"}, {"log bar"}, TestChannel::BAR},
 };
-static constexpr uint32_t default_flags = FOO;
 
-static Log::Channel test_channel(test_categories, default_flags);
+static Log::Channel test_channel(test_categories, TestChannel::FOO);
+
+namespace lldb_private {
+template <> Log::Channel &LogChannelFor() { return test_channel; }
+} // namespace lldb_private
 
 // Wrap enable, disable and list functions to make them easier to test.
 static bool EnableChannel(std::shared_ptr stream_sp,
@@ -93,7 +104,7 @@
   std::string error;
   ASSERT_TRUE(EnableChannel(m_stream_sp, 0, "chan", {}, error));
 
-  m_log = test_channel.GetLogIfAll(FOO);
+  m_log = GetLog(TestChannel::FOO);
   ASSERT_NE(nullptr, m_log);
 }
 
@@ -124,18 +135,18 @@
 TEST(LogTest, Unregister) {
   llvm::llvm_shutdown_obj obj;
   Log::Register("chan", test_channel);
-  EXPECT_EQ(nullptr, test_channel.GetLogIfAny(FOO));
+  EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
   std::shared_ptr stream_sp(
   new llvm::raw_string_ostream(message));
   EXPECT_TRUE(Log::EnableLogChannel(stream_sp, 0, "chan", {"foo"}, llvm::nulls()));
-  EXPECT_NE(nullptr, test_channel.GetLogIfAny(FOO));
+  EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   Log::Unregister("chan");
-  EXPECT_EQ(nullptr, test_channel.GetLogIfAny(FOO));
+  EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
 }
 
 TEST_F(LogChannelTest, Enable) {
-  EXPECT_EQ(nullptr, test_channel.GetLogIfAll(FOO));
+  EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
   std::shared_ptr stream_sp(
   new llvm::raw_string_ostream(message));
@@ -144,20 +155,22 @@
   EXPECT_EQ("Invalid log channel 'chanchan'.\n", error);
 
   EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {}, error));
-  EXPECT_NE(nullptr, test_channel.GetLogIfAll(FOO));
-  EXPECT_EQ(nullptr, test_channel.GetLogIfAll(BAR));
+  EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
+  EXPECT_EQ(nullptr, GetLog(TestChannel::BAR));
 
   EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {"bar"}, error));
-  EXPECT_NE(nullptr, test_channel.GetLogIfAll(FOO | BAR));
+  EXPECT_NE(nullptr, test_channel.GetLogIfAll(
+ Log::MaskType(TestChannel::FOO | TestChannel::BAR)));
 
   EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {"baz"}, error));
   EXPECT_NE(std::string::npos, error.find("unrecognized log category 'baz'"))
   << "error: " << error;
-  EXPECT_NE(nullptr, test_channel.GetLogIfAll(FOO | BAR));
+  EXPECT_NE(nullptr, test_channel.GetLogIfAll(
+ Log::MaskType(TestChannel::FOO | TestChannel::BAR)));
 }
 
 TEST_F(LogChannelTest, EnableOptions) {
-  EXPECT_EQ(nullptr, test_channel.GetLogIfAll(FOO));
+  EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
   std::shared_ptr stream_sp(
   new llvm::raw_string_ostream(message));
@@ -165,32 +178,33 @@
   EXPECT_TRUE(
   EnableChannel(stream_sp, LLDB_LOG_OPTION_VERBOSE, "chan", {}, error));
 
-  Log *log = test_channel.GetLogIfAll(FOO);
+  Log *log = GetLog(TestChannel::FOO);
   ASSERT_NE(nullptr, log);
   EXPECT_TRUE(log->GetVerbose());
 }
 
 TEST_F(LogChannelTest, Disable) {
-  EXPECT_EQ(nullptr, test_channel.GetLogIfAll(FOO));
+  EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string me

[Lldb-commits] [PATCH] D117490: [lldb] Make logging machinery type-safe

[Pavel Labath via Phabricator via lldb-commits]

labath marked 4 inline comments as done.
labath added a comment.

This should be ready for a full review now.




Comment at: lldb/include/lldb/Utility/Logging.h:15
-// Log Bits specific to logging in lldb
-#define LIBLLDB_LOG_PROCESS (1u << 1)
-#define LIBLLDB_LOG_THREAD (1u << 2)

Bit zero was unused, so we still have one bit left, but I've created a new 
typedef for holding the category flags, so extending it should be easy.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteLog.cpp:17
 
 static constexpr Log::Category g_categories[] = {
+{{"async"}, {"log asynchronous activity"}, uint32_t(GDBR_LOG_ASYNC)},

labath wrote:
> clayborg wrote:
> > Should the Log::Category be templatized with GDBRLog and then we just store 
> > GDBRLog enum values instead of uint32_t values?
> The problem with that is that it would require templatizing pretty much all 
> of the logging code (as it deals with categories in one way or another), but 
> I agree that we should at least make the category definitions nice. It should 
> be possible to keep templates here, but then convert to integers when we get 
> to the core logging code, but i did not want to get entagled in that for the 
> prototype.
I think I've mostly solved this by templatizing the `Log::Category` constructor 
(instead of the whole class). That way the generic code remains template free, 
and there are no casts in the user code. The constructor converts the enum 
values to integers, after verifying that they are of the right type.



Comment at: lldb/source/Utility/Logging.cpp:28-40
+{{"dyld"},
+ {"log shared library related activities"},
+ LLDBLog::DynamicLoader},
+{{"event"},
+ {"log broadcaster, listener and event queue activities"},
+ LLDBLog::Events},
+{{"expr"}, {"log expressions"}, LLDBLog::Expressions},

This would also be a good time to bikeshed on the names, as some of these are 
fairly inconsistent (`dyld` vs `DynamicLoader`, `event` vs `Events`, `jit` vs 
`JITLoader`). I think we could standardise on the shorter, user-facing ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117490

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

[Lldb-commits] [PATCH] D114627: [lldb] add new overload for SymbolFile::FindTypes that accepts a scope

[Lasse Folger via Phabricator via lldb-commits]

lassefolger updated this revision to Diff 401947.
lassefolger added a comment.

rebase patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114627

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Symbol/SymbolFile.cpp

Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -133,6 +133,19 @@
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {}
 
+void SymbolFile::FindTypes(
+ConstString basename, ConstString scope,
+const CompilerDeclContext &parent_decl_ctx, uint32_t max_matches,
+llvm::DenseSet &searched_symbol_files,
+TypeMap &types) {
+  FindTypes(basename, parent_decl_ctx, max_matches, searched_symbol_files,
+types);
+  TypeClass type_class = eTypeClassAny;
+  types.RemoveMismatchedTypes(std::string(basename.GetCString()),
+  std::string(scope.GetCString()), type_class,
+  scope.GetStringRef().startswith("::"));
+}
+
 void SymbolFile::FindTypes(llvm::ArrayRef pattern,
LanguageSet languages,
llvm::DenseSet &searched_symbol_files,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -191,6 +191,13 @@
   const std::string &scope_qualified_name,
   std::vector &mangled_names) override;
 
+  void
+  FindTypes(lldb_private::ConstString name, lldb_private::ConstString scope,
+const lldb_private::CompilerDeclContext &parent_decl_ctx,
+uint32_t max_matches,
+llvm::DenseSet &searched_symbol_files,
+lldb_private::TypeMap &types) override;
+
   void
   FindTypes(lldb_private::ConstString name,
 const lldb_private::CompilerDeclContext &parent_decl_ctx,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -9,6 +9,7 @@
 #include "SymbolFileDWARF.h"
 
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Threading.h"
 
@@ -21,6 +22,7 @@
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Scalar.h"
 #include "lldb/Utility/StreamString.h"
@@ -2409,6 +2411,15 @@
 uint32_t max_matches,
 llvm::DenseSet &searched_symbol_files,
 TypeMap &types) {
+  FindTypes(name, ConstString(), parent_decl_ctx, max_matches,
+searched_symbol_files, types);
+}
+
+void SymbolFileDWARF::FindTypes(
+ConstString name, ConstString scope,
+const CompilerDeclContext &parent_decl_ctx, uint32_t max_matches,
+llvm::DenseSet &searched_symbol_files,
+TypeMap &types) {
   std::lock_guard guard(GetModuleMutex());
   // Make sure we haven't already searched this SymbolFile before.
   if (!searched_symbol_files.insert(this).second)
@@ -2435,10 +2446,25 @@
   if (!DeclContextMatchesThisSymbolFile(parent_decl_ctx))
 return;
 
+  // prepare string to check against in case this is a fully qualified search
+  bool has_scope = !scope.IsEmpty();
+  llvm::StringRef scope_ref(scope.GetStringRef());
+  if (scope_ref.size() != 2) {
+// DWARFDIE prefixes only variables in global scope with '::'
+scope_ref.consume_front("::");
+  }
+  std::string storage;
+
   m_index->GetTypes(name, [&](DWARFDIE die) {
 if (!DIEInDeclContext(parent_decl_ctx, die))
   return true; // The containing decl contexts don't match
 
+if (has_scope) {
+  die.GetQualifiedName(storage);
+  if (!storage.empty() && !llvm::StringRef(storage).startswith(scope_ref))
+return true;
+}
+
 Type *matching_type = ResolveType(die, true, true);
 if (!matching_type)
   return true;
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -947,6 +947,17 @@
searched_symbol_files, types);
 }
 
+void Module::FindTypes_Impl(
+ConstString name, ConstString scope,
+const CompilerDeclContext &parent_decl_ctx, size_t max_matches,
+llvm::DenseSet &searched_symbol_files,
+TypeMap &types) {
+  LLDB_SCOPED_TIMER();
+  if (SymbolFile *symb

[Lldb-commits] [lldb] 75e164f - [llvm] Cleanup header dependencies in ADT and Support

[via lldb-commits]

Author: serge-sans-paille
Date: 2022-01-21T13:54:49+01:00
New Revision: 75e164f61d391979b4829bf2746a5d74b94e95f2

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

LOG: [llvm] Cleanup header dependencies in ADT and Support

The cleanup was manual, but assisted by "include-what-you-use". It consists in

1. Removing unused forward declaration. No impact expected.
2. Removing unused headers in .cpp files. No impact expected.
3. Removing unused headers in .h files. This removes implicit dependencies and
   is generally considered a good thing, but this may break downstream builds.
   I've updated llvm, clang, lld, lldb and mlir deps, and included a list of the
   modification in the second part of the commit.
4. Replacing header inclusion by forward declaration. This has the same impact
   as 3.

Notable changes:

- llvm/Support/TargetParser.h no longer includes 
llvm/Support/AArch64TargetParser.h nor llvm/Support/ARMTargetParser.h
- llvm/Support/TypeSize.h no longer includes llvm/Support/WithColor.h
- llvm/Support/YAMLTraits.h no longer includes llvm/Support/Regex.h
- llvm/ADT/SmallVector.h no longer includes llvm/Support/MemAlloc.h nor 
llvm/Support/ErrorHandling.h

You may need to add some of these headers in your compilation units, if needs 
be.

As an hint to the impact of the cleanup, running

clang++ -E  -Iinclude -I../llvm/include ../llvm/lib/Support/*.cpp -std=c++14 
-fno-rtti -fno-exceptions | wc -l

before: 8000919 lines
after:  7917500 lines

Reduced dependencies also helps incremental rebuilds and is more ccache
friendly, something not shown by the above metric :-)

Discourse thread on the topic: 
https://llvm.discourse.group/t/include-what-you-use-include-cleanup/5831

Added: 


Modified: 
clang/lib/Basic/Targets/AArch64.h
clang/lib/Basic/Targets/ARM.h
clang/lib/Driver/SanitizerArgs.cpp
clang/lib/Driver/ToolChains/Arch/AArch64.cpp
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Arch/ARM.h
clang/tools/libclang/BuildSystem.cpp
lldb/source/Host/common/Socket.cpp
llvm/include/llvm/ADT/Optional.h
llvm/include/llvm/ADT/SmallVector.h
llvm/include/llvm/MC/MCStreamer.h
llvm/include/llvm/Object/ELFObjectFile.h
llvm/include/llvm/Support/AArch64TargetParser.h
llvm/include/llvm/Support/ARMAttributeParser.h
llvm/include/llvm/Support/Allocator.h
llvm/include/llvm/Support/BinaryStreamReader.h
llvm/include/llvm/Support/BinaryStreamRef.h
llvm/include/llvm/Support/BinaryStreamWriter.h
llvm/include/llvm/Support/BlockFrequency.h
llvm/include/llvm/Support/BranchProbability.h
llvm/include/llvm/Support/ConvertUTF.h
llvm/include/llvm/Support/ELFAttributeParser.h
llvm/include/llvm/Support/Error.h
llvm/include/llvm/Support/ExtensibleRTTI.h
llvm/include/llvm/Support/FileCollector.h
llvm/include/llvm/Support/FileUtilities.h
llvm/include/llvm/Support/FormatVariadic.h
llvm/include/llvm/Support/GraphWriter.h
llvm/include/llvm/Support/ItaniumManglingCanonicalizer.h
llvm/include/llvm/Support/RISCVISAInfo.h
llvm/include/llvm/Support/SymbolRemappingReader.h
llvm/include/llvm/Support/TargetParser.h
llvm/include/llvm/Support/TimeProfiler.h
llvm/include/llvm/Support/Timer.h
llvm/include/llvm/Support/TrigramIndex.h
llvm/include/llvm/Support/TypeSize.h
llvm/include/llvm/Support/YAMLTraits.h
llvm/lib/Debuginfod/Debuginfod.cpp
llvm/lib/Object/Object.cpp
llvm/lib/Support/APInt.cpp
llvm/lib/Support/ARMAttributeParser.cpp
llvm/lib/Support/ARMWinEH.cpp
llvm/lib/Support/BlockFrequency.cpp
llvm/lib/Support/DAGDeltaAlgorithm.cpp
llvm/lib/Support/DataExtractor.cpp
llvm/lib/Support/ELFAttributeParser.cpp
llvm/lib/Support/FileOutputBuffer.cpp
llvm/lib/Support/FileUtilities.cpp
llvm/lib/Support/GraphWriter.cpp
llvm/lib/Support/InitLLVM.cpp
llvm/lib/Support/JSON.cpp
llvm/lib/Support/MSP430AttributeParser.cpp
llvm/lib/Support/MemoryBuffer.cpp
llvm/lib/Support/NativeFormatting.cpp
llvm/lib/Support/PrettyStackTrace.cpp
llvm/lib/Support/ScopedPrinter.cpp
llvm/lib/Support/Signals.cpp
llvm/lib/Support/Signposts.cpp
llvm/lib/Support/SmallPtrSet.cpp
llvm/lib/Support/SmallVector.cpp
llvm/lib/Support/SpecialCaseList.cpp
llvm/lib/Support/StringMap.cpp
llvm/lib/Support/SymbolRemappingReader.cpp
llvm/lib/Support/TargetParser.cpp
llvm/lib/Support/ThreadPool.cpp
llvm/lib/Support/TimeProfiler.cpp
llvm/lib/Support/ToolOutputFile.cpp
llvm/lib/Support/Triple.cpp
llvm/lib/Support/TypeSize.cpp
llvm/lib/Support/VirtualFileSystem.cpp
llvm/lib/Support/X86TargetParser.cpp
llvm/lib/Support/YAMLParser.cpp
llvm/lib/Support/YAMLTraits.cpp
llvm/lib/Support/raw_ostream.cpp
llvm/lib/Target/AArch64/AArch64Subtarget.cpp
llv

[Lldb-commits] [lldb] e9211e0 - Remove dependency from raw_ostream on

[via lldb-commits]

Author: serge-sans-paille
Date: 2022-01-21T15:17:39+01:00
New Revision: e9211e03937751ab75bbb34e38acc330b85fb0d8

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

LOG: Remove dependency from raw_ostream on 

The tryLockFor method from raw_fd_sotreamis the sole user of that
header, and it's not referenced in the mono repo. I still chose to keep
it (may be useful for downstream user) but added a transient type that's
forward declared to hold the duration parameter.

Notable changes:

- "llvm/Support/Duration.h" must be included in order to use tryLockFor.
- "llvm/Support/raw_ostream.h" no longer includes 

This sole change has an interesting impact on the number of processed
line, as measured by:

clang++ -E  -Iinclude -I../llvm/include ../llvm/lib/Support/*.cpp -std=c++14 
-fno-rtti -fno-exceptions | wc -l

before: 7917500
after:  7835142

Discourse thread on the topic: 
https://llvm.discourse.group/t/include-what-you-use-include-cleanup/5831

Added: 
llvm/include/llvm/Support/Duration.h

Modified: 
lldb/tools/lldb-vscode/FifoFiles.h
llvm/include/llvm/Debuginfod/Debuginfod.h
llvm/include/llvm/Debuginfod/HTTPClient.h
llvm/include/llvm/Support/raw_ostream.h
llvm/lib/Support/raw_ostream.cpp
llvm/unittests/Support/Path.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/FifoFiles.h 
b/lldb/tools/lldb-vscode/FifoFiles.h
index f186f65e86c43..a0c4562b5a6b7 100644
--- a/lldb/tools/lldb-vscode/FifoFiles.h
+++ b/lldb/tools/lldb-vscode/FifoFiles.h
@@ -14,6 +14,8 @@
 
 #include "JSONUtils.h"
 
+#include 
+
 namespace lldb_vscode {
 
 /// Struct that controls the life of a fifo file in the filesystem.

diff  --git a/llvm/include/llvm/Debuginfod/Debuginfod.h 
b/llvm/include/llvm/Debuginfod/Debuginfod.h
index fcb8ed3a9222b..064cfa75b1a1b 100644
--- a/llvm/include/llvm/Debuginfod/Debuginfod.h
+++ b/llvm/include/llvm/Debuginfod/Debuginfod.h
@@ -23,6 +23,8 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 
+#include 
+
 namespace llvm {
 
 typedef ArrayRef BuildIDRef;

diff  --git a/llvm/include/llvm/Debuginfod/HTTPClient.h 
b/llvm/include/llvm/Debuginfod/HTTPClient.h
index e8f0e7ef8f786..ca3b76ca9f3f4 100644
--- a/llvm/include/llvm/Debuginfod/HTTPClient.h
+++ b/llvm/include/llvm/Debuginfod/HTTPClient.h
@@ -19,6 +19,8 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 
+#include 
+
 namespace llvm {
 
 enum class HTTPMethod { GET };

diff  --git a/llvm/include/llvm/Support/Duration.h 
b/llvm/include/llvm/Support/Duration.h
new file mode 100644
index 0..a5a0e2a3357aa
--- /dev/null
+++ b/llvm/include/llvm/Support/Duration.h
@@ -0,0 +1,28 @@
+//===--- Duration.h - wrapper around std::chrono::Duration --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  The sole purpose of this file is to avoid the dependency on  in
+//  raw_ostream.
+//
+//===--===//
+
+#ifndef LLVM_SUPPORT_DURATION_H
+#define LLVM_SUPPORT_DURATION_H
+
+#include 
+
+namespace llvm {
+class Duration {
+  std::chrono::milliseconds Value;
+  public:
+  Duration(std::chrono::milliseconds Value) : Value(Value) {}
+  std::chrono::milliseconds getDuration() const { return Value; }
+};
+}
+
+#endif

diff  --git a/llvm/include/llvm/Support/raw_ostream.h 
b/llvm/include/llvm/Support/raw_ostream.h
index fc46ec0d74564..e288ac27e804d 100644
--- a/llvm/include/llvm/Support/raw_ostream.h
+++ b/llvm/include/llvm/Support/raw_ostream.h
@@ -17,7 +17,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DataTypes.h"
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -30,6 +29,7 @@
 
 namespace llvm {
 
+class Duration;
 class formatv_object_base;
 class format_object_base;
 class FormattedString;
@@ -574,7 +574,7 @@ class raw_fd_ostream : public raw_pwrite_stream {
   ///
   /// It is used as @ref lock.
   LLVM_NODISCARD
-  Expected tryLockFor(std::chrono::milliseconds Timeout);
+  Expected tryLockFor(Duration const& Timeout);
 };
 
 /// This returns a reference to a raw_fd_ostream for standard output. Use it

diff  --git a/llvm/lib/Support/raw_ostream.cpp 
b/llvm/lib/Support/raw_ostream.cpp
index 1b1b0af79ae8d..e4b747b68beaa 100644
--- a/llvm/lib/Support/raw_ostream.cpp
+++ b/llvm/lib/Support/raw_ostream.cpp
@@ -15,6 +15,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/Duration.h"
 #include "llvm/Support/ErrorHandling.h"
 #include

[Lldb-commits] [PATCH] D117490: [lldb] Make logging machinery type-safe

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/Log.h:57
+  /// };
+  using MaskType = uint32_t;
+

Didn't this all start with Greg wanting to make this a `uint64_t`?



Comment at: lldb/include/lldb/Utility/Logging.h:19-50
+  API = Log::ChannelFlag<0>,
+  AST = Log::ChannelFlag<1>,
+  Breakpoints = Log::ChannelFlag<2>,
+  Commands = Log::ChannelFlag<3>,
+  Communication = Log::ChannelFlag<4>,
+  Connection = Log::ChannelFlag<5>,
+  DataFormatters = Log::ChannelFlag<6>,

I would put this into a `.def` file and have it generate both this list as well 
as the one with the macros below. The last entry is annoying. We could either 
omit it from the list but then you risk someone updating the def file but not 
this. I don't think there's a way to do this automatically? Maybe just an 
additional define? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117490

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

[Lldb-commits] [PATCH] D117490: [lldb] Make logging machinery type-safe

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

This is a nice refactor, I am curious was there a motivating bug or issue for 
this change or just refactoring?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117490

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

[Lldb-commits] [PATCH] D117707: [lldb] [Platform] Support synthesizing siginfo_t

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

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

Simplify the tests to use dotted member notation for field lookup. Make the 
test utility generate exact test code.


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

https://reviews.llvm.org/D117707

Files:
  lldb/bindings/interface/SBPlatform.i
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/Platform.cpp
  lldb/unittests/Platform/CMakeLists.txt
  lldb/unittests/Platform/PlatformLinuxTest.cpp
  lldb/unittests/Platform/tools/generate_siginfo.c

Index: lldb/unittests/Platform/tools/generate_siginfo.c
===
--- /dev/null
+++ lldb/unittests/Platform/tools/generate_siginfo.c
@@ -0,0 +1,63 @@
+//===-- generate_siginfo_linux.c --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include 
+#include 
+#include 
+
+siginfo_t siginfo;
+
+#define P(member)  \
+  printf("   {\"%s\", %zd, %zd},\n", #member,   \
+ offsetof(siginfo_t, member), sizeof(siginfo.member));
+
+// undef annoying glibc macros
+#undef si_pid
+#undef si_uid
+#undef si_overrun
+#undef si_status
+#undef si_utime
+#undef si_stime
+#undef si_addr
+#undef si_addr_lsb
+#undef si_band
+#undef si_fd
+
+int main() {
+  printf("  ExpectFields(siginfo_type,\n");
+  printf("   {\n");
+  P(si_signo);
+  P(si_errno);
+  P(si_code);
+  P(_sifields._kill.si_pid);
+  P(_sifields._kill.si_uid);
+  P(_sifields._timer.si_tid);
+  P(_sifields._timer.si_overrun);
+  P(_sifields._timer.si_sigval);
+  P(_sifields._rt.si_pid);
+  P(_sifields._rt.si_uid);
+  P(_sifields._rt.si_sigval);
+  P(_sifields._sigchld.si_pid);
+  P(_sifields._sigchld.si_uid);
+  P(_sifields._sigchld.si_status);
+  P(_sifields._sigchld.si_utime);
+  P(_sifields._sigchld.si_stime);
+  P(_sifields._sigfault.si_addr);
+  P(_sifields._sigfault.si_addr_lsb);
+  P(_sifields._sigfault._bounds._addr_bnd._lower);
+  P(_sifields._sigfault._bounds._addr_bnd._upper);
+  P(_sifields._sigfault._bounds._pkey);
+  P(_sifields._sigpoll.si_band);
+  P(_sifields._sigpoll.si_fd);
+  P(_sifields._sigsys._call_addr);
+  P(_sifields._sigsys._syscall);
+  P(_sifields._sigsys._arch);
+  printf("   });\n");
+
+  return 0;
+}
Index: lldb/unittests/Platform/PlatformLinuxTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/PlatformLinuxTest.cpp
@@ -0,0 +1,268 @@
+//===-- PlatformLinuxTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include 
+#include 
+
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::repro;
+
+namespace {
+class PlatformLinuxTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+  PlatformSP platform_sp;
+  DebuggerSP debugger_sp;
+  TargetSP target_sp;
+
+public:
+  CompilerType siginfo_type;
+
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+platform_linux::PlatformLinux::Initialize();
+  }
+
+  void TearDown() override {
+platform_linux::PlatformLinux::Terminate();
+Reproducer::Terminate();
+  }
+
+  typedef std::tuple field_tuple;
+
+  void ExpectField(const CompilerType &siginfo_type, field_tuple field) {
+const char *path;
+uint64_t offset, size;
+std::tie(path, offset, size) = field;
+
+SCOPED_TRACE(path);
+CompilerType field_type = siginfo_type;
+uint64_t total_offset = 0;
+for (auto field_name : llvm::split(path, '.')) {
+  uint64_t bit_offset;
+  ASSERT_NE(field_type.GetIndexOfFieldWithName(field_name.str().c_str(),
+   &field_type, &bit_offset),
+UINT32_MAX);
+  total_offset += bit_offset;
+}
+
+EXPECT_EQ

[Lldb-commits] [PATCH] D117707: [lldb] [Platform] Support synthesizing siginfo_t

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

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

Update following instrumentation changes.


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

https://reviews.llvm.org/D117707

Files:
  lldb/bindings/interface/SBPlatform.i
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/Platform.cpp
  lldb/unittests/Platform/CMakeLists.txt
  lldb/unittests/Platform/PlatformLinuxTest.cpp
  lldb/unittests/Platform/tools/generate_siginfo.c

Index: lldb/unittests/Platform/tools/generate_siginfo.c
===
--- /dev/null
+++ lldb/unittests/Platform/tools/generate_siginfo.c
@@ -0,0 +1,63 @@
+//===-- generate_siginfo_linux.c --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include 
+#include 
+#include 
+
+siginfo_t siginfo;
+
+#define P(member)  \
+  printf("   {\"%s\", %zd, %zd},\n", #member,   \
+ offsetof(siginfo_t, member), sizeof(siginfo.member));
+
+// undef annoying glibc macros
+#undef si_pid
+#undef si_uid
+#undef si_overrun
+#undef si_status
+#undef si_utime
+#undef si_stime
+#undef si_addr
+#undef si_addr_lsb
+#undef si_band
+#undef si_fd
+
+int main() {
+  printf("  ExpectFields(siginfo_type,\n");
+  printf("   {\n");
+  P(si_signo);
+  P(si_errno);
+  P(si_code);
+  P(_sifields._kill.si_pid);
+  P(_sifields._kill.si_uid);
+  P(_sifields._timer.si_tid);
+  P(_sifields._timer.si_overrun);
+  P(_sifields._timer.si_sigval);
+  P(_sifields._rt.si_pid);
+  P(_sifields._rt.si_uid);
+  P(_sifields._rt.si_sigval);
+  P(_sifields._sigchld.si_pid);
+  P(_sifields._sigchld.si_uid);
+  P(_sifields._sigchld.si_status);
+  P(_sifields._sigchld.si_utime);
+  P(_sifields._sigchld.si_stime);
+  P(_sifields._sigfault.si_addr);
+  P(_sifields._sigfault.si_addr_lsb);
+  P(_sifields._sigfault._bounds._addr_bnd._lower);
+  P(_sifields._sigfault._bounds._addr_bnd._upper);
+  P(_sifields._sigfault._bounds._pkey);
+  P(_sifields._sigpoll.si_band);
+  P(_sifields._sigpoll.si_fd);
+  P(_sifields._sigsys._call_addr);
+  P(_sifields._sigsys._syscall);
+  P(_sifields._sigsys._arch);
+  printf("   });\n");
+
+  return 0;
+}
Index: lldb/unittests/Platform/PlatformLinuxTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/PlatformLinuxTest.cpp
@@ -0,0 +1,268 @@
+//===-- PlatformLinuxTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include 
+#include 
+
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::repro;
+
+namespace {
+class PlatformLinuxTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+  PlatformSP platform_sp;
+  DebuggerSP debugger_sp;
+  TargetSP target_sp;
+
+public:
+  CompilerType siginfo_type;
+
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+platform_linux::PlatformLinux::Initialize();
+  }
+
+  void TearDown() override {
+platform_linux::PlatformLinux::Terminate();
+Reproducer::Terminate();
+  }
+
+  typedef std::tuple field_tuple;
+
+  void ExpectField(const CompilerType &siginfo_type, field_tuple field) {
+const char *path;
+uint64_t offset, size;
+std::tie(path, offset, size) = field;
+
+SCOPED_TRACE(path);
+CompilerType field_type = siginfo_type;
+uint64_t total_offset = 0;
+for (auto field_name : llvm::split(path, '.')) {
+  uint64_t bit_offset;
+  ASSERT_NE(field_type.GetIndexOfFieldWithName(field_name.str().c_str(),
+   &field_type, &bit_offset),
+UINT32_MAX);
+  total_offset += bit_offset;
+}
+
+EXPECT_EQ(total_offset, offset * 8);
+EXPECT_EQ(field_type.GetByteSize(nullptr)

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: clayborg, labath.
JDevlieghere requested review of this revision.

Add statistics about the memory usage of the string pool. I'm particularly 
interested in the memory used by the allocator, i.e. the number of bytes 
actually used by the allocator it self as well as the number of bytes allocated 
through the allocator.


https://reviews.llvm.org/D117914

Files:
  lldb/include/lldb/Target/Statistics.h
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Target/Statistics.cpp
  lldb/source/Utility/ConstString.cpp
  lldb/test/API/commands/statistics/basic/TestStats.py

Index: lldb/test/API/commands/statistics/basic/TestStats.py
===
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -135,6 +135,7 @@
 
 (lldb) statistics dump
 {
+  "strings" : {...},
   "modules" : [...],
   "targets" : [
 {
@@ -160,6 +161,7 @@
 target = self.createTestTarget()
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'strings',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
@@ -197,6 +199,7 @@
 
 (lldb) statistics dump
 {
+  "strings" : {...},
   "modules" : [...],
   "targets" : [
 {
@@ -227,6 +230,7 @@
   lldb.SBFileSpec("main.c"))
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'strings',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
@@ -254,6 +258,36 @@
 self.assertGreater(stats['launchOrAttachTime'], 0.0)
 self.assertGreater(stats['targetCreateTime'], 0.0)
 
+def test_strings(self):
+"""
+Test "statistics dump" and the string memory information.
+"""
+exe = self.getBuildArtifact("a.out")
+target = self.createTestTarget(file_path=exe)
+debug_stats = self.get_stats()
+debug_stat_keys = [
+'strings',
+'modules',
+'targets',
+'totalSymbolTableParseTime',
+'totalSymbolTableIndexTime',
+'totalSymbolTablesLoadedFromCache',
+'totalSymbolTablesSavedToCache',
+'totalDebugInfoParseTime',
+'totalDebugInfoIndexTime',
+'totalDebugInfoIndexLoadedFromCache',
+'totalDebugInfoIndexSavedToCache',
+'totalDebugInfoByteSize'
+]
+self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
+stats = debug_stats['strings']
+keys_exist = [
+'bytesTotal',
+'bytesAllocated',
+'bytesWasted',
+]
+self.verify_keys(stats, '"stats"', keys_exist, None)
+
 def find_module_in_metrics(self, path, stats):
 modules = stats['modules']
 for module in modules:
@@ -269,6 +303,7 @@
 target = self.createTestTarget(file_path=exe)
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'strings',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
@@ -312,6 +347,7 @@
 Output expected to be something like:
 
 {
+  "strings" : {...},
   "modules" : [...],
   "targets" : [
 {
@@ -355,6 +391,7 @@
 self.runCmd("b a_function")
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'strings',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -171,6 +171,17 @@
 return mem_size;
   }
 
+  ConstString::MemoryStats GetMemoryStats() const {
+ConstString::MemoryStats stats;
+for (const auto &pool : m_string_pools) {
+  llvm::sys::SmartScopedReader rlock(pool.m_mutex);
+  const Allocator &alloc = pool.m_string_map.getAllocator();
+  stats.total_memory += alloc.getTotalMemory();
+  stats.bytes_allocated += alloc.getBytesAllocated();
+}
+return stats;
+  }
+
 protected:
   uint8_t hash(const llvm::StringRef &s) const {
 uint32_t h = llvm::djbHash(s);
@@ -332,6 +343,10 @@
   return StringPool().MemorySize();
 }
 
+ConstString::MemoryStats ConstString::GetMemoryStats() {
+  return StringPool().GetMemoryStats();
+}
+
 void llvm::format_provider::format(const ConstString &CS,
 llvm::raw_ostream &OS,
 llvm::StringRef Options) {
Index: lldb/source/Target/Statistics.cpp
===
--- lldb/s

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/ConstString.h:409
   /// in memory.
   static size_t StaticMemorySize();
 

I purposely didn't use this function for the reasons explained in the summary. 
It seems like a few classes implement this, but it's not actually used 
anywhere. If we care, I can reimplement this in terms of the new MemoryStats. 
If we don't I can remove it in a follow-up patch.


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

https://reviews.llvm.org/D117914

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

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Dave Lee via Phabricator via lldb-commits]

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

nice


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

https://reviews.llvm.org/D117914

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

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Target/Statistics.cpp:72
+  obj.try_emplace("bytesAllocated", stats.GetBytesAllocated());
+  obj.try_emplace("bytesWasted", stats.GetBytesWasted());
+  return obj;

Maybe "bytesUnallocated" instead of "bytesWasted"?



Comment at: lldb/source/Target/Statistics.cpp:228
   {"modules", std::move(json_modules)},
+  {"strings", string_stats.ToJSON()},
   {"totalSymbolTableParseTime", symtab_parse_time},

"constStrings" maybe? or "stringPool"? "strings" makes it seem like we are 
tracking all strings in LLDB

Do we want another top level key here for "memory" that we can slowly add 
things to? It would be nice to know how much memory symbols tables consume, 
DWARF data structures like the DIE vectors, and many others things. So maybe 
adding a "memory" key/value pair at the top here might be a good idea?

```
"memory": {"strings": ..., "symtab": ..., "dwarf":  }
```


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

https://reviews.llvm.org/D117914

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

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/Utility/ConstString.h:415-416
+size_t GetBytesWasted() const { return total_memory - bytes_allocated; }
+size_t total_memory;
+size_t bytes_allocated;
+  };




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

https://reviews.llvm.org/D117914

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

[Lldb-commits] [PATCH] D117490: [lldb] Make logging machinery type-safe

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/Utility/Log.h:57
+  /// };
+  using MaskType = uint32_t;
+

JDevlieghere wrote:
> Didn't this all start with Greg wanting to make this a `uint64_t`?
yes, uint64_t please!



Comment at: lldb/include/lldb/Utility/Logging.h:56-86
+#define LIBLLDB_LOG_PROCESS ::lldb_private::LLDBLog::Process
+#define LIBLLDB_LOG_THREAD ::lldb_private::LLDBLog::Thread
+#define LIBLLDB_LOG_DYNAMIC_LOADER ::lldb_private::LLDBLog::DynamicLoader
+#define LIBLLDB_LOG_EVENTS ::lldb_private::LLDBLog::Events
+#define LIBLLDB_LOG_BREAKPOINTS ::lldb_private::LLDBLog::Breakpoints
+#define LIBLLDB_LOG_WATCHPOINTS ::lldb_private::LLDBLog::Watchpoints
+#define LIBLLDB_LOG_STEP ::lldb_private::LLDBLog::Step

I know we need these for now, but it would be great to get rid of these in 
follow up patches



Comment at: lldb/source/Plugins/Process/POSIX/ProcessPOSIXLog.h:29-35
+#define POSIX_LOG_PROCESS ::lldb_private::POSIXLog::Process
+#define POSIX_LOG_THREAD ::lldb_private::POSIXLog::Thread
+#define POSIX_LOG_MEMORY ::lldb_private::POSIXLog::Memory
+#define POSIX_LOG_PTRACE ::lldb_private::POSIXLog::Ptrace
+#define POSIX_LOG_REGISTERS ::lldb_private::POSIXLog::Registers
+#define POSIX_LOG_BREAKPOINTS ::lldb_private::POSIXLog::Breakpoints
+#define POSIX_LOG_WATCHPOINTS ::lldb_private::POSIXLog::Watchpoints

Do we need these #define lines?



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h:32-46
+#define GDBR_LOG_PROCESS ::lldb_private::process_gdb_remote::GDBRLog::Process
+#define GDBR_LOG_THREAD ::lldb_private::process_gdb_remote::GDBRLog::Thread
+#define GDBR_LOG_PACKETS ::lldb_private::process_gdb_remote::GDBRLog::Packets
+#define GDBR_LOG_MEMORY ::lldb_private::process_gdb_remote::GDBRLog::Memory
+#define GDBR_LOG_MEMORY_DATA_SHORT 
\
+  ::lldb_private::process_gdb_remote::GDBRLog::MemoryDataShort
+#define GDBR_LOG_MEMORY_DATA_LONG  
\

Do we need these anymore?



Comment at: lldb/unittests/Utility/LogTest.cpp:34-38
+static Log::Channel test_channel(test_categories, TestChannel::FOO);
+
+namespace lldb_private {
+template <> Log::Channel &LogChannelFor() { return test_channel; }
+} // namespace lldb_private

Should we make a macro for this in Log.h? Something that would define the log 
channel global variable, and make the template for us?

```
LLDB_CREATE_LOG(test_categories, TestChannel, TestChannel::FOO);
```
which would expand to the above code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117490

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

[Lldb-commits] [PATCH] D117490: [lldb] Make logging machinery type-safe

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I do like the direction of this patch BTW!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117490

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

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Target/Statistics.cpp:228
   {"modules", std::move(json_modules)},
+  {"strings", string_stats.ToJSON()},
   {"totalSymbolTableParseTime", symtab_parse_time},

clayborg wrote:
> "constStrings" maybe? or "stringPool"? "strings" makes it seem like we are 
> tracking all strings in LLDB
> 
> Do we want another top level key here for "memory" that we can slowly add 
> things to? It would be nice to know how much memory symbols tables consume, 
> DWARF data structures like the DIE vectors, and many others things. So maybe 
> adding a "memory" key/value pair at the top here might be a good idea?
> 
> ```
> "memory": {"strings": ..., "symtab": ..., "dwarf":  }
> ```
I like that


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

https://reviews.llvm.org/D117914

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

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 402093.
JDevlieghere marked 3 inline comments as done.
JDevlieghere added a comment.

Put string statistics under memory:

  "memory": {
"strings": {
  "bytesAllocated": 2056916,
  "bytesTotal": 3145728,
  "bytesUnallocated": 1088812
}
  },


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

https://reviews.llvm.org/D117914

Files:
  lldb/include/lldb/Target/Statistics.h
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Target/Statistics.cpp
  lldb/source/Utility/ConstString.cpp
  lldb/test/API/commands/statistics/basic/TestStats.py

Index: lldb/test/API/commands/statistics/basic/TestStats.py
===
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -135,6 +135,7 @@
 
 (lldb) statistics dump
 {
+  "memory" : {...},
   "modules" : [...],
   "targets" : [
 {
@@ -160,6 +161,7 @@
 target = self.createTestTarget()
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'memory',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
@@ -197,6 +199,7 @@
 
 (lldb) statistics dump
 {
+  "memory" : {...},
   "modules" : [...],
   "targets" : [
 {
@@ -227,6 +230,7 @@
   lldb.SBFileSpec("main.c"))
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'memory',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
@@ -254,6 +258,44 @@
 self.assertGreater(stats['launchOrAttachTime'], 0.0)
 self.assertGreater(stats['targetCreateTime'], 0.0)
 
+def test_memory(self):
+"""
+Test "statistics dump" and the memory information.
+"""
+exe = self.getBuildArtifact("a.out")
+target = self.createTestTarget(file_path=exe)
+debug_stats = self.get_stats()
+debug_stat_keys = [
+'memory',
+'modules',
+'targets',
+'totalSymbolTableParseTime',
+'totalSymbolTableIndexTime',
+'totalSymbolTablesLoadedFromCache',
+'totalSymbolTablesSavedToCache',
+'totalDebugInfoParseTime',
+'totalDebugInfoIndexTime',
+'totalDebugInfoIndexLoadedFromCache',
+'totalDebugInfoIndexSavedToCache',
+'totalDebugInfoByteSize'
+]
+self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
+
+memory = debug_stats['memory']
+memory_keys= [
+'strings',
+]
+self.verify_keys(memory, '"memory"', memory_keys, None)
+
+strings = memory['strings']
+strings_keys= [
+'bytesTotal',
+'bytesAllocated',
+'bytesUnallocated',
+]
+self.verify_keys(strings, '"strings"', strings_keys, None)
+
+
 def find_module_in_metrics(self, path, stats):
 modules = stats['modules']
 for module in modules:
@@ -269,6 +311,7 @@
 target = self.createTestTarget(file_path=exe)
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'memory',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
@@ -312,6 +355,7 @@
 Output expected to be something like:
 
 {
+  "memory" : {...},
   "modules" : [...],
   "targets" : [
 {
@@ -355,6 +399,7 @@
 self.runCmd("b a_function")
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'memory',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -171,6 +171,17 @@
 return mem_size;
   }
 
+  ConstString::MemoryStats GetMemoryStats() const {
+ConstString::MemoryStats stats;
+for (const auto &pool : m_string_pools) {
+  llvm::sys::SmartScopedReader rlock(pool.m_mutex);
+  const Allocator &alloc = pool.m_string_map.getAllocator();
+  stats.bytes_total += alloc.getTotalMemory();
+  stats.bytes_allocated += alloc.getBytesAllocated();
+}
+return stats;
+  }
+
 protected:
   uint8_t hash(const llvm::StringRef &s) const {
 uint32_t h = llvm::djbHash(s);
@@ -332,6 +343,10 @@
   return StringPool().MemorySize();
 }
 
+ConstString::MemoryStats ConstString::GetMemoryStats() {
+  return StringPool().GetMemoryStats();
+}
+
 void llvm::format_provider::format(const ConstString &CS,
 llvm::raw_ostream &OS,
 

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

I find "Unallocated" ambiguous. To the Allocator, maybe it's not, but to VM it 
is. What about Total/UsedUnused? Note that the docs for `BytesAllocated` say it 
can be used to calculated "wasted" space.

  /// How many bytes we've allocated.
  ///
  /// Used so that we can compute how much space was wasted.
  size_t BytesAllocated = 0;


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

https://reviews.llvm.org/D117914

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

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D117914#3262467 , @kastiglione 
wrote:

> I find "Unallocated" ambiguous. To the Allocator, maybe it's not, but to VM 
> it is. What about Total/Used/Unused? Note that the docs for `BytesAllocated` 
> say it can be used to calculated "wasted" space.
>
>   /// How many bytes we've allocated.
>   ///
>   /// Used so that we can compute how much space was wasted.
>   size_t BytesAllocated = 0;

I got the original terminology from the dumpStats function on the 
BumpPtrAllocator. I like Total/Used/Unused because there's less ambiguity.


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

https://reviews.llvm.org/D117914

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

[Lldb-commits] [PATCH] D117914: [lldb] Add ConstString memory usage statistics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 402104.
JDevlieghere added a comment.

  "memory": {
"strings": {
  "bytesTotal": 3145728,
  "bytesUnused": 1088812,
  "bytesUsed": 2056916
}
  },


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

https://reviews.llvm.org/D117914

Files:
  lldb/include/lldb/Target/Statistics.h
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Target/Statistics.cpp
  lldb/source/Utility/ConstString.cpp
  lldb/test/API/commands/statistics/basic/TestStats.py

Index: lldb/test/API/commands/statistics/basic/TestStats.py
===
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -135,6 +135,7 @@
 
 (lldb) statistics dump
 {
+  "memory" : {...},
   "modules" : [...],
   "targets" : [
 {
@@ -160,6 +161,7 @@
 target = self.createTestTarget()
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'memory',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
@@ -197,6 +199,7 @@
 
 (lldb) statistics dump
 {
+  "memory" : {...},
   "modules" : [...],
   "targets" : [
 {
@@ -227,6 +230,7 @@
   lldb.SBFileSpec("main.c"))
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'memory',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
@@ -254,6 +258,44 @@
 self.assertGreater(stats['launchOrAttachTime'], 0.0)
 self.assertGreater(stats['targetCreateTime'], 0.0)
 
+def test_memory(self):
+"""
+Test "statistics dump" and the memory information.
+"""
+exe = self.getBuildArtifact("a.out")
+target = self.createTestTarget(file_path=exe)
+debug_stats = self.get_stats()
+debug_stat_keys = [
+'memory',
+'modules',
+'targets',
+'totalSymbolTableParseTime',
+'totalSymbolTableIndexTime',
+'totalSymbolTablesLoadedFromCache',
+'totalSymbolTablesSavedToCache',
+'totalDebugInfoParseTime',
+'totalDebugInfoIndexTime',
+'totalDebugInfoIndexLoadedFromCache',
+'totalDebugInfoIndexSavedToCache',
+'totalDebugInfoByteSize'
+]
+self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
+
+memory = debug_stats['memory']
+memory_keys= [
+'strings',
+]
+self.verify_keys(memory, '"memory"', memory_keys, None)
+
+strings = memory['strings']
+strings_keys= [
+'bytesTotal',
+'bytesUsed',
+'bytesUnused',
+]
+self.verify_keys(strings, '"strings"', strings_keys, None)
+
+
 def find_module_in_metrics(self, path, stats):
 modules = stats['modules']
 for module in modules:
@@ -269,6 +311,7 @@
 target = self.createTestTarget(file_path=exe)
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'memory',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
@@ -312,6 +355,7 @@
 Output expected to be something like:
 
 {
+  "memory" : {...},
   "modules" : [...],
   "targets" : [
 {
@@ -355,6 +399,7 @@
 self.runCmd("b a_function")
 debug_stats = self.get_stats()
 debug_stat_keys = [
+'memory',
 'modules',
 'targets',
 'totalSymbolTableParseTime',
Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -171,6 +171,17 @@
 return mem_size;
   }
 
+  ConstString::MemoryStats GetMemoryStats() const {
+ConstString::MemoryStats stats;
+for (const auto &pool : m_string_pools) {
+  llvm::sys::SmartScopedReader rlock(pool.m_mutex);
+  const Allocator &alloc = pool.m_string_map.getAllocator();
+  stats.bytes_total += alloc.getTotalMemory();
+  stats.bytes_used += alloc.getBytesAllocated();
+}
+return stats;
+  }
+
 protected:
   uint8_t hash(const llvm::StringRef &s) const {
 uint32_t h = llvm::djbHash(s);
@@ -332,6 +343,10 @@
   return StringPool().MemorySize();
 }
 
+ConstString::MemoryStats ConstString::GetMemoryStats() {
+  return StringPool().GetMemoryStats();
+}
+
 void llvm::format_provider::format(const ConstString &CS,
 llvm::raw_ostream &OS,
 llvm::StringRef Options) {
Index: lldb/source/Target/Statistics.cpp
=

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

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

ljmf00 created this revision.
ljmf00 added a project: LLDB.
Herald added subscribers: JDevlieghere, pengfei.
ljmf00 requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch disables the following tests on non-AVX machines:

- `lldb-shell :: Register/x86-64-write.test`
- `lldb-shell :: Register/x86-mm-xmm-write.test`

This is due to a bug on GDB that prevents writing some XMM registers on
those machines https://sourceware.org/bugzilla/show_bug.cgi?id=28803 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117928

Files:
  lldb/test/Shell/Register/x86-64-write.test
  lldb/test/Shell/Register/x86-mm-xmm-write.test


Index: lldb/test/Shell/Register/x86-mm-xmm-write.test
===
--- lldb/test/Shell/Register/x86-mm-xmm-write.test
+++ lldb/test/Shell/Register/x86-mm-xmm-write.test
@@ -1,5 +1,9 @@
 # XFAIL: system-darwin
 # XFAIL: system-windows
+
+# Bug on GDB https://sourceware.org/bugzilla/show_bug.cgi?id=28803
+# XFAIL: !native-cpu-avx
+
 # REQUIRES: native && (target-x86 || target-x86_64) && native-cpu-sse
 # RUN: %clangxx_host %p/Inputs/x86-mm-xmm-write.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
Index: lldb/test/Shell/Register/x86-64-write.test
===
--- lldb/test/Shell/Register/x86-64-write.test
+++ lldb/test/Shell/Register/x86-64-write.test
@@ -1,5 +1,9 @@
 # XFAIL: system-darwin
 # XFAIL: system-windows
+
+# Bug on GDB https://sourceware.org/bugzilla/show_bug.cgi?id=28803
+# XFAIL: !native-cpu-avx
+
 # REQUIRES: native && target-x86_64
 # RUN: %clangxx_host %p/Inputs/x86-64-write.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s


Index: lldb/test/Shell/Register/x86-mm-xmm-write.test
===
--- lldb/test/Shell/Register/x86-mm-xmm-write.test
+++ lldb/test/Shell/Register/x86-mm-xmm-write.test
@@ -1,5 +1,9 @@
 # XFAIL: system-darwin
 # XFAIL: system-windows
+
+# Bug on GDB https://sourceware.org/bugzilla/show_bug.cgi?id=28803
+# XFAIL: !native-cpu-avx
+
 # REQUIRES: native && (target-x86 || target-x86_64) && native-cpu-sse
 # RUN: %clangxx_host %p/Inputs/x86-mm-xmm-write.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
Index: lldb/test/Shell/Register/x86-64-write.test
===
--- lldb/test/Shell/Register/x86-64-write.test
+++ lldb/test/Shell/Register/x86-64-write.test
@@ -1,5 +1,9 @@
 # XFAIL: system-darwin
 # XFAIL: system-windows
+
+# Bug on GDB https://sourceware.org/bugzilla/show_bug.cgi?id=28803
+# XFAIL: !native-cpu-avx
+
 # REQUIRES: native && target-x86_64
 # RUN: %clangxx_host %p/Inputs/x86-64-write.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

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

ljmf00 added inline comments.



Comment at: lldb/test/Shell/Register/x86-mm-xmm-write.test:1-2
 # XFAIL: system-darwin
 # XFAIL: system-windows
+

Maybe those XFAILs are related to this too. Can anyone test this on those 
systems, if you have a CPU with AVX extensions? I don't know the rationale, 
although.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117928

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

[Lldb-commits] [PATCH] D117707: [lldb] [Platform] Support synthesizing siginfo_t

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

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

Turn `PlatformLinuxTest` into a more generic `PlatformSiginfoTest`, to improve 
code reuse between platforms. Start preparing for testing FreeBSD.


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

https://reviews.llvm.org/D117707

Files:
  lldb/bindings/interface/SBPlatform.i
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/Platform.cpp
  lldb/unittests/Platform/CMakeLists.txt
  lldb/unittests/Platform/PlatformSiginfoTest.cpp
  lldb/unittests/Platform/tools/generate_siginfo.c

Index: lldb/unittests/Platform/tools/generate_siginfo.c
===
--- /dev/null
+++ lldb/unittests/Platform/tools/generate_siginfo.c
@@ -0,0 +1,63 @@
+//===-- generate_siginfo_linux.c --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include 
+#include 
+#include 
+
+siginfo_t siginfo;
+
+#define P(member)  \
+  printf("   {\"%s\", %zd, %zd},\n", #member,   \
+ offsetof(siginfo_t, member), sizeof(siginfo.member));
+
+// undef annoying glibc macros
+#undef si_pid
+#undef si_uid
+#undef si_overrun
+#undef si_status
+#undef si_utime
+#undef si_stime
+#undef si_addr
+#undef si_addr_lsb
+#undef si_band
+#undef si_fd
+
+int main() {
+  printf("  ExpectFields(siginfo_type,\n");
+  printf("   {\n");
+  P(si_signo);
+  P(si_errno);
+  P(si_code);
+  P(_sifields._kill.si_pid);
+  P(_sifields._kill.si_uid);
+  P(_sifields._timer.si_tid);
+  P(_sifields._timer.si_overrun);
+  P(_sifields._timer.si_sigval);
+  P(_sifields._rt.si_pid);
+  P(_sifields._rt.si_uid);
+  P(_sifields._rt.si_sigval);
+  P(_sifields._sigchld.si_pid);
+  P(_sifields._sigchld.si_uid);
+  P(_sifields._sigchld.si_status);
+  P(_sifields._sigchld.si_utime);
+  P(_sifields._sigchld.si_stime);
+  P(_sifields._sigfault.si_addr);
+  P(_sifields._sigfault.si_addr_lsb);
+  P(_sifields._sigfault._bounds._addr_bnd._lower);
+  P(_sifields._sigfault._bounds._addr_bnd._upper);
+  P(_sifields._sigfault._bounds._pkey);
+  P(_sifields._sigpoll.si_band);
+  P(_sifields._sigpoll.si_fd);
+  P(_sifields._sigsys._call_addr);
+  P(_sifields._sigsys._syscall);
+  P(_sifields._sigsys._arch);
+  printf("   });\n");
+
+  return 0;
+}
Index: lldb/unittests/Platform/PlatformSiginfoTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/PlatformSiginfoTest.cpp
@@ -0,0 +1,281 @@
+//===-- PlatformSiginfoTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include 
+#include 
+
+#include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::repro;
+
+namespace {
+class PlatformSiginfoTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+  PlatformSP platform_sp;
+  DebuggerSP debugger_sp;
+  TargetSP target_sp;
+
+public:
+  CompilerType siginfo_type;
+
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+platform_freebsd::PlatformFreeBSD::Initialize();
+platform_linux::PlatformLinux::Initialize();
+  }
+
+  void TearDown() override {
+platform_linux::PlatformLinux::Terminate();
+platform_freebsd::PlatformFreeBSD::Terminate();
+Reproducer::Terminate();
+  }
+
+  typedef std::tuple field_tuple;
+
+  void ExpectField(const CompilerType &siginfo_type, field_tuple field) {
+const char *path;
+uint64_t offset, size;
+std::tie(path, offset, size) = field;
+
+SCOPED_TRACE(path);
+CompilerType field_type = siginfo_type;
+uint64_t total_offset = 0;
+for (auto field_name : llvm::split(path, '.')) {
+  uint64_t bit_offset;
+  ASSERT_NE(field_type.GetIndexOfFieldW

[Lldb-commits] [PATCH] D117707: [lldb] [Platform] Support synthesizing siginfo_t

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

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

Add tests for FreeBSD (no implementation yet).


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

https://reviews.llvm.org/D117707

Files:
  lldb/bindings/interface/SBPlatform.i
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/Platform.cpp
  lldb/unittests/Platform/CMakeLists.txt
  lldb/unittests/Platform/PlatformSiginfoTest.cpp
  lldb/unittests/Platform/tools/generate_siginfo.c

Index: lldb/unittests/Platform/tools/generate_siginfo.c
===
--- /dev/null
+++ lldb/unittests/Platform/tools/generate_siginfo.c
@@ -0,0 +1,82 @@
+//===-- generate_siginfo_linux.c --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include 
+#include 
+#include 
+
+siginfo_t siginfo;
+
+#define P(member)  \
+  printf("   {\"%s\", %zd, %zd},\n", #member,   \
+ offsetof(siginfo_t, member), sizeof(siginfo.member));
+
+// undef annoying "POSIX friendliness" macros
+#undef si_pid
+#undef si_uid
+#undef si_overrun
+#undef si_status
+#undef si_utime
+#undef si_stime
+#undef si_addr
+#undef si_addr_lsb
+#undef si_band
+#undef si_fd
+
+int main() {
+  printf("  ExpectFields(siginfo_type,\n");
+  printf("   {\n");
+
+  P(si_signo);
+  P(si_errno);
+  P(si_code);
+
+#if defined(__GLIBC__)
+  P(_sifields._kill.si_pid);
+  P(_sifields._kill.si_uid);
+  P(_sifields._timer.si_tid);
+  P(_sifields._timer.si_overrun);
+  P(_sifields._timer.si_sigval);
+  P(_sifields._rt.si_pid);
+  P(_sifields._rt.si_uid);
+  P(_sifields._rt.si_sigval);
+  P(_sifields._sigchld.si_pid);
+  P(_sifields._sigchld.si_uid);
+  P(_sifields._sigchld.si_status);
+  P(_sifields._sigchld.si_utime);
+  P(_sifields._sigchld.si_stime);
+  P(_sifields._sigfault.si_addr);
+  P(_sifields._sigfault.si_addr_lsb);
+  P(_sifields._sigfault._bounds._addr_bnd._lower);
+  P(_sifields._sigfault._bounds._addr_bnd._upper);
+  P(_sifields._sigfault._bounds._pkey);
+  P(_sifields._sigpoll.si_band);
+  P(_sifields._sigpoll.si_fd);
+  P(_sifields._sigsys._call_addr);
+  P(_sifields._sigsys._syscall);
+  P(_sifields._sigsys._arch);
+#endif // defined(__GLIBC__)
+
+#if defined(__FreeBSD__)
+  // these are top-level fields on FreeBSD
+  P(si_pid);
+  P(si_uid);
+  P(si_status);
+  P(si_addr);
+  P(si_value);
+  P(_reason._fault._trapno);
+  P(_reason._timer._timerid);
+  P(_reason._timer._overrun);
+  P(_reason._mesgq._mqd);
+  P(_reason._poll._band);
+#endif // defined(__FreeBSD__)
+
+  printf("   });\n");
+
+  return 0;
+}
Index: lldb/unittests/Platform/PlatformSiginfoTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/PlatformSiginfoTest.cpp
@@ -0,0 +1,344 @@
+//===-- PlatformSiginfoTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include 
+#include 
+
+#include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::repro;
+
+namespace {
+class PlatformSiginfoTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+  PlatformSP platform_sp;
+  DebuggerSP debugger_sp;
+  TargetSP target_sp;
+
+public:
+  CompilerType siginfo_type;
+
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+platform_freebsd::PlatformFreeBSD::Initialize();
+platform_linux::PlatformLinux::Initialize();
+  }
+
+  void TearDown() override {
+platform_linux::PlatformLinux::Terminate();
+platform_freebsd::PlatformFreeBSD::Terminate();
+Reproducer::Terminate();
+  }
+
+  typedef std::tuple field_tuple;
+
+  void ExpectField(const CompilerType &siginfo_type, field_tuple field) {
+const char *path;

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Why are we disabling a lldb test because of a bug in GDB?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117928

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

[Lldb-commits] [PATCH] D117707: [lldb] [Platform] Support synthesizing siginfo_t

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

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

Implemented FreeBSD.


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

https://reviews.llvm.org/D117707

Files:
  lldb/bindings/interface/SBPlatform.i
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/Platform.cpp
  lldb/unittests/Platform/CMakeLists.txt
  lldb/unittests/Platform/PlatformSiginfoTest.cpp
  lldb/unittests/Platform/tools/generate_siginfo.c

Index: lldb/unittests/Platform/tools/generate_siginfo.c
===
--- /dev/null
+++ lldb/unittests/Platform/tools/generate_siginfo.c
@@ -0,0 +1,82 @@
+//===-- generate_siginfo_linux.c --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include 
+#include 
+#include 
+
+siginfo_t siginfo;
+
+#define P(member)  \
+  printf("   {\"%s\", %zd, %zd},\n", #member,   \
+ offsetof(siginfo_t, member), sizeof(siginfo.member));
+
+// undef annoying "POSIX friendliness" macros
+#undef si_pid
+#undef si_uid
+#undef si_overrun
+#undef si_status
+#undef si_utime
+#undef si_stime
+#undef si_addr
+#undef si_addr_lsb
+#undef si_band
+#undef si_fd
+
+int main() {
+  printf("  ExpectFields(siginfo_type,\n");
+  printf("   {\n");
+
+  P(si_signo);
+  P(si_errno);
+  P(si_code);
+
+#if defined(__GLIBC__)
+  P(_sifields._kill.si_pid);
+  P(_sifields._kill.si_uid);
+  P(_sifields._timer.si_tid);
+  P(_sifields._timer.si_overrun);
+  P(_sifields._timer.si_sigval);
+  P(_sifields._rt.si_pid);
+  P(_sifields._rt.si_uid);
+  P(_sifields._rt.si_sigval);
+  P(_sifields._sigchld.si_pid);
+  P(_sifields._sigchld.si_uid);
+  P(_sifields._sigchld.si_status);
+  P(_sifields._sigchld.si_utime);
+  P(_sifields._sigchld.si_stime);
+  P(_sifields._sigfault.si_addr);
+  P(_sifields._sigfault.si_addr_lsb);
+  P(_sifields._sigfault._bounds._addr_bnd._lower);
+  P(_sifields._sigfault._bounds._addr_bnd._upper);
+  P(_sifields._sigfault._bounds._pkey);
+  P(_sifields._sigpoll.si_band);
+  P(_sifields._sigpoll.si_fd);
+  P(_sifields._sigsys._call_addr);
+  P(_sifields._sigsys._syscall);
+  P(_sifields._sigsys._arch);
+#endif // defined(__GLIBC__)
+
+#if defined(__FreeBSD__)
+  // these are top-level fields on FreeBSD
+  P(si_pid);
+  P(si_uid);
+  P(si_status);
+  P(si_addr);
+  P(si_value);
+  P(_reason._fault._trapno);
+  P(_reason._timer._timerid);
+  P(_reason._timer._overrun);
+  P(_reason._mesgq._mqd);
+  P(_reason._poll._band);
+#endif // defined(__FreeBSD__)
+
+  printf("   });\n");
+
+  return 0;
+}
Index: lldb/unittests/Platform/PlatformSiginfoTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/PlatformSiginfoTest.cpp
@@ -0,0 +1,344 @@
+//===-- PlatformSiginfoTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include 
+#include 
+
+#include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::repro;
+
+namespace {
+class PlatformSiginfoTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+  PlatformSP platform_sp;
+  DebuggerSP debugger_sp;
+  TargetSP target_sp;
+
+public:
+  CompilerType siginfo_type;
+
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+platform_freebsd::PlatformFreeBSD::Initialize();
+platform_linux::PlatformLinux::Initialize();
+  }
+
+  void TearDown() override {
+platform_linux::PlatformLinux::Terminate();
+platform_freebsd::PlatformFreeBSD::Terminate();
+Reproducer::Terminate();
+  }
+
+  typedef std::tuple field_tuple;
+
+  void 

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D117928#3262656 , @JDevlieghere 
wrote:

> Why are we disabling a lldb test because of a bug in GDB?

Maybe somebody is running the lldb testsuite with gdbserver?  If so, we should 
make an xfail category for gdbserver and xfail with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117928

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

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

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

ljmf00 added a comment.

In D117928#3262656 , @JDevlieghere 
wrote:

> Why are we disabling a lldb test because of a bug in GDB?

Because GDB is used on systems that don't implement a `RegisterContext`. I 
added an inline comment about Darwin and Windows XFAIL, as I'm not sure if this 
is just Linux specific. See here 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp#L296
 . Take a look at the issue. You can also reproduce this with `register 
write`/`register read` right after.

This ended up breaking my testsuite for a long time, since my server has an 
x86-64-v2 CPU, instead of an x86-64-v3. The buildbots seem to have at least 
x86-64-v3. As described by the issue I tested this on an AVX capable machine 
(x86-64-v3) and virtualized an older CPU to reproduce in a clean environment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117928

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

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

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

ljmf00 added a comment.

In D117928#3262689 , @jingham wrote:

> In D117928#3262656 , @JDevlieghere 
> wrote:
>
>> Why are we disabling a lldb test because of a bug in GDB?
>
> Maybe somebody is running the lldb testsuite with gdbserver?  That seems a 
> legit thing to do, but if so, we should make an xfail category for gdbserver 
> and xfail with that.

I haven't considered that. Shouldn't the testsuite run with lldb-server anyway? 
I think so, but how can I confirm that for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117928

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

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D117928#3262708 , @ljmf00 wrote:

> In D117928#3262689 , @jingham wrote:
>
>> In D117928#3262656 , @JDevlieghere 
>> wrote:
>>
>>> Why are we disabling a lldb test because of a bug in GDB?
>>
>> Maybe somebody is running the lldb testsuite with gdbserver?  That seems a 
>> legit thing to do, but if so, we should make an xfail category for gdbserver 
>> and xfail with that.
>
> I haven't considered that. Shouldn't the testsuite run with lldb-server 
> anyway? I think so, but how can I confirm that for you?

I guess we're just all confused that you are mentioning GDB at all, and the 
only thing I could think of is gdbserver, since that's the only way anything 
GDB could get mixed with anything LLDB...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117928

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

[Lldb-commits] [PATCH] D117837: [lldb] Fix timer logging inverted quiet condition

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG58ee14e29e98: [lldb] Fix timer logging inverted quiet 
condition (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117837

Files:
  lldb/source/Utility/Timer.cpp


Index: lldb/source/Utility/Timer.cpp
===
--- lldb/source/Utility/Timer.cpp
+++ lldb/source/Utility/Timer.cpp
@@ -63,7 +63,7 @@
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 
 // Indent
@@ -89,7 +89,7 @@
   Signposts->endInterval(this, m_category.GetName());
 
   TimerStack &stack = GetTimerStackForCurrentThread();
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 ::fprintf(stdout, "%*s%.9f sec (%.9f sec)\n",
   int(stack.size() - 1) * TIMER_INDENT_AMOUNT, "",


Index: lldb/source/Utility/Timer.cpp
===
--- lldb/source/Utility/Timer.cpp
+++ lldb/source/Utility/Timer.cpp
@@ -63,7 +63,7 @@
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 
 // Indent
@@ -89,7 +89,7 @@
   Signposts->endInterval(this, m_category.GetName());
 
   TimerStack &stack = GetTimerStackForCurrentThread();
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 ::fprintf(stdout, "%*s%.9f sec (%.9f sec)\n",
   int(stack.size() - 1) * TIMER_INDENT_AMOUNT, "",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 58ee14e - [lldb] Fix timer logging inverted quiet condition

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2022-01-21T15:34:07-08:00
New Revision: 58ee14e29e98384bba6d2e1c1789b7f8e3060d24

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

LOG: [lldb] Fix timer logging inverted quiet condition

The logic of `g_quiet` was inverted in D26243. This corrects the issue.

Without this, running `log timers enable` produces a high volume of incremental
timer output.

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

Added: 


Modified: 
lldb/source/Utility/Timer.cpp

Removed: 




diff  --git a/lldb/source/Utility/Timer.cpp b/lldb/source/Utility/Timer.cpp
index 2f3afe4c87037..b190f35007d50 100644
--- a/lldb/source/Utility/Timer.cpp
+++ b/lldb/source/Utility/Timer.cpp
@@ -63,7 +63,7 @@ Timer::Timer(Timer::Category &category, const char *format, 
...)
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 
 // Indent
@@ -89,7 +89,7 @@ Timer::~Timer() {
   Signposts->endInterval(this, m_category.GetName());
 
   TimerStack &stack = GetTimerStackForCurrentThread();
-  if (g_quiet && stack.size() <= g_display_depth) {
+  if (!g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());
 ::fprintf(stdout, "%*s%.9f sec (%.9f sec)\n",
   int(stack.size() - 1) * TIMER_INDENT_AMOUNT, "",



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

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

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

mgorny added a comment.

I'm sorry but I don't understand what you're talking about. Could you explain 
how are you running tests? Is there some way we can reproduce the problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117928

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

[Lldb-commits] [PATCH] D117259: [lldb] Allow aliases to aliases to command-regex

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 402132.
kastiglione added a comment.

add a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117259

Files:
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/test/API/commands/command/nested_alias/TestNestedAlias.py


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias ls', check=False)
+self.runCmd('command unalias sh', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,7 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias sh platform shell -h --')
+self.expect('command alias ls sh ls')
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias ls', check=False)
+self.runCmd('command unalias sh', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,7 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias sh platform shell -h --')
+self.expect('command alias ls sh ls')
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117259: [lldb] Allow aliases to aliases of raw input commands

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

That tests that we made the alias, but not that it actually works.  If you did 
something like:

command alias start_with_two expr -- 2
command alias add_two start_with_two +

then

add_two 3

should produce 5, which would be easy to test for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117259

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

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

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

ljmf00 added a comment.

In D117928#3262710 , @jingham wrote:

> I guess we're just all confused that you are mentioning GDB at all, and the 
> only thing I could think of is gdbserver, since that's the only way anything 
> GDB could get mixed with anything LLDB...

Ok, I will try to give some context. I can be wrong/confused and this can be 
unrelated to GDB and a coincidence with my observable behaviour, but running 
LLDB with default settings, `register write` command triggers the following 
function 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp#L296
 .

Interestingly, I just did a test right now: removed GDB from my machine and did 
a clean build. That function (`GDBRemoteRegisterContext::WriteRegister`) still 
triggers when `register write`. I'm a bit confused now on why this is 
triggered. Maybe there is a fallback system? I don't have any guidance on this 
whatsoever, so my view on how those plugins work can be wrong. Would be cool to 
have a brief explanation about the //pipeline// of `RegisterContext` and how it 
interacts with `GDBRemoteRegisterContext` to be in sync.

Can be confusing to add a link to the GDB bug tracker, but one thing I can be 
sure of is that this is dependent on the CPU and by the observed behaviour, the 
presence of AVX or some other extension introduced on x86_64-v2 related to XMM 
registers (can't think of another one other than SSE). I can also describe my 
testing environment and give the libvirt/qemu parameters I added to virtualize 
the CPU flags, if needed, but the CPU configuration is similar to the models I 
described. I ran LLDB on a clean archiso live installation.

In D117928#3262762 , @mgorny wrote:

> I'm sorry but I don't understand what you're talking about. Could you explain 
> how are you running tests? Is there some way we can reproduce the problem?

You can see a build log I posted on discourse: 
https://ipfs.io/ipfs/bafybeidsf745d4qxrzhbbir4h23ov4rppvo2uke7qh75oj6lfoi6zkcyre/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117928

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

Re: [Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

[Jim Ingham via lldb-commits]

Ah, I think your confusion is that you missed the “Remote” part of all the 
classes in lldb that start with “GDB”.   They are so called because they use 
the “gdb remote serial protocol” to communicate with the debug monitor, not 
because they have anything to do with gdb the debugger.  That’s a protocol for 
communication between a debugger and a debug monitor of some sort, devised by 
people working on gdb aeons ago:

https://sourceware.org/gdb/onlinedocs/gdb/Remote-Protocol.html

In lldb’s case, that debug monitor can be debugserver (the default on Darwin 
systems) lldb-server (the default on Linux systems) or anything else that 
speaks the gdb remote protocol (a lot of JTAG parts come with a gdb remote 
protocol stub) including potentially gdb’s implementation of the monitor: 
gdbserver...

But there’s no shared code between gdb & lldb, just a shared communication 
protocol.

Jim

> On Jan 21, 2022, at 4:34 PM, Luís Ferreira via Phabricator 
>  wrote:
> 
> ljmf00 added a comment.
> 
> In D117928#3262710 , @jingham wrote:
> 
>> I guess we're just all confused that you are mentioning GDB at all, and the 
>> only thing I could think of is gdbserver, since that's the only way anything 
>> GDB could get mixed with anything LLDB...
> 
> Ok, I will try to give some context. I can be wrong/confused and this can be 
> unrelated to GDB and a coincidence with my observable behaviour, but running 
> LLDB with default settings, `register write` command triggers the following 
> function 
> https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp#L296
>  .
> 
> Interestingly, I just did a test right now: removed GDB from my machine and 
> did a clean build. That function (`GDBRemoteRegisterContext::WriteRegister`) 
> still triggers when `register write`. I'm a bit confused now on why this is 
> triggered. Maybe there is a fallback system? I don't have any guidance on 
> this whatsoever, so my view on how those plugins work can be wrong. Would be 
> cool to have a brief explanation about the //pipeline// of `RegisterContext` 
> and how it interacts with `GDBRemoteRegisterContext` to be in sync.
> 
> Can be confusing to add a link to the GDB bug tracker, but one thing I can be 
> sure of is that this is dependent on the CPU and by the observed behaviour, 
> the presence of AVX or some other extension introduced on x86_64-v2 related 
> to XMM registers (can't think of another one other than SSE). I can also 
> describe my testing environment and give the libvirt/qemu parameters I added 
> to virtualize the CPU flags, if needed, but the CPU configuration is similar 
> to the models I described. I ran LLDB on a clean archiso live installation.
> 
> In D117928#3262762 , @mgorny wrote:
> 
>> I'm sorry but I don't understand what you're talking about. Could you 
>> explain how are you running tests? Is there some way we can reproduce the 
>> problem?
> 
> You can see a build log I posted on discourse: 
> https://ipfs.io/ipfs/bafybeidsf745d4qxrzhbbir4h23ov4rppvo2uke7qh75oj6lfoi6zkcyre/
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D117928/new/
> 
> https://reviews.llvm.org/D117928
> 

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

[Lldb-commits] [PATCH] D117259: [lldb] Allow aliases to aliases of raw input commands

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 402143.
kastiglione edited the summary of this revision.
kastiglione added a comment.

use Jim's testing improvement


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117259

Files:
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/test/API/commands/command/nested_alias/TestNestedAlias.py


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias add_two', check=False)
+self.runCmd('command unalias two', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,8 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias two expr -- 2')
+self.expect('command alias add_two two +')
+self.expect('add_two 3', patterns=[' = 5$'])
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias add_two', check=False)
+self.runCmd('command unalias two', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,8 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias two expr -- 2')
+self.expect('command alias add_two two +')
+self.expect('add_two 3', patterns=[' = 5$'])
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117259: [lldb] Allow aliases to aliases of raw input commands

[Jim Ingham via Phabricator via lldb-commits]

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

What an excellent test!

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117259

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

[Lldb-commits] [lldb] b951504 - [lldb] Allow aliases to aliases of raw input commands

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2022-01-21T17:57:34-08:00
New Revision: b95150418fb6e2d22a0bd84abcdc1f3cc7e5a0bf

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

LOG: [lldb] Allow aliases to aliases of raw input commands

Allow users to create aliases for aliases to raw input commands. That probably
sounds convoluted, so here's an example:

```
command alias some-setup env SOMEVAR=SOMEVALUE
```

This an alias based on `env`, which itself is an alias for `_regex-env`.
`_regex-env` is a `command regex` command, which takes raw input.

The above `some-setup` alias fails with:

```
error: Unable to create requested alias.
```

This change allows such aliases to be created. lldb already supports aliases to
aliases for parsed commands.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectCommands.cpp
lldb/test/API/commands/command/nested_alias/TestNestedAlias.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectCommands.cpp 
b/lldb/source/Commands/CommandObjectCommands.cpp
index 1ec54cf7ededa..defa21af7c170 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@ rather than using a positional placeholder:"
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(

diff  --git a/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py 
b/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
index d4fc99492a698..bbe9c14f69f6d 100644
--- a/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ b/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@ def cleanup():
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias add_two', check=False)
+self.runCmd('command unalias two', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,8 @@ def cleanup():
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias two expr -- 2')
+self.expect('command alias add_two two +')
+self.expect('add_two 3', patterns=[' = 5$'])



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

[Lldb-commits] [PATCH] D117259: [lldb] Allow aliases to aliases of raw input commands

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb95150418fb6: [lldb] Allow aliases to aliases of raw input 
commands (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117259

Files:
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/test/API/commands/command/nested_alias/TestNestedAlias.py


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias add_two', check=False)
+self.runCmd('command unalias two', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,8 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias two expr -- 2')
+self.expect('command alias add_two two +')
+self.expect('add_two 3', patterns=[' = 5$'])
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -46,6 +46,8 @@
 self.runCmd('command unalias rd', check=False)
 self.runCmd('command unalias fo', check=False)
 self.runCmd('command unalias foself', check=False)
+self.runCmd('command unalias add_two', check=False)
+self.runCmd('command unalias two', check=False)
 
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
@@ -96,3 +98,8 @@
 'Show variables for the current',
 'stack frame.'],
 matching=True)
+
+# Check that aliases can be created for raw input commands.
+self.expect('command alias two expr -- 2')
+self.expect('command alias add_two two +')
+self.expect('add_two 3', patterns=[' = 5$'])
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -485,8 +485,9 @@
 OptionArgVectorSP option_arg_vector_sp =
 OptionArgVectorSP(new OptionArgVector);
 
-if (CommandObjectSP cmd_obj_sp =
-m_interpreter.GetCommandSPExact(cmd_obj.GetCommandName())) {
+const bool include_aliases = true;
+if (CommandObjectSP cmd_obj_sp = m_interpreter.GetCommandSPExact(
+cmd_obj.GetCommandName(), include_aliases)) {
   if (m_interpreter.AliasExists(alias_command) ||
   m_interpreter.UserCommandExists(alias_command)) {
 result.AppendWarningWithFormat(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

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

ljmf00 added a comment.

In D117928#3262874 , @jingham wrote:

> Ah, I think your confusion is that you missed the “Remote” part of all the 
> classes in lldb that start with “GDB”.   They are so called because they use 
> the “gdb remote serial protocol” to communicate with the debug monitor, not 
> because they have anything to do with gdb the debugger.  That’s a protocol 
> for communication between a debugger and a debug monitor of some sort, 
> devised by people working on gdb aeons ago:
>
> https://sourceware.org/gdb/onlinedocs/gdb/Remote-Protocol.html
>
> In lldb’s case, that debug monitor can be debugserver (the default on Darwin 
> systems) lldb-server (the default on Linux systems) or anything else that 
> speaks the gdb remote protocol (a lot of JTAG parts come with a gdb remote 
> protocol stub) including potentially gdb’s implementation of the monitor: 
> gdbserver...
>
> But there’s no shared code between gdb & lldb, just a shared communication 
> protocol.
>
> Jim

Thanks, this makes much more sense in my brain and gives me much more control 
to debug this. I'm digging a bit more. I'm tracing Linux kernel syscalls to 
check if data is sent correctly. At first glance, it seems fine to me, so maybe 
it's a `ptrace` issue? I need to investigate a bit more to say that tho. 
`strace` say me this:

  ptrace(PTRACE_SETREGSET, 29772, NT_FPREGSET, {iov_base={cwd=0x37f, swd=0, 
ftw=0, fop=0, rip=0, rdp=0, mxcsr=0x1f80, mxcr_mask=0, st_space=[0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0], xmm_space=[0xa090807, 0xe0d0c0b, 0x1211100f, 0x16151413, 0xa090807, 
0xe0d0c0b, 0x1211100f, 0x16151413, 0xa090807, 0xe0d0c0b, 0x1211100f, 
0x16151413, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ...], 
padding=[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0]}, iov_len=512}) = 0
  ptrace(PTRACE_GETREGSET, 29772, NT_FPREGSET, {iov_base={cwd=0x37f, swd=0, 
ftw=0, fop=0, rip=0, rdp=0, mxcsr=0x1f80, mxcr_mask=0, st_space=[0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0], xmm_space=[0xa090807, 0xe0d0c0b, 0x1211100f, 0x16151413, 0xa090807, 
0xe0d0c0b, 0x1211100f, 0x16151413, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, ...], padding=[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]}, iov_len=512}) = 0

First call when setting xmm2 and second call when getting the current registers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117928

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