[Lldb-commits] [lldb] 25fa678 - [lldb] Skip variant/optional libc++ tests for Clang 5/6

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-06-17T09:52:09+02:00
New Revision: 25fa67868b36c99d2704bd291b3b495737f16f0e

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

LOG: [lldb] Skip variant/optional libc++ tests for Clang 5/6

Clang 5 and Clang 6 can no longer parse newer versions of libc++. As we can't
specify the specific libc++ version in the decorator, let's only allow Clang
versions that can parse all currently available libc++ versions.

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
index f013d02d14f7..27c8d7f474ed 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
@@ -15,8 +15,9 @@ class LibcxxOptionalDataFormatterTestCase(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @add_test_categories(["libc++"])
-## We are skipping clang version less that 5.0 since this test requires 
-std=c++17
-@skipIf(oslist=no_match(["macosx"]), compiler="clang", 
compiler_version=['<', '5.0'])
+## Clang 7.0 is the oldest Clang that can reliably parse newer libc++ 
versions
+## with -std=c++17.
+@skipIf(oslist=no_match(["macosx"]), compiler="clang", 
compiler_version=['<', '7.0'])
 ## We are skipping gcc version less that 5.1 since this test requires 
-std=c++17
 @skipIf(compiler="gcc", compiler_version=['<', '5.1'])
 

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
index cea4fd6bf1c3..181dca772493 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
@@ -14,8 +14,9 @@ class LibcxxVariantDataFormatterTestCase(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @add_test_categories(["libc++"])
-## We are skipping clang version less that 5.0 since this test requires 
-std=c++17
-@skipIf(oslist=no_match(["macosx"]), compiler="clang", 
compiler_version=['<', '5.0'])
+## Clang 7.0 is the oldest Clang that can reliably parse newer libc++ 
versions
+## with -std=c++17.
+@skipIf(oslist=no_match(["macosx"]), compiler="clang", 
compiler_version=['<', '7.0'])
 ## We are skipping gcc version less that 5.1 since this test requires 
-std=c++17
 @skipIf(compiler="gcc", compiler_version=['<', '5.1'])
 ## std::get is unavailable for std::variant before macOS 10.14



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

[Lldb-commits] [PATCH] D102757: [lldb] Remove non address bits when looking up memory regions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/source/Target/Process.cpp:5854
+  if (auto abi = GetABI()) {
+// TODO: can we always assume data addresses here?
+load_addr = abi->FixDataAddress(load_addr);

omjavaid wrote:
> Yes this is ok for now as both Data and Code masks are same in case of Linux. 
> If anything changes in future we ll take care of that at that time. 
Cool. I was also thinking that if they were different you could just apply both 
masks. Since the virtual address size will be the same for either type of 
pointer.

I'll remove this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102757

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

[Lldb-commits] [PATCH] D104437: Add test for functions with extended characters.

[Raphael Isemann via Phabricator via lldb-commits]

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

Thanks for writing tests, it's really appreciated! FWIW, I think that you can 
just check in new tests without having to go through a full review (unless you 
do want feedback for it).

Having said that, would it be possible to maybe just extend the 
TestUnicodeSymbols.py test for the breakpoint part? You could just try setting 
a breakpoint and see if it gets resolved, this way the test doesn't have to 
start a new process (which is really expensive). There is also already a test 
for unicode variables (`TestUnicodeInVariable.py`)so this is redundant IIUC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104437

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

[Lldb-commits] [PATCH] D103626: [lldb][AArch64] Remove non address bits from memory read arguments

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Maybe this is something worth considering for LLDB memory dumps?

I'm working on that at the moment, it's on my github branch. This is what it 
looks like with the right options: (others look weird at the moment due to 
ordering issues)

  (lldb) memory read mte_buf mte_buf+32 -f "x" -l 1 -s 16
  0xf7ff6000: 0x (tag: 0x0)
  0xf7ff6010: 0x (tag: 0x1)

The format is mostly a guess on my part, given that I'm mostly testing the 
debugger itself not MTE enabled software.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103626

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

[Lldb-commits] [PATCH] D97281: [lldb][AArch64] Add class for managing memory tags

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

@DavidSpickett Do you have any further plans for this and other patches in the 
series. I was wondering if there is nothing to add we can go ahead and merge 
this series.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97281

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

[Lldb-commits] [PATCH] D97281: [lldb][AArch64] Add class for managing memory tags

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I am going to go over them this afternoon. I'm about to land some lldb patches 
that allow some easy refactoring, and your ABI patch might be useable too. If 
using the ABI method is better as a follow up then we can get started landing 
them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97281

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

[Lldb-commits] [PATCH] D103172: [lldb][NFC] Allow range-based for loops over DWARFDIE's children

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 352666.
teemperor added a comment.
Herald added a subscriber: mgorny.

- Added a unit test (thanks Shafik!)


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

https://reviews.llvm.org/D103172

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
  lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
===
--- /dev/null
+++ lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
@@ -0,0 +1,105 @@
+//===-- DWARFDIETest.cpp --=---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/SymbolFile/DWARF/DWARFDIE.h"
+#include "TestingSupport/Symbol/YAMLModuleTester.h"
+#include "llvm/ADT/STLExtras.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(DWARFDIETest, ChildIteration) {
+  // Tests DWARFDIE::child_iterator.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_yes
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Code:0x0002
+  Tag: DW_TAG_base_type
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_encoding
+  Form:DW_FORM_data1
+- Attribute:   DW_AT_byte_size
+  Form:DW_FORM_data1
+  debug_info:
+- Version: 4
+  AddrSize:8
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0007 # DW_ATE_unsigned
+- Value:   0x0004
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0007 # DW_ATE_unsigned
+- Value:   0x0008
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0005 # DW_ATE_signed
+- Value:   0x0008
+- AbbrCode:0x
+)";
+
+  YAMLModuleTester t(yamldata);
+  ASSERT_TRUE((bool)t.GetDwarfUnit());
+
+  DWARFUnit *unit = t.GetDwarfUnit();
+  const DWARFDebugInfoEntry *die_first = unit->DIE().GetDIE();
+
+  // Create a DWARFDIE that has three DW_TAG_base_type children.
+  DWARFDIE top_die(unit, die_first);
+
+  // Create the iterator range that has the three tags as elements.
+  llvm::iterator_range children = top_die.children();
+
+  // Compare begin() to the first child DIE.
+  DWARFDIE::child_iterator child_iter = children.begin();
+  ASSERT_NE(child_iter, children.end());
+  const DWARFDebugInfoEntry *die_child0 = die_first->GetFirstChild();
+  EXPECT_EQ((*child_iter).GetDIE(), die_child0);
+
+  // Step to the second child DIE.
+  ++child_iter;
+  ASSERT_NE(child_iter, children.end());
+  const DWARFDebugInfoEntry *die_child1 = die_child0->GetSibling();
+  EXPECT_EQ((*child_iter).GetDIE(), die_child1);
+
+  // Step to the third child DIE.
+  ++child_iter;
+  ASSERT_NE(child_iter, children.end());
+  const DWARFDebugInfoEntry *die_child2 = die_child1->GetSibling();
+  EXPECT_EQ((*child_iter).GetDIE(), die_child2);
+
+  // Step to the end of the range.
+  ++child_iter;
+  EXPECT_EQ(child_iter, children.end());
+
+  // Take one of the DW_TAG_base_type DIEs (which has no children) and make
+  // sure the children range is now empty.
+  DWARFDIE no_children_die(unit, die_child0);
+  EXPECT_TRUE(no_children_die.children().empty());
+}
Index: lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
===
--- lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
+++ lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(SymbolFileDWARFTests
   DWARFASTParserClangTests.cpp
+  DWARFDIETest.cpp
   DWARFUnitTest.cpp
   SymbolFileDWARFTests.cpp
   XcodeSDKModuleTests.cpp
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--

[Lldb-commits] [PATCH] D103172: [lldb][NFC] Allow range-based for loops over DWARFDIE's children

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h:117
+// (CU, (DIE)nullptr) == (nullptr, nullptr) -> true
+if (!m_die.IsValid() && !it.m_die.IsValid())
+  return true;

shafik wrote:
> I think:
> 
> ```
> bool operator==(const DWARFBaseDIE &lhs, const DWARFBaseDIE &rhs) {
>   return lhs.GetDIE() == rhs.GetDIE() && lhs.GetCU() == rhs.GetCU();
> }
> ```
> 
> will do the same thing but maybe I am confused.
> 
> Maybe we should have a unit test?
That's the `DWARFDIE::operator==` implementation called below and this special 
case is because of the comment one line above (we need to match the end 
iterator if it's default constructed when `GetSibling()` returns a DWARFDIE 
that has a valid CU but a `null` DIE).


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

https://reviews.llvm.org/D103172

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

[Lldb-commits] [PATCH] D104395: [LLDB][GUI] Add initial forms support

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.

> did you consider implementing each field as a Window?

My first implementation was actually done completely with fields as sub windows.
However, two things deterred me from taking that approach.

- While sub-windows in ncurses are mostly sub-spaces with their own local 
coordinate system and cursor, which would be a good abstraction to use for 
fields, their implementation in the Window class seemed more involved than 
needed, with panels for drawing even. So I thought maybe they are not supposed 
to be used in that manner. I also though about introducing a type that is more 
lightweight and suitable for this kind of thing, but that didn't seem worth it 
at the moment. I will come back to this in a future patch though.
- Field construction doesn't have access to the possible parent window needed 
to create the sub-window, which means we will either have to pass the window 
around during construction or we will have to conditionally construct it the 
first window draw. Neither of those sounded like a good method to me.

The field drawing code used some special coordinate computation anyways, so
sub-spacing there was natural to do. Moreover, I don't think the duplicated code
with the Window is a lot, a non-border box drawing function is still useful to
have so I wouldn't think of it as a duplicate code.

> how complex it the scrolling code going to be?

I will try to answer this question now to make it easier to decide what we need 
to
do. But I basically wanted full flexibility when it comes to drawing, with 
possibly
multiple row fields like the choices field and multi-column forms. The forms 
ncurses
library uses pages instead of scrolling to make this easier, so they would put 
the
most important fields in the first page and more fields in later pages. But I 
will let
you know what I think after I look into scrolling first.




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:962
+
+  int GetContentLength() { return (int)m_content.length(); }
+

clayborg wrote:
> Does this need to be an integer? Can we return "size_t"?
The compiler kept throwing sign comparison warnings in expressions like 
`m_cursor_position < m_content.length()`, so I just casted to an integer. How 
should I properly handle this?



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1120
+switch (key) {
+case ' ':
+  ToggleContent();

clayborg wrote:
> enter or return key as well? or is that saved for submit in the main window?
Sure, we can do that. Enter is only handled if the button is selected.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1442
+  // The index of the selected field. This can be equal to the number of 
fields,
+  // in which case, it denotes that the button is selected.
+  int m_selected_field_index;

clayborg wrote:
> The submit button?
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104395

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

[Lldb-commits] [PATCH] D104380: [lldb] Set return object failed status even if error string is empty

[David Spickett via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG983ed1b58ef9: [lldb] Set return object failed status even if 
error string is empty (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104380

Files:
  lldb/source/Interpreter/CommandReturnObject.cpp


Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -98,9 +98,9 @@
 }
 
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
+  SetStatus(eReturnStatusFailed);
   if (in_string.empty())
 return;
-  SetStatus(eReturnStatusFailed);
   error(GetErrorStream()) << in_string.rtrim() << '\n';
 }
 
@@ -113,6 +113,7 @@
 }
 
 void CommandReturnObject::SetError(llvm::StringRef error_str) {
+  SetStatus(eReturnStatusFailed);
   if (error_str.empty())
 return;
 
@@ -123,10 +124,10 @@
 // append "\n" to the end of it.
 
 void CommandReturnObject::AppendRawError(llvm::StringRef in_string) {
+  SetStatus(eReturnStatusFailed);
   if (in_string.empty())
 return;
   GetErrorStream() << in_string;
-  SetStatus(eReturnStatusFailed);
 }
 
 void CommandReturnObject::SetStatus(ReturnStatus status) { m_status = status; }


Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -98,9 +98,9 @@
 }
 
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
+  SetStatus(eReturnStatusFailed);
   if (in_string.empty())
 return;
-  SetStatus(eReturnStatusFailed);
   error(GetErrorStream()) << in_string.rtrim() << '\n';
 }
 
@@ -113,6 +113,7 @@
 }
 
 void CommandReturnObject::SetError(llvm::StringRef error_str) {
+  SetStatus(eReturnStatusFailed);
   if (error_str.empty())
 return;
 
@@ -123,10 +124,10 @@
 // append "\n" to the end of it.
 
 void CommandReturnObject::AppendRawError(llvm::StringRef in_string) {
+  SetStatus(eReturnStatusFailed);
   if (in_string.empty())
 return;
   GetErrorStream() << in_string;
-  SetStatus(eReturnStatusFailed);
 }
 
 void CommandReturnObject::SetStatus(ReturnStatus status) { m_status = status; }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 983ed1b - [lldb] Set return object failed status even if error string is empty

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-06-17T12:20:52+01:00
New Revision: 983ed1b58ef9d0f97c9cec2876f631e47609d437

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

LOG: [lldb] Set return object failed status even if error string is empty

The idea is now that AppendError<...> will set eReturnStatusFailed
for you so you don't have to call SetStatus again.

Previously if the error message was empty, the status wouldn't
be set.

I don't think there are any sitautions where the message is in
fact empty but it potentially could be depending on where
we get the string from.

So let's set the status up front then return early if the message is empty.

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/source/Interpreter/CommandReturnObject.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandReturnObject.cpp 
b/lldb/source/Interpreter/CommandReturnObject.cpp
index 531c1f246bd86..5edd9a3af9968 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -98,9 +98,9 @@ void CommandReturnObject::AppendWarning(llvm::StringRef 
in_string) {
 }
 
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
+  SetStatus(eReturnStatusFailed);
   if (in_string.empty())
 return;
-  SetStatus(eReturnStatusFailed);
   error(GetErrorStream()) << in_string.rtrim() << '\n';
 }
 
@@ -113,6 +113,7 @@ void CommandReturnObject::SetError(const Status &error,
 }
 
 void CommandReturnObject::SetError(llvm::StringRef error_str) {
+  SetStatus(eReturnStatusFailed);
   if (error_str.empty())
 return;
 
@@ -123,10 +124,10 @@ void CommandReturnObject::SetError(llvm::StringRef 
error_str) {
 // append "\n" to the end of it.
 
 void CommandReturnObject::AppendRawError(llvm::StringRef in_string) {
+  SetStatus(eReturnStatusFailed);
   if (in_string.empty())
 return;
   GetErrorStream() << in_string;
-  SetStatus(eReturnStatusFailed);
 }
 
 void CommandReturnObject::SetStatus(ReturnStatus status) { m_status = status; }



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

[Lldb-commits] [lldb] 7a580f3 - [lldb] Remove redundant calls to set eReturnStatusFailed

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-06-17T12:21:54+01:00
New Revision: 7a580f3c28cf47a7e489faa1fc1ab7b88d9a5dbd

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

LOG: [lldb] Remove redundant calls to set eReturnStatusFailed

Since https://reviews.llvm.org/D103701 AppendError<...>
sets this for you.

This change includes all of the non-command uses.

Some uses remain where it's either tricky to reason about
the logic, or they aren't paired with AppendError calls.

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/source/API/SBCommandInterpreter.cpp
lldb/source/Breakpoint/BreakpointIDList.cpp
lldb/source/Interpreter/CommandAlias.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/Options.cpp
lldb/source/Interpreter/ScriptInterpreter.cpp

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp

Removed: 




diff  --git a/lldb/source/API/SBCommandInterpreter.cpp 
b/lldb/source/API/SBCommandInterpreter.cpp
index d28bc411042c1..b4a69c3e972a4 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -185,7 +185,6 @@ lldb::ReturnStatus SBCommandInterpreter::HandleCommand(
   } else {
 result->AppendError(
 "SBCommandInterpreter or the command line is not valid");
-result->SetStatus(eReturnStatusFailed);
   }
 
   return result.GetStatus();
@@ -203,7 +202,6 @@ void SBCommandInterpreter::HandleCommandsFromFile(
 
   if (!IsValid()) {
 result->AppendError("SBCommandInterpreter is not valid.");
-result->SetStatus(eReturnStatusFailed);
 return;
   }
 
@@ -211,7 +209,6 @@ void SBCommandInterpreter::HandleCommandsFromFile(
 SBStream s;
 file.GetDescription(s);
 result->AppendErrorWithFormat("File is not valid: %s.", s.GetData());
-result->SetStatus(eReturnStatusFailed);
   }
 
   FileSpec tmp_spec = file.ref();
@@ -439,7 +436,6 @@ void SBCommandInterpreter::ResolveCommand(const char 
*command_line,
   } else {
 result->AppendError(
 "SBCommandInterpreter or the command line is not valid");
-result->SetStatus(eReturnStatusFailed);
   }
 }
 
@@ -469,7 +465,6 @@ void SBCommandInterpreter::SourceInitFileInHomeDirectory(
 m_opaque_ptr->SourceInitFileHome(result.ref());
   } else {
 result->AppendError("SBCommandInterpreter is not valid");
-result->SetStatus(eReturnStatusFailed);
   }
 }
 
@@ -487,7 +482,6 @@ void SBCommandInterpreter::SourceInitFileInHomeDirectory(
 m_opaque_ptr->SourceInitFileHome(result.ref(), is_repl);
   } else {
 result->AppendError("SBCommandInterpreter is not valid");
-result->SetStatus(eReturnStatusFailed);
   }
 }
 
@@ -506,7 +500,6 @@ void 
SBCommandInterpreter::SourceInitFileInCurrentWorkingDirectory(
 m_opaque_ptr->SourceInitFileCwd(result.ref());
   } else {
 result->AppendError("SBCommandInterpreter is not valid");
-result->SetStatus(eReturnStatusFailed);
   }
 }
 

diff  --git a/lldb/source/Breakpoint/BreakpointIDList.cpp 
b/lldb/source/Breakpoint/BreakpointIDList.cpp
index e6a5dceeb93b2..172674dc2dd2d 100644
--- a/lldb/source/Breakpoint/BreakpointIDList.cpp
+++ b/lldb/source/Breakpoint/BreakpointIDList.cpp
@@ -141,7 +141,6 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args 
&old_args, Target *target,
   if (!error.Success()) {
 new_args.Clear();
 result.AppendError(error.AsCString());
-result.SetStatus(eReturnStatusFailed);
 return;
   } else
 names_found.insert(std::string(current_arg));
@@ -170,7 +169,6 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args 
&old_args, Target *target,
 new_args.Clear();
 result.AppendErrorWithFormat("'%d' is not a valid breakpoint 
ID.\n",
  bp_id->GetBreakpointID());
-result.SetStatus(eReturnStatusFailed);
 return;
   }
   const size_t num_locations = breakpoint_sp->GetNumLocations();
@@ -199,7 +197,6 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args 
&old_args, Target *target,
   new_args.Clear();
   result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n",
range_from.str().c_str());
-  result.SetStatus(eReturnStatusFailed);
   return;
 }
 
@@ -208,7 +205,6 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args 
&old_args, Target *target,
   new_args.Clear();
   res

[Lldb-commits] [PATCH] D104379: [lldb] Remove redundant calls to set eReturnStatusFailed

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7a580f3c28cf: [lldb] Remove redundant calls to set 
eReturnStatusFailed (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104379

Files:
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/source/Breakpoint/BreakpointIDList.cpp
  lldb/source/Interpreter/CommandAlias.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/Options.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp

Index: lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
===
--- lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -809,7 +809,6 @@
StructuredDataDarwinLog::GetStaticPluginName())) {
   result.AppendError("failed to get StructuredDataPlugin for "
  "the process");
-  result.SetStatus(eReturnStatusFailed);
 }
 StructuredDataDarwinLog &plugin =
 *static_cast(plugin_sp.get());
@@ -833,7 +832,6 @@
 // Report results.
 if (!error.Success()) {
   result.AppendError(error.AsCString());
-  result.SetStatus(eReturnStatusFailed);
   // Our configuration failed, so we're definitely disabled.
   plugin.SetEnabled(false);
 } else {
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -880,7 +880,6 @@
 if (argc > 0) {
   result.AppendErrorWithFormat("'%s' take no arguments, only options",
m_cmd_name.c_str());
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
 SetDefaultOptionsIfNoneAreSet();
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -5246,7 +5246,6 @@
"amount to be transferred when "
"reading/writing",
m_cmd_name.c_str());
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
 
@@ -5287,7 +5286,6 @@
   result.AppendErrorWithFormat(
   "'%s' takes a one or more packet content arguments",
   m_cmd_name.c_str());
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
 
@@ -5337,7 +5335,6 @@
 if (command.empty()) {
   result.AppendErrorWithFormat("'%s' takes a command string argument",
m_cmd_name.c_str());
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
 
Index: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
===
--- lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -910,7 +910,6 @@
   if (!m_command_byte.GetOptionValue().OptionWasSet()) {
 result.AppendError(
 "the --command option must be set to a valid command byte");
-result.SetStatus(eReturnStatusFailed);
   } else {
 const uint64_t command_byte =
 m_command_byte.GetOptionValue().GetUInt64Value(0);
@@ -933,7 +932,6 @@
"even number of ASCII hex "
"characters: '%s'",
ascii_hex_bytes_cstr);
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
 payload_bytes.resize(ascii_hex_bytes_cstr_len / 2);
@@ -943,7 +941,6 @@
"ASCII hex characters (no "
"spaces or hex prefixes): '%s'",
ascii_hex_bytes_cstr);
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
   }
@@ -970,30 +967,25 @@
 else
   result.AppendErrorWithFormat("unknown error 0x%8.8x",
 

[Lldb-commits] [PATCH] D104448: [lldb] Remove redundant calls to set eReturnStatusFailed

[Raphael Isemann via Phabricator via lldb-commits]

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

There are two places where this isn't redundant (see inline comments) but 
otherwise this LGTM. Thanks!




Comment at: lldb/source/Commands/CommandObjectWatchpoint.cpp:892
 if (command.GetArgumentCount() <= 0) {
   result.GetErrorStream().Printf("error: required argument missing; "
  "specify your program variable to watch "

`AppendErrorWithFormat` otherwise the status isn't changed.



Comment at: lldb/source/Commands/CommandObjectWatchpoint.cpp:915
 if (command.GetArgumentCount() != 1) {
   result.GetErrorStream().Printf(
   "error: specify exactly one variable to watch for\n");

Same as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104448

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

[Lldb-commits] [PATCH] D104448: [lldb] Remove redundant calls to set eReturnStatusFailed

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM now, just a small nit about the redundant "error: " string.




Comment at: lldb/source/Commands/CommandObjectWatchpoint.cpp:879
 if (command.GetArgumentCount() <= 0) {
-  result.GetErrorStream().Printf("error: required argument missing; "
- "specify your program variable to watch "
- "for\n");
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError("error: required argument missing; "
+ "specify your program variable to watch "

`AppendError` adds the `error: ` prefix so it shouldn't be in the passed message



Comment at: lldb/source/Commands/CommandObjectWatchpoint.cpp:901
 if (command.GetArgumentCount() != 1) {
-  result.GetErrorStream().Printf(
-  "error: specify exactly one variable to watch for\n");
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError("error: specify exactly one variable to watch for\n");
   return false;

here too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104448

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

[Lldb-commits] [PATCH] D97281: [lldb][AArch64] Add class for managing memory tags

[David Spickett via Phabricator via lldb-commits]

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

Rebase onto main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97281

Files:
  lldb/include/lldb/Target/MemoryTagManager.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
  lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp

Index: lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
@@ -0,0 +1,120 @@
+//===-- MemoryTagManagerAArch64MTETest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(MemoryTagManagerAArch64MTETest, UnpackTagsData) {
+  MemoryTagManagerAArch64MTE manager;
+
+  // Error for insufficient tag data
+  std::vector input;
+  ASSERT_THAT_EXPECTED(
+  manager.UnpackTagsData(input, 2),
+  llvm::FailedWithMessage(
+  "Packed tag data size does not match expected number of tags. "
+  "Expected 2 tag(s) for 2 granules, got 0 tag(s)."));
+
+  // This is out of the valid tag range
+  input.push_back(0x1f);
+  ASSERT_THAT_EXPECTED(
+  manager.UnpackTagsData(input, 1),
+  llvm::FailedWithMessage(
+  "Found tag 0x1f which is > max MTE tag value of 0xf."));
+
+  // MTE tags are 1 per byte
+  input.pop_back();
+  input.push_back(0xe);
+  input.push_back(0xf);
+
+  std::vector expected{0xe, 0xf};
+
+  llvm::Expected> got =
+  manager.UnpackTagsData(input, 2);
+  ASSERT_THAT_EXPECTED(got, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*got));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, GetLogicalTag) {
+  MemoryTagManagerAArch64MTE manager;
+
+  // Set surrounding bits to check shift is correct
+  ASSERT_EQ((lldb::addr_t)0, manager.GetLogicalTag(0xe0e0));
+  // Max tag value
+  ASSERT_EQ((lldb::addr_t)0xf, manager.GetLogicalTag(0x0f00));
+  ASSERT_EQ((lldb::addr_t)2, manager.GetLogicalTag(0x0200));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, ExpandToGranule) {
+  MemoryTagManagerAArch64MTE manager;
+  // Reading nothing, no alignment needed
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(0, 0),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0, 0)));
+
+  // Ranges with 0 size are unchanged even if address is non 0
+  // (normally 0x1234 would be aligned to 0x1230)
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(0x1234, 0),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0x1234, 0)));
+
+  // Ranges already aligned don't change
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(0x100, 64),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0x100, 64)));
+
+  // Any read of less than 1 granule is rounded up to reading 1 granule
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(0, 16),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0, 1)));
+
+  // Start address is aligned down, and length modified accordingly
+  // Here bytes 8 through 24 straddle 2 granules. So the resulting range starts
+  // at 0 and covers 32 bytes.
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(0, 32),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(8, 16)));
+
+  // Here only the size of the range needs aligning
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(16, 32),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(16, 24)));
+
+  // Start and size need aligning here but we only need 1 granule to cover it
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(16, 16),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(18, 4)));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, RemoveNonAddressBits) {
+  MemoryTagManagerAArch64MTE manager;
+
+  ASSERT_EQ(0, 0);
+  ASSERT_EQ((lldb::addr_t)0x00ffeedd11223344,
+manager.RemoveNonAddressBits(0x00ffeedd11223344));
+  ASSERT_EQ((lldb::addr_t)0x,
+manager.RemoveNonAddressBits(0xFF00));
+  ASSERT_EQ((lldb::addr_t)0x0055,
+manager.RemoveNonAddressBits(0xee55));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, AddressDiff) {
+  MemoryTagManagerAArch64MTE

[Lldb-commits] [PATCH] D97281: [lldb][AArch64] Add class for managing memory tags

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

@omjavaid As it stands, RemoveNonAddressBits here removes the top byte 
unconditionally.

You asked for pointer signatures to be removed as well, which would be neatly 
solved by using the ABI plugin. Ok to land this as is, then write a follow up 
to migrate to using the ABI plugin?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97281

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

[Lldb-commits] [PATCH] D97282: [lldb][AArch64] Add memory-tagging qSupported feature

[David Spickett via Phabricator via lldb-commits]

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

Rebase, this is good to land as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97282

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Target/Process.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -1025,6 +1025,14 @@
 self.assertEqual(supported_dict.get('fork-events', '-'), '-')
 self.assertEqual(supported_dict.get('vfork-events', '-'), '-')
 
+# We need to be able to self.runCmd to get cpuinfo,
+# which is not possible when using a remote platform.
+@skipIfRemote
+def test_qSupported_memory_tagging(self):
+supported_dict = self.get_qSupported_dict()
+self.assertEqual(supported_dict.get("memory-tagging", '-'),
+ '+' if self.isAArch64MTE() else '-')
+
 @skipIfWindows # No pty support to test any inferior output
 def test_written_M_content_reads_back_correctly(self):
 self.build()
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -235,6 +235,8 @@
   friend class GDBRemoteCommunicationClient;
   friend class GDBRemoteRegisterContext;
 
+  bool SupportsMemoryTagging() override;
+
   /// Broadcaster event bits definitions.
   enum {
 eBroadcastBitAsyncContinue = (1 << 0),
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2767,6 +2767,10 @@
   return 0;
 }
 
+bool ProcessGDBRemote::SupportsMemoryTagging() {
+  return m_gdb_comm.GetMemoryTaggingSupported();
+}
+
 Status ProcessGDBRemote::WriteObjectFile(
 std::vector entries) {
   Status error;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3608,6 +3608,8 @@
 ret.push_back("qXfer:auxv:read+");
   if (bool(plugin_features & Extension::libraries_svr4))
 ret.push_back("qXfer:libraries-svr4:read+");
+  if (bool(plugin_features & Extension::memory_tagging))
+ret.push_back("memory-tagging+");
 
   // check for client features
   m_extensions_supported = {};
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -451,6 +451,8 @@
 
   bool GetSharedCacheInfoSupported();
 
+  bool GetMemoryTaggingSupported();
+
   /// Use qOffsets to query the offset used when relocating the target
   /// executable. If successful, the returned structure will contain at least
   /// one value in the offsets field.
@@ -558,6 +560,7 @@
   LazyBool m_supports_QPassSignals = eLazyBoolCalculate;
   LazyBool m_supports_error_string_reply = eLazyBoolCalculate;
   LazyBool m_supports_multiprocess = eLazyBoolCalculate;
+  LazyBool m_supports_memory_tagging = eLazyBoolCalculate;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
   m_supports_qUserName : 1, m_supports_qGroupName : 1,
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -309,6 +309,7 @@
   m_supports_multiprocess = eLazyBoolNo;
   m_supports_qEcho = eLazyBoolNo;
   m_supports_QPassSignals = eLazyBoolNo;
+  m_supports_memory_tagging = eLazyBoolNo;
 
   m_max_packet_size = UINT64_MAX; // It's supposed to always be

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server

[David Spickett via Phabricator via lldb-commits]

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

Rebase, this is now ready to land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Host/linux/Ptrace.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/memory-tagging/Makefile
  lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
  lldb/test/API/tools/lldb-server/memory-tagging/main.c

Index: lldb/test/API/tools/lldb-server/memory-tagging/main.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/memory-tagging/main.c
@@ -0,0 +1,55 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int print_result(char *ptr) {
+  // Page size allows the test to try reading off of the end of the page
+  printf("buffer: %p page_size: 0x%x\n", ptr, sysconf(_SC_PAGESIZE));
+
+  // Exit after some time, so we don't leave a zombie process
+  // if the test framework lost track of us.
+  sleep(60);
+  return 0;
+}
+
+int main(int argc, char const *argv[]) {
+  if (prctl(PR_SET_TAGGED_ADDR_CTRL,
+PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC |
+// Allow all tags to be generated by the addg
+// instruction __arm_mte_increment_tag produces.
+(0x << PR_MTE_TAG_SHIFT),
+0, 0, 0)) {
+return print_result(NULL);
+  }
+
+  size_t page_size = sysconf(_SC_PAGESIZE);
+  char *buf = mmap(0, page_size, PROT_READ | PROT_WRITE | PROT_MTE,
+   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (buf == MAP_FAILED)
+return print_result(NULL);
+
+  // Set incrementing tags until end of the page
+  char *tagged_ptr = buf;
+  // This intrinsic treats the addresses as if they were untagged
+  while (__arm_mte_ptrdiff(tagged_ptr, buf) < page_size) {
+// This sets the allocation tag
+__arm_mte_set_tag(tagged_ptr);
+// Set the tag of the next granule (hence +16) to the next
+// tag value. Returns a new pointer with the new logical tag.
+// Tag values wrap at 0xF so it'll cycle.
+tagged_ptr = __arm_mte_increment_tag(tagged_ptr + 16, 1);
+  }
+
+  // lldb-server should be removing the top byte from addresses passed
+  // to ptrace. So put some random bits in there.
+  // ptrace expects you to remove them but it can still succeed if you
+  // don't. So this isn't proof that we're removing them, it's just a
+  // smoke test in case something didn't account for them.
+  buf = (char *)((size_t)buf | ((size_t)0xAA << 56));
+  return print_result(buf);
+}
Index: lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
@@ -0,0 +1,116 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestGdbRemoteMemoryTagging(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def check_qmemtags_response(self, body, expected):
+self.test_sequence.add_log_lines(["read packet: $qMemTags:{}#00".format(body),
+  "send packet: ${}#00".format(expected),
+  ],
+ True)
+self.expect_gdbremote_sequence()
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_qmemtags_packets(self):
+""" Test that qMemTags packets are parsed correctly and/or rejected. """
+
+self.build()
+self.set_inferior_startup_launch()
+procs = self.prep_debug_monitor_and_inferior()
+
+# Run the process
+self.test_sequence.add_log_lines(
+[
+# Start running after initial stop
+"read packet: $c#63",
+		# Match the address of the MTE page
+{"type": "output_match",
+ "regex": self.maybe_strict_output_regex(r"buffer: (.+) page_size: (.+)\r\n"),
+ "captur

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading to lldb

[David Spickett via Phabricator via lldb-commits]

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

Rebase, now ready to land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95602

Files:
  lldb/include/lldb/Core/Architecture.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp
  lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
  lldb/source/Plugins/Architecture/AArch64/CMakeLists.txt
  lldb/source/Plugins/Architecture/CMakeLists.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Process.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -466,3 +466,68 @@
   EXPECT_EQ(llvm::None, GetQOffsets("TextSeg=0x1234"));
   EXPECT_EQ(llvm::None, GetQOffsets("TextSeg=12345678123456789"));
 }
+
+static void
+check_qmemtags(TestClient &client, MockServer &server, size_t read_len,
+   const char *packet, llvm::StringRef response,
+   llvm::Optional> expected_tag_data) {
+  const auto &ReadMemoryTags = [&](size_t len, const char *packet,
+   llvm::StringRef response) {
+std::future result = std::async(std::launch::async, [&] {
+  return client.ReadMemoryTags(0xDEF0, read_len, 1);
+});
+
+HandlePacket(server, packet, response);
+return result.get();
+  };
+
+  auto result = ReadMemoryTags(0, packet, response);
+  if (expected_tag_data) {
+ASSERT_TRUE(result);
+llvm::ArrayRef expected(*expected_tag_data);
+llvm::ArrayRef got = result->GetData();
+ASSERT_THAT(expected, testing::ContainerEq(got));
+  } else {
+ASSERT_FALSE(result);
+  }
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, ReadMemoryTags) {
+  // Zero length reads are valid
+  check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m",
+ std::vector{});
+
+  // The client layer does not check the length of the received data.
+  // All we need is the "m" and for the decode to use all of the chars
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m09",
+ std::vector{0x9});
+
+  // Zero length response is fine as long as the "m" is present
+  check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m",
+ std::vector{});
+
+  // Normal responses
+  check_qmemtags(client, server, 16, "qMemTags:def0,10:1", "m66",
+ std::vector{0x66});
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m0102",
+ std::vector{0x1, 0x2});
+
+  // Empty response is an error
+  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "", llvm::None);
+  // Usual error response
+  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "E01", llvm::None);
+  // Leading m missing
+  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "01", llvm::None);
+  // Anything other than m is an error
+  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "z01", llvm::None);
+  // Decoding tag data doesn't use all the chars in the packet
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m09zz", llvm::None);
+  // Data that is not hex bytes
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "mhello",
+ llvm::None);
+  // Data is not a complete hex char
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m9", llvm::None);
+  // Data has a trailing hex char
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m01020",
+ llvm::None);
+}
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -6065,3 +6065,84 @@
 
   return false;
 }
+
+llvm::Expected
+Process::GetMemoryTagManager(lldb::addr_t addr, lldb::addr_t end_addr) {
+  Architecture *arch = GetTarget().GetArchitecturePlugin();
+  const MemoryTagManager *tag_manager =
+  arch ? arch->GetMemoryTagManager() : nullptr;
+  if (!arch || !tag_manager) {
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"This architecture does not support memory tagging",
+GetPluginName().GetCString());
+  }
+
+  if (!SupportsMemoryTagging()) {
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Process does not support memory tagging");
+  }
+
+  ptrdiff_t len = tag_manager->AddressDiff(end_add

[Lldb-commits] [PATCH] D97285: [lldb][AArch64] Add "memory tag read" command

[David Spickett via Phabricator via lldb-commits]

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

Rebase. Remove `result.SetStatus(eReturnStatusFailed)` which
is now implicitly done when you add an error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97285

Files:
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectMemoryTag.h
  lldb/test/API/functionalities/memory/tag/Makefile
  lldb/test/API/functionalities/memory/tag/TestMemoryTag.py
  lldb/test/API/functionalities/memory/tag/main.cpp
  lldb/test/API/linux/aarch64/mte_tag_read/Makefile
  lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
  lldb/test/API/linux/aarch64/mte_tag_read/main.c

Index: lldb/test/API/linux/aarch64/mte_tag_read/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_tag_read/main.c
@@ -0,0 +1,77 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char const *argv[]) {
+  // We assume that the test runner has checked we're on an MTE system
+
+  if (prctl(PR_SET_TAGGED_ADDR_CTRL,
+PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC |
+// Allow all tags to be generated by the addg
+// instruction __arm_mte_increment_tag produces.
+(0x << PR_MTE_TAG_SHIFT),
+0, 0, 0)) {
+return 1;
+  }
+
+  size_t page_size = sysconf(_SC_PAGESIZE);
+
+  // Allocate memory with MTE
+  // We ask for two pages. One is read only so that we get
+  // 2 mappings in /proc/.../smaps so we can check reading
+  // a range across mappings.
+  // The first allocation will start at the highest address,
+  // so we allocate buf2 first to get:
+  //  | buf | buf2 | 
+  int prot = PROT_READ | PROT_MTE;
+  int flags = MAP_PRIVATE | MAP_ANONYMOUS;
+
+  char *buf2 = mmap(0, page_size, prot, flags, -1, 0);
+  if (buf2 == MAP_FAILED)
+return 1;
+
+  // Writeable so we can set tags on it later
+  char *buf = mmap(0, page_size, prot | PROT_WRITE, flags, -1, 0);
+  if (buf == MAP_FAILED)
+return 1;
+
+  // We expect the mappings to be next to each other
+  if (buf2 - buf != page_size)
+return 1;
+
+  // And without MTE
+  char *non_mte_buf = mmap(0, page_size, PROT_READ | PROT_WRITE, flags, -1, 0);
+  if (non_mte_buf == MAP_FAILED)
+return 1;
+
+  // Set incrementing tags until end of the first page
+  char *tagged_ptr = buf;
+  // This ignores tag bits when subtracting the addresses
+  while (__arm_mte_ptrdiff(tagged_ptr, buf) < page_size) {
+// Set the allocation tag for this location
+__arm_mte_set_tag(tagged_ptr);
+// + 16 for 16 byte granules
+// Earlier we allowed all tag values, so this will give us an
+// incrementing pattern 0-0xF wrapping back to 0.
+tagged_ptr = __arm_mte_increment_tag(tagged_ptr + 16, 1);
+  }
+
+  // Tag the original pointer with 9
+  buf = __arm_mte_create_random_tag(buf, ~(1 << 9));
+  // A different tag so that buf_alt_tag > buf if you don't handle the tag
+  char *buf_alt_tag = __arm_mte_create_random_tag(buf, ~(1 << 10));
+
+  // lldb should be removing the whole top byte, not just the tags.
+  // So fill 63-60 with something non zero so we'll fail if we only remove tags.
+#define SET_TOP_NIBBLE(ptr) (char *)((size_t)(ptr) | (0xA << 60))
+  buf = SET_TOP_NIBBLE(buf);
+  buf_alt_tag = SET_TOP_NIBBLE(buf_alt_tag);
+  buf2 = SET_TOP_NIBBLE(buf2);
+
+  // Breakpoint here
+  return 0;
+}
Index: lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
@@ -0,0 +1,126 @@
+"""
+Test "memory tag read" command on AArch64 Linux with MTE.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxMTEMemoryTagReadTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_mte_tag_read(self):
+if not self.isAArch64MTE():
+self.skipTest('Target must support MTE.')
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c",
+line_number('main.c', '// Breakpoint here'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOIN

[Lldb-commits] [PATCH] D104395: [LLDB][GUI] Add initial forms support

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev updated this revision to Diff 352761.
OmarEmaraDev added a comment.

- Remove PutCStringTruncatedWidth, use a character limit instead.
- Handle review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104395

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -392,6 +392,12 @@
   void Box(chtype v_char = ACS_VLINE, chtype h_char = ACS_HLINE) {
 ::box(m_window, v_char, h_char);
   }
+  void VerticalLine(int n, chtype v_char = ACS_VLINE) {
+::wvline(m_window, v_char, n);
+  }
+  void HorizontalLine(int n, chtype h_char = ACS_HLINE) {
+::whline(m_window, h_char, n);
+  }
   void Clear() { ::wclear(m_window); }
   void Erase() { ::werase(m_window); }
   Rect GetBounds() const {
@@ -674,6 +680,36 @@
   AttributeOff(attr);
   }
 
+  void DrawBox(const Rect &bounds, chtype v_char = ACS_VLINE,
+   chtype h_char = ACS_HLINE) {
+MoveCursor(bounds.origin.x, bounds.origin.y);
+VerticalLine(bounds.size.height);
+HorizontalLine(bounds.size.width);
+PutChar(ACS_ULCORNER);
+
+MoveCursor(bounds.origin.x + bounds.size.width - 1, bounds.origin.y);
+VerticalLine(bounds.size.height);
+PutChar(ACS_URCORNER);
+
+MoveCursor(bounds.origin.x, bounds.origin.y + bounds.size.height - 1);
+HorizontalLine(bounds.size.width);
+PutChar(ACS_LLCORNER);
+
+MoveCursor(bounds.origin.x + bounds.size.width - 1,
+   bounds.origin.y + bounds.size.height - 1);
+PutChar(ACS_LRCORNER);
+  }
+
+  void DrawTitledBox(const Rect &bounds, const char *title,
+ chtype v_char = ACS_VLINE, chtype h_char = ACS_HLINE) {
+DrawBox(bounds, v_char, h_char);
+int title_offset = 2;
+MoveCursor(bounds.origin.x + title_offset, bounds.origin.y);
+PutChar('[');
+PutCString(title, bounds.size.width - title_offset);
+PutChar(']');
+  }
+
   virtual void Draw(bool force) {
 if (m_delegate_sp && m_delegate_sp->WindowDelegateDraw(*this, force))
   return;
@@ -869,6 +905,545 @@
   const Window &operator=(const Window &) = delete;
 };
 
+/
+// Forms
+/
+
+class FieldDelegate {
+public:
+  virtual ~FieldDelegate() = default;
+
+  virtual Rect FieldDelegateGetBounds() = 0;
+
+  virtual void FieldDelegateDraw(Window &window, bool is_active) = 0;
+
+  virtual HandleCharResult FieldDelegateHandleChar(int key) {
+return eKeyNotHandled;
+  }
+};
+
+typedef std::shared_ptr FieldDelegateSP;
+
+class TextFieldDelegate : public FieldDelegate {
+public:
+  TextFieldDelegate(const char *label, int width, Point origin,
+const char *content)
+  : m_label(label), m_width(width), m_origin(origin), m_cursor_position(0),
+m_first_visibile_char(0) {
+if (content)
+  m_content = content;
+assert(m_width > 2);
+  }
+
+  // Get the bounding box of the field. The text field has a height of 3, 2
+  // lines for borders and 1 for the content.
+  Rect FieldDelegateGetBounds() override {
+return Rect(m_origin, Size(m_width, 3));
+  }
+
+  // Get the start X position of the content in window space, without the
+  // borders.
+  int GetX() { return m_origin.x + 1; }
+
+  // Get the start Y position of the content in window space, without the
+  // borders.
+  int GetY() { return m_origin.y + 1; }
+
+  // Get the effective width of the field, without the borders.
+  int GetEffectiveWidth() { return m_width - 2; }
+
+  // Get the cursor position in window space.
+  int GetCursorWindowXPosition() {
+return GetX() + m_cursor_position - m_first_visibile_char;
+  }
+
+  int GetContentLength() { return (int)m_content.length(); }
+
+  void FieldDelegateDraw(Window &window, bool is_active) override {
+// Draw label box.
+window.DrawTitledBox(FieldDelegateGetBounds(), m_label.c_str());
+
+// Draw content.
+window.MoveCursor(GetX(), GetY());
+const char *text = m_content.c_str() + m_first_visibile_char;
+window.PutCString(text, GetEffectiveWidth());
+
+// Highlight the cursor.
+window.MoveCursor(GetCursorWindowXPosition(), GetY());
+if (is_active)
+  window.AttributeOn(A_REVERSE);
+if (m_cursor_position == GetContentLength())
+  // Cursor is past the last character. Highlight an empty space.
+  window.PutChar(' ');
+else
+  window.PutChar(m_content[m_cursor_position]);
+if (is_active)
+  window.AttributeOff(A_REVERSE);
+  }
+
+  // The cursor is allowed to move one character past the string.
+  // m_cursor_position is in range [0, GetContentLength()].
+  void MoveCursorRight() {
+if (m_cursor_position < GetContentLength())
+  m_cursor_position++;
+  }
+
+  void MoveCursorLeft() {
+if (m_cursor_position > 0)
+  m_cursor_position--;

[Lldb-commits] [PATCH] D104395: [LLDB][GUI] Add initial forms support

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added inline comments.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1108
+  window.AttributeOn(A_REVERSE);
+window.PutChar(m_content ? 'X' : ' ');
+if (is_active)

clayborg wrote:
> We could use ACS_DIAMOND or ACS_BULLET? Check it out and see how it looks?
What do you think?

{F17450120}

{F17450119}

{F17450118}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104395

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

[Lldb-commits] [PATCH] D104380: [lldb] Set return object failed status even if error string is empty

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

This might be a case of an overly reduced patch - this patch alone I'd expect 
to be observable in some way, and tested. If the "instring.empty()" condition 
never fires, then I'd expect to change that to an assert, rather than changing 
the semantics of it in an unobservable way.

But I see there were follow-up NFC/refactoring commits that did take advantage 
of the new behavior - might've been more suitable to group the refactoring with 
this change in behavior, at least in one example of each of the 3 changes here 
(eg: for each one, change one caller and the implementation - then change the 
rest of the callers in separate follow-up commits if you prefer to break them 
out that way, to keep patches small in case they do introduce any regressions)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104380

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

[Lldb-commits] [PATCH] D104395: [LLDB][GUI] Add initial forms support

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev updated this revision to Diff 352778.
OmarEmaraDev added a comment.

- Always scroll left on removing a character


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104395

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -392,6 +392,12 @@
   void Box(chtype v_char = ACS_VLINE, chtype h_char = ACS_HLINE) {
 ::box(m_window, v_char, h_char);
   }
+  void VerticalLine(int n, chtype v_char = ACS_VLINE) {
+::wvline(m_window, v_char, n);
+  }
+  void HorizontalLine(int n, chtype h_char = ACS_HLINE) {
+::whline(m_window, h_char, n);
+  }
   void Clear() { ::wclear(m_window); }
   void Erase() { ::werase(m_window); }
   Rect GetBounds() const {
@@ -674,6 +680,36 @@
   AttributeOff(attr);
   }
 
+  void DrawBox(const Rect &bounds, chtype v_char = ACS_VLINE,
+   chtype h_char = ACS_HLINE) {
+MoveCursor(bounds.origin.x, bounds.origin.y);
+VerticalLine(bounds.size.height);
+HorizontalLine(bounds.size.width);
+PutChar(ACS_ULCORNER);
+
+MoveCursor(bounds.origin.x + bounds.size.width - 1, bounds.origin.y);
+VerticalLine(bounds.size.height);
+PutChar(ACS_URCORNER);
+
+MoveCursor(bounds.origin.x, bounds.origin.y + bounds.size.height - 1);
+HorizontalLine(bounds.size.width);
+PutChar(ACS_LLCORNER);
+
+MoveCursor(bounds.origin.x + bounds.size.width - 1,
+   bounds.origin.y + bounds.size.height - 1);
+PutChar(ACS_LRCORNER);
+  }
+
+  void DrawTitledBox(const Rect &bounds, const char *title,
+ chtype v_char = ACS_VLINE, chtype h_char = ACS_HLINE) {
+DrawBox(bounds, v_char, h_char);
+int title_offset = 2;
+MoveCursor(bounds.origin.x + title_offset, bounds.origin.y);
+PutChar('[');
+PutCString(title, bounds.size.width - title_offset);
+PutChar(']');
+  }
+
   virtual void Draw(bool force) {
 if (m_delegate_sp && m_delegate_sp->WindowDelegateDraw(*this, force))
   return;
@@ -869,6 +905,550 @@
   const Window &operator=(const Window &) = delete;
 };
 
+/
+// Forms
+/
+
+class FieldDelegate {
+public:
+  virtual ~FieldDelegate() = default;
+
+  virtual Rect FieldDelegateGetBounds() = 0;
+
+  virtual void FieldDelegateDraw(Window &window, bool is_active) = 0;
+
+  virtual HandleCharResult FieldDelegateHandleChar(int key) {
+return eKeyNotHandled;
+  }
+};
+
+typedef std::shared_ptr FieldDelegateSP;
+
+class TextFieldDelegate : public FieldDelegate {
+public:
+  TextFieldDelegate(const char *label, int width, Point origin,
+const char *content)
+  : m_label(label), m_width(width), m_origin(origin), m_cursor_position(0),
+m_first_visibile_char(0) {
+if (content)
+  m_content = content;
+assert(m_width > 2);
+  }
+
+  // Get the bounding box of the field. The text field has a height of 3, 2
+  // lines for borders and 1 for the content.
+  Rect FieldDelegateGetBounds() override {
+return Rect(m_origin, Size(m_width, 3));
+  }
+
+  // Get the start X position of the content in window space, without the
+  // borders.
+  int GetX() { return m_origin.x + 1; }
+
+  // Get the start Y position of the content in window space, without the
+  // borders.
+  int GetY() { return m_origin.y + 1; }
+
+  // Get the effective width of the field, without the borders.
+  int GetEffectiveWidth() { return m_width - 2; }
+
+  // Get the cursor position in window space.
+  int GetCursorWindowXPosition() {
+return GetX() + m_cursor_position - m_first_visibile_char;
+  }
+
+  int GetContentLength() { return (int)m_content.length(); }
+
+  void FieldDelegateDraw(Window &window, bool is_active) override {
+// Draw label box.
+window.DrawTitledBox(FieldDelegateGetBounds(), m_label.c_str());
+
+// Draw content.
+window.MoveCursor(GetX(), GetY());
+const char *text = m_content.c_str() + m_first_visibile_char;
+window.PutCString(text, GetEffectiveWidth());
+
+// Highlight the cursor.
+window.MoveCursor(GetCursorWindowXPosition(), GetY());
+if (is_active)
+  window.AttributeOn(A_REVERSE);
+if (m_cursor_position == GetContentLength())
+  // Cursor is past the last character. Highlight an empty space.
+  window.PutChar(' ');
+else
+  window.PutChar(m_content[m_cursor_position]);
+if (is_active)
+  window.AttributeOff(A_REVERSE);
+  }
+
+  // The cursor is allowed to move one character past the string.
+  // m_cursor_position is in range [0, GetContentLength()].
+  void MoveCursorRight() {
+if (m_cursor_position < GetContentLength())
+  m_cursor_position++;
+  }
+
+  void MoveCursorLeft() {
+if (m_cursor_position > 0)
+  m_cursor_position--;
+  }
+
+  // If the cursor moved past

[Lldb-commits] [PATCH] D104067: [lldb] Decouple ObjCLanguage from Symtab

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104067

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

[Lldb-commits] [lldb] aa4685c - [lldb-vscode] only report long running progress events

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-06-17T12:01:27-07:00
New Revision: aa4685c0fb3aab5acb90be5fd3eb5ba8bf1e3211

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

LOG: [lldb-vscode] only report long running progress events

When the number of shared libs is massive, there could be hundreds of
thousands of short lived progress events sent to the IDE, which makes it
irresponsive while it's processing all this data. As these small jobs
take less than a second to process, the user doesn't even see them,
because the IDE only display the progress of long operations. So it's
better not to send these events.

I'm fixing that by sending only the events that are taking longer than 5
seconds to process.
In a specific run, I got the number of events from ~500k to 100, because
there was only 1 big lib to parse.

I've tried this on several small and massive targets, and it seems to
work fine.

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

Added: 


Modified: 
lldb/tools/lldb-vscode/ProgressEvent.cpp
lldb/tools/lldb-vscode/ProgressEvent.h
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/ProgressEvent.cpp 
b/lldb/tools/lldb-vscode/ProgressEvent.cpp
index c282021a1969b..bb26e1bf30098 100644
--- a/lldb/tools/lldb-vscode/ProgressEvent.cpp
+++ b/lldb/tools/lldb-vscode/ProgressEvent.cpp
@@ -13,43 +13,88 @@
 using namespace lldb_vscode;
 using namespace llvm;
 
-ProgressEvent::ProgressEvent(uint64_t progress_id, const char *message,
- uint64_t completed, uint64_t total)
-: m_progress_id(progress_id), m_message(message) {
-  if (completed == total)
-m_event_type = progressEnd;
-  else if (completed == 0)
+// The minimum duration of an event for it to be reported
+const std::chrono::duration kStartProgressEventReportDelay =
+std::chrono::seconds(1);
+// The minimum time interval between update events for reporting. If multiple
+// updates fall within the same time interval, only the latest is reported.
+const std::chrono::duration kUpdateProgressEventReportDelay =
+std::chrono::milliseconds(250);
+
+ProgressEvent::ProgressEvent(uint64_t progress_id, Optional message,
+ uint64_t completed, uint64_t total,
+ const ProgressEvent *prev_event)
+: m_progress_id(progress_id) {
+  if (message)
+m_message = message->str();
+
+  const bool calculate_percentage = total != UINT64_MAX;
+  if (completed == 0) {
+// Start event
 m_event_type = progressStart;
-  else if (completed < total)
-m_event_type = progressUpdate;
-  else
-m_event_type = progressInvalid;
+// Wait a bit before reporting the start event in case in completes really
+// quickly.
+m_minimum_allowed_report_time =
+m_creation_time + kStartProgressEventReportDelay;
+if (calculate_percentage)
+  m_percentage = 0;
+  } else if (completed == total) {
+// End event
+m_event_type = progressEnd;
+// We should report the end event right away.
+m_minimum_allowed_report_time = std::chrono::seconds::zero();
+if (calculate_percentage)
+  m_percentage = 100;
+  } else {
+// Update event
+assert(prev_event);
+m_percentage = std::min(
+(uint32_t)((double)completed / (double)total * 100.0), (uint32_t)99);
+if (prev_event->Reported()) {
+  // Add a small delay between reports
+  m_minimum_allowed_report_time =
+  prev_event->m_minimum_allowed_report_time +
+  kUpdateProgressEventReportDelay;
+} else {
+  // We should use the previous timestamp, as it's still pending
+  m_minimum_allowed_report_time = 
prev_event->m_minimum_allowed_report_time;
+}
+  }
+}
+
+Optional ProgressEvent::Create(uint64_t progress_id,
+  Optional message,
+  uint64_t completed,
+  uint64_t total,
+  const ProgressEvent *prev_event) 
{
+  ProgressEvent event(progress_id, message, completed, total, prev_event);
+  // We shouldn't show unnamed start events in the IDE
+  if (event.GetEventType() == progressStart && event.GetEventName().empty())
+return None;
 
-  if (0 < total && total < UINT64_MAX)
-m_percentage = (uint32_t)(((float)completed / (float)total) * 100.0);
+  if (prev_event && prev_event->EqualsForIDE(event))
+return None;
+
+  return event;
 }
 
-bool ProgressEvent::operator==(const ProgressEvent &other) const {
+bool ProgressEvent::EqualsForIDE(const ProgressEvent &other) const {
   return m_progress_id == other.m_progress_id &&
 

[Lldb-commits] [PATCH] D101128: [lldb-vscode] only report long running progress events

[Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaa4685c0fb3a: [lldb-vscode] only report long running 
progress events (authored by wallace, committed by Walter Erquinigo 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101128

Files:
  lldb/tools/lldb-vscode/ProgressEvent.cpp
  lldb/tools/lldb-vscode/ProgressEvent.h
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -389,8 +389,7 @@
 const char *message = lldb::SBDebugger::GetProgressFromEvent(
 event, progress_id, completed, total, is_debugger_specific);
 if (message)
-  g_vsc.SendProgressEvent(
-  ProgressEvent(progress_id, message, completed, total));
+  g_vsc.SendProgressEvent(progress_id, message, completed, total);
   }
 }
   }
Index: lldb/tools/lldb-vscode/VSCode.h
===
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -114,7 +114,7 @@
   uint32_t reverse_request_seq;
   std::map request_handlers;
   bool waiting_for_run_in_terminal;
-  ProgressEventFilterQueue progress_event_queue;
+  ProgressEventReporter progress_event_reporter;
   // Keep track of the last stop thread index IDs as threads won't go away
   // unless we send a "thread" event to indicate the thread exited.
   llvm::DenseSet thread_ids;
@@ -125,10 +125,6 @@
   int64_t GetLineForPC(int64_t sourceReference, lldb::addr_t pc) const;
   ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
   ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
-  // Send the JSON in "json_str" to the "out" stream. Correctly send the
-  // "Content-Length:" field followed by the length, followed by the raw
-  // JSON bytes.
-  void SendJSON(const std::string &json_str);
 
   // Serialize the JSON value into a string and send the JSON packet to
   // the "out" stream.
@@ -138,7 +134,8 @@
 
   void SendOutput(OutputType o, const llvm::StringRef output);
 
-  void SendProgressEvent(const ProgressEvent &event);
+  void SendProgressEvent(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total);
 
   void __attribute__((format(printf, 3, 4)))
   SendFormattedOutput(OutputType o, const char *format, ...);
@@ -208,6 +205,12 @@
   /// The callback to execute when the given request is triggered by the
   /// IDE.
   void RegisterRequestCallback(std::string request, RequestCallback callback);
+
+private:
+  // Send the JSON in "json_str" to the "out" stream. Correctly send the
+  // "Content-Length:" field followed by the length, followed by the raw
+  // JSON bytes.
+  void SendJSON(const std::string &json_str);
 };
 
 extern VSCode g_vsc;
Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -42,7 +42,7 @@
   focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),
   stop_at_entry(false), is_attach(false), reverse_request_seq(0),
   waiting_for_run_in_terminal(false),
-  progress_event_queue(
+  progress_event_reporter(
   [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }) {
   const char *log_file_path = getenv("LLDBVSCODE_LOG");
 #if defined(_WIN32)
@@ -322,8 +322,9 @@
 //   };
 // }
 
-void VSCode::SendProgressEvent(const ProgressEvent &event) {
-  progress_event_queue.Push(event);
+void VSCode::SendProgressEvent(uint64_t progress_id, const char *message,
+   uint64_t completed, uint64_t total) {
+  progress_event_reporter.Push(progress_id, message, completed, total);
 }
 
 void __attribute__((format(printf, 3, 4)))
Index: lldb/tools/lldb-vscode/ProgressEvent.h
===
--- lldb/tools/lldb-vscode/ProgressEvent.h
+++ lldb/tools/lldb-vscode/ProgressEvent.h
@@ -6,6 +6,10 @@
 //
 //===--===//
 
+#include 
+#include 
+#include 
+
 #include "VSCodeForward.h"
 
 #include "llvm/Support/JSON.h"
@@ -13,50 +17,142 @@
 namespace lldb_vscode {
 
 enum ProgressEventType {
-  progressInvalid,
   progressStart,
   progressUpdate,
   progressEnd
 };
 
+class ProgressEvent;
+using ProgressEventReportCallback = std::function;
+
 class ProgressEvent {
 public:
-  ProgressEvent() {}
-
-  ProgressEvent(uint64_t progress_id, const char *message, uint64_t completed,
-uint64_t total);

[Lldb-commits] [PATCH] D104437: Add test for functions with extended characters.

[Neal via Phabricator via lldb-commits]

nealsid added a comment.

Ah, my grep/find skills clearly failed me :)

Yeah, those tests are exactly the same scenarios.  However, if I understand 
correctly, don't they use the API? I wanted to add some coverage for the shell 
because I'm making changes to the Editline wrapper, and the existing tests 
don't appear to cover user input from the command line.  But maybe I'm missing 
how those two tie together.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104437

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

[Lldb-commits] [PATCH] D103500: [trace][intel-pt] Create basic SB API

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 352835.
wallace marked 2 inline comments as done.
wallace added a comment.

address comments:

- use llvm::ArrayRef where needed
- improved documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103500

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBProcess.i
  lldb/bindings/interface/SBStructuredData.i
  lldb/bindings/interface/SBTarget.i
  lldb/bindings/interface/SBTrace.i
  lldb/bindings/interface/SBTraceOptions.i
  lldb/bindings/interfaces.swig
  lldb/docs/.htaccess
  lldb/docs/design/overview.rst
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/API/SBThread.h
  lldb/include/lldb/API/SBTrace.h
  lldb/include/lldb/API/SBTraceOptions.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/TraceGDBRemotePackets.h
  lldb/include/lldb/Utility/TraceOptions.h
  lldb/include/lldb/lldb-forward.h
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/API/SBTrace.cpp
  lldb/source/API/SBTraceOptions.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectThreadUtil.h
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/TestTraceSchema.py
  lldb/test/API/commands/trace/TestTraceStartStop.py
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -12,7 +12,6 @@
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/StructuredData.h"
-#include "lldb/Utility/TraceOptions.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Testing/Support/Error.h"
Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -1,53 +1,49 @@
 import lldb
+from intelpt_testcase import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 
-ADDRESS_REGEX = '0x[0-9a-fA-F]*'
-
-class TestTraceStartStopMultipleThreads(TestBase):
+class TestTraceStartStopMultipleThreads(TraceIntelPTTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
-NO_DEBUG_INFO_TESTCASE = True
-
-def setUp(self):
-TestBase.setUp(self)
-if 'intel-pt' not in configuration.enabled_plugins:
-self.skipTest("The intel-pt test plugin is not enabled")
 
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+@testSBAPIAndCommands
 def testStartMultipleLiveThreads(self):
 self.build()
-target = self.createTestTarget()
+exe = self.getBuildArtifact("a.out")
+
+self.dbg.CreateTarget(exe)
 
 self.expect("b main")
 self.expect("b 6")
 self.expect("b 11")
 
 self.expect("r")
-self.expect("proce trace start")
-
+self.traceStartProcess()
 
-# We'll see here the first thread
 self.expect("continue")
 self.expect("thread trace dump instructions", substrs=['main.cpp:9'])
-
+
 # We'll see here the second thread
 self.expect("continue")
 self.expect("thread trace dump i

[Lldb-commits] [PATCH] D103500: [trace][intel-pt] Create basic SB API

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

Looks good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103500

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

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, aprantl, JDevlieghere, jingham, wallace.
Herald added subscribers: kristof.beyls, emaste.
clayborg requested review of this revision.
Herald added subscribers: lldb-commits, MaskRay.
Herald added a project: LLDB.

This fix was created after profiling the target creation of a large C/C++/ObjC 
application that contained almost 4,000,000 redacted symbol names. The symbol 
table parsing code was creating names for each of these synthetic symbols and 
adding them to the name indexes. The code was also adding the object file 
basename to the end of the symbol name which doesn't allow symbols from 
different shared libraries to share the names in the constant string pool.

Prior to this fix this was creating 180MB of "___lldb_unnamed_symbol" symbol 
names and was taking a long time to generate each name, add them to the string 
pool and then add each of these names to the name index.

This patch fixes the issue by:

- not adding a name to synthetic symbols at creation time, and allows name to 
be dynamically generated when accessed
- doesn't add synthetic symbol names to the name indexes, but catches this 
special case as name lookup time. Users won't typically set breakpoints or 
lookup these synthetic names, but support was added to do the lookup in case it 
does happen
- removes the object file baseanme from the generated names to allow the names 
to be shared in the constant string pool

Prior to this fix the startup times for a large application was:
12.5 seconds (cold file caches)
8.5 seconds (warm file caches)

After this fix:
9.7 seconds (cold file caches)
5.7 seconds (warm file caches)

The names of the symbols are auto generated by appending the symbol's UserID to 
the end of the "___lldb_unnamed_symbol" string and is only done when the name 
is requested from a synthetic symbol if it has no name.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104488

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Symbol/Symbol.h
  lldb/include/lldb/Symbol/Symtab.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Symbol/ObjectFile.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Symbol/Symtab.cpp

Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -291,7 +291,7 @@
   // the trampoline symbols to be searchable by name we can remove this and
   // then possibly add a new bool to any of the Symtab functions that
   // lookup symbols by name to indicate if they want trampolines.
-  if (symbol->IsTrampoline())
+  if (symbol->IsTrampoline() || symbol->IsSynthetic())
 continue;
 
   // If the symbol's name string matched a Mangled::ManglingScheme, it is
@@ -614,6 +614,36 @@
   }
 }
 
+uint32_t Symtab::GetNameIndexes(ConstString symbol_name,
+std::vector &indexes) {
+  auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
+  const uint32_t count = name_to_index.GetValues(symbol_name, indexes);
+  if (count)
+return count;
+  // Synthetic symbol names are not added to the name indexes, but they start
+  // with a prefix and end with a the symbol UserID, so allow users to find
+  // these without having to add them to the name indexes. This queries will
+  // not happen very often since the names don't mean anything, so performance
+  // is not paramount in this case.
+  llvm::StringRef name = symbol_name.GetStringRef();
+  // String the synthetic prefix if the name starts with it.
+  if (!name.consume_front(Symbol::GetSyntheticSymbolPrefix()))
+return 0; // Not a synthetic symbol name
+
+  // Extract the user ID from the symbol name
+  user_id_t uid = 0;
+  if (getAsUnsignedInteger(name, 10 /* Radix */, uid))
+return 0; // Failed to extract the user ID as an integer
+  Symbol *symbol = FindSymbolByID(uid);
+  if (symbol == nullptr)
+return 0;
+  const uint32_t symbol_idx = GetIndexForSymbol(symbol);
+  if (symbol_idx == UINT32_MAX)
+return 0;
+  indexes.push_back(symbol_idx);
+  return 1;
+}
+
 uint32_t Symtab::AppendSymbolIndexesWithName(ConstString symbol_name,
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
@@ -623,8 +653,7 @@
 if (!m_name_indexes_computed)
   InitNameIndexes();
 
-auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
-return name_to_index.GetValues(symbol_name, indexes);
+return GetNameIndexes(symbol_name, indexes);
   }
   return 0;
 }
@@ -641,10 +670,9 @@
 if (!m_name_indexes_computed)
   InitNameIndexes();
 
-auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
 std::vector all_name_indexes;
 const size_t name_match_count =
-name_to_index.GetValue

[Lldb-commits] [lldb] c1360fd - [lldb-vscode] remove failed test

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-06-17T15:13:16-07:00
New Revision: c1360fd5fced42ea45ce1eacbf1c5e1ed54fcac5

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

LOG: [lldb-vscode] remove failed test

Found in 
http://green.lab.llvm.org/green/job/lldb-cmake/32891/testReport/lldb-api/tools_lldb-vscode_launch/TestVSCode_launch_py/

the lldb-vscode changed and that test makes no sense anymore

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py 
b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
index c40e7e2dd4aa..593701e7ca64 100644
--- a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -455,65 +455,3 @@ def test_terminate_commands(self):
 self.vscode.request_disconnect(terminateDebuggee=True)
 output = self.collect_console(duration=1.0)
 self.verify_commands('terminateCommands', output, terminateCommands)
-
-
-@skipIfWindows
-@skipIfRemote
-@skipIf(oslist=["linux"])
-def test_progress_events(self):
-'''
-Tests the progress events to ensure we are receiving them.
-'''
-program = self.getBuildArtifact("a.out")
-self.build_and_launch(program)
-# Set a breakpoint at 'main'. This will cause all of the symbol tables
-# for all modules in LLDB to be parsed and we should get a progress
-# event for each shared library.
-breakpoint_ids = self.set_function_breakpoints(['main'])
-self.continue_to_breakpoints(breakpoint_ids)
-# Make sure we at least got some progress events
-self.assertTrue(len(self.vscode.progress_events) > 0)
-# Track all 'progressStart' events by saving all 'progressId' values.
-progressStart_ids = set()
-# Track all 'progressEnd' events by saving all 'progressId' values.
-progressEnd_ids = set()
-# We will watch for events whose title starts with
-# 'Parsing symbol table for ' and we will save the remainder of the
-# line which will contain the shared library basename. Since we set a
-# breakpoint by name for 'main', we will expect to see progress events
-# for all shared libraries that say that the symbol table is being
-# parsed.
-symtab_progress_shlibs = set()
-# Get a list of modules in the current target so we can verify that
-# we do in fact get a progress event for each shared library.
-target_shlibs = self.vscode.get_modules()
-
-# Iterate over all progress events and save all start and end IDs, and
-# remember any shared libraries that got symbol table parsing progress
-# events.
-# Sleep for 5 seconds to make sure progress_events gets populated
-time.sleep(5)
-for progress_event in self.vscode.progress_events:
-event_type = progress_event['event']
-if event_type == 'progressStart':
-progressStart_ids.add(progress_event['body']['progressId'])
-title = progress_event['body']['title']
-if title.startswith('Parsing symbol table for '):
-symtab_progress_shlibs.add(title[25:])
-if event_type == 'progressEnd':
-progressEnd_ids.add(progress_event['body']['progressId'])
-# Make sure for each 'progressStart' event, we got a matching
-# 'progressEnd' event.
-self.assertTrue(progressStart_ids == progressEnd_ids,
-('Make sure we got a "progressEnd" for each '
- '"progressStart" event that we have.'))
-
-ignored_libraries = {"[vdso]"}
-
-# Verify we got a symbol table parsing progress event for each shared
-# library in our target.
-for target_shlib_basename in target_shlibs.keys():
-if target_shlib_basename in ignored_libraries:
-continue
-self.assertIn(target_shlib_basename, symtab_progress_shlibs,
-'Make sure we got a symbol table progress event 
for "%s"' % (target_shlib_basename))



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

[Lldb-commits] [lldb] bf9f21a - [trace][intel-pt] Create basic SB API

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-06-17T15:14:47-07:00
New Revision: bf9f21a28be171dc500cc68b4cb1fcd3fc33f229

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

LOG: [trace][intel-pt] Create basic SB API

This adds a basic SB API for creating and stopping traces.
Note: This doesn't add any APIs for inspecting individual instructions. That'd 
be a more complicated change and it might be better to enhande the dump 
functionality to output the data in binary format. I'll leave that for a later 
diff.

This also enhances the existing tests so that they test the same flow using 
both the command interface and the SB API.

I also did some cleanup of legacy code.

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

Added: 
lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h

Modified: 
lldb/bindings/headers.swig
lldb/bindings/interface/SBProcess.i
lldb/bindings/interface/SBStructuredData.i
lldb/bindings/interface/SBTarget.i
lldb/bindings/interface/SBTrace.i
lldb/bindings/interfaces.swig
lldb/docs/.htaccess
lldb/docs/design/overview.rst
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/API/LLDB.h
lldb/include/lldb/API/SBDefines.h
lldb/include/lldb/API/SBProcess.h
lldb/include/lldb/API/SBStructuredData.h
lldb/include/lldb/API/SBTarget.h
lldb/include/lldb/API/SBThread.h
lldb/include/lldb/API/SBTrace.h
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/Target.h
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Utility/TraceGDBRemotePackets.h
lldb/include/lldb/lldb-forward.h
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/source/API/CMakeLists.txt
lldb/source/API/SBProcess.cpp
lldb/source/API/SBReproducer.cpp
lldb/source/API/SBStructuredData.cpp
lldb/source/API/SBTarget.cpp
lldb/source/API/SBTrace.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/CommandObjectThreadUtil.h
lldb/source/Commands/CommandObjectTrace.cpp
lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/Trace.cpp
lldb/test/API/commands/trace/TestTraceDumpInstructions.py
lldb/test/API/commands/trace/TestTraceLoad.py
lldb/test/API/commands/trace/TestTraceSchema.py
lldb/test/API/commands/trace/TestTraceStartStop.py

lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Removed: 
lldb/bindings/interface/SBTraceOptions.i
lldb/include/lldb/API/SBTraceOptions.h
lldb/include/lldb/Utility/TraceOptions.h
lldb/source/API/SBTraceOptions.cpp



diff  --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig
index 6e1668ea4c42..3c2cd85f504d 100644
--- a/lldb/bindings/headers.swig
+++ b/lldb/bindings/headers.swig
@@ -62,7 +62,6 @@
 #include "lldb/API/SBThreadCollection.h"
 #include "lldb/API/SBThreadPlan.h"
 #include "lldb/API/SBTrace.h"
-#include "lldb/API/SBTraceOptions.h"
 #include "lldb/API/SBType.h"
 #include "lldb/API/SBTypeCategory.h"
 #include "lldb/API/SBTypeEnumMember.h"

diff  --git a/lldb/bindings/interface/SBProcess.i 
b/lldb/bindings/interface/SBProcess.i
index e30b89d1ed39..27c015df85cd 100644
--- a/lldb/bindings/interface/SBProcess.i
+++ b/lldb/bindings/interface/SBProcess.i
@@ -400,9 +400,6 @@ public:
 lldb::SBError
 SaveCore(const char *file_name);
 
-lldb::SBTrace
-StartTrace(SBTraceOptions &options, lldb::SBError &error);
-
 lldb::SBError
 GetMemoryRegionInfo(lldb::addr_t load_addr, lldb::SBMemoryRegionInfo 
®ion_info);
 

diff  --git a/lldb/bindings/interface/SBStructuredData.i 
b/lldb/bindings/interface/SBStructuredData.i
index 5aba35229855..ba5b7e075065 100644
--- a/lldb/bindings/interface/SBStructuredData.i
+++ b/lldb/bindings/interface/SBStructuredData.i
@@ -58,5 +58,8 @@ This class wraps the event type generated by StructuredData 
features."
 
 lldb::SBError
 SetFromJSON(lldb::SBStream &stream);
+
+lldb::SBError
+SetFromJSON(const char *json);
 };
 }

diff  --git a/lldb/bindings/interface/SBTarget.i 
b/lldb/bindings/interface/SBTarget.i
index 772ee1a2eb73..3f9e4cdc6d67 100644
--- a/lldb/bin

[Lldb-commits] [PATCH] D103500: [trace][intel-pt] Create basic SB API

[Phabricator via lldb-commits]

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103500

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBProcess.i
  lldb/bindings/interface/SBStructuredData.i
  lldb/bindings/interface/SBTarget.i
  lldb/bindings/interface/SBTrace.i
  lldb/bindings/interface/SBTraceOptions.i
  lldb/bindings/interfaces.swig
  lldb/docs/.htaccess
  lldb/docs/design/overview.rst
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/API/SBThread.h
  lldb/include/lldb/API/SBTrace.h
  lldb/include/lldb/API/SBTraceOptions.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/TraceGDBRemotePackets.h
  lldb/include/lldb/Utility/TraceOptions.h
  lldb/include/lldb/lldb-forward.h
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/API/SBTrace.cpp
  lldb/source/API/SBTraceOptions.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectThreadUtil.h
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/TestTraceSchema.py
  lldb/test/API/commands/trace/TestTraceStartStop.py
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -12,7 +12,6 @@
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/StructuredData.h"
-#include "lldb/Utility/TraceOptions.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Testing/Support/Error.h"
Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -1,53 +1,49 @@
 import lldb
+from intelpt_testcase import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 
-ADDRESS_REGEX = '0x[0-9a-fA-F]*'
-
-class TestTraceStartStopMultipleThreads(TestBase):
+class TestTraceStartStopMultipleThreads(TraceIntelPTTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
-NO_DEBUG_INFO_TESTCASE = True
-
-def setUp(self):
-TestBase.setUp(self)
-if 'intel-pt' not in configuration.enabled_plugins:
-self.skipTest("The intel-pt test plugin is not enabled")
 
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+@testSBAPIAndCommands
 def testStartMultipleLiveThreads(self):
 self.build()
-target = self.createTestTarget()
+exe = self.getBuildArtifact("a.out")
+
+self.dbg.CreateTarget(exe)
 
 self.expect("b main")
 self.expect("b 6")
 self.expect("b 11")
 
 self.expect("r")
-self.expect("proce trace start")
-
+self.traceStartProcess()
 
-# We'll see here the first thread
 self.expect("continue")
 self.expect("thread trace dump instructions", substrs=['mai

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

lgtm

I wonder if it's lldb's style to rename m_mangled to m_mangled_do_not_use (or 
something like that) to prevent people from using it in the future.




Comment at: lldb/source/Symbol/Symtab.cpp:625
+  // with a prefix and end with a the symbol UserID, so allow users to find
+  // these without having to add them to the name indexes. This queries will
+  // not happen very often since the names don't mean anything, so performance

these queries


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104488

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

[Lldb-commits] [PATCH] D104422: [trace] Add a TraceCursor class

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 11 inline comments as done.
wallace added inline comments.



Comment at: lldb/include/lldb/Target/TraceCursor.h:39-40
+///
+///  The Trace initially points to a dummy invalid instruction error signaling
+///  the end of a trace, similar to a C++ collections' end iterator.
+///

clayborg wrote:
> Should the TraceCursor point to the last instruction more like a begin() STL 
> call?
yes, i overcomplicated it



Comment at: lldb/include/lldb/Target/TraceCursor.h:116-117
+  /// \return
+  /// The size in bytes of the instruction opcode the cursor is pointing 
at.
+  /// If the cursor points to an error in the trace, return \b 0.
+  virtual size_t GetInstructionSize() = 0;

clayborg wrote:
> Do we even need the instruction size in this cursor class interface? Clients 
> can easily grab an instruction from GetLoadAddress() and do that themselves?
yes, this makes no sense. I sometimes use it for determining if two 
instructions are sequential, but i might as well use the control flow type to 
check that, i.e. if it's a normal or not taken branch, then the next 
instruction is sequential.



Comment at: lldb/include/lldb/Target/TraceCursor.h:72-75
+  /// \return
+  /// \b true if the cursor is pointing to an error in the trace. The 
actual
+  /// error message can be gotten by calling \a TraceCursor::GetError().
+  bool IsError();

clayborg wrote:
> Do we need this if we have GetError?
i imagine it might not always be a no-op to construct and error message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104422

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

[Lldb-commits] [PATCH] D104422: [trace] Add a TraceCursor class

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 352872.
wallace marked 2 inline comments as done.
wallace added a comment.
Herald added a subscriber: mgorny.

address comments:

- now simply pointing to the last instruction of the trace by default
- removed the GetInstructionSize, as it's really not needed
- made sure that the target lldbCore builds correctly
- improved documentation
- some other small fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104422

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceCursor.cpp

Index: lldb/source/Target/TraceCursor.cpp
===
--- /dev/null
+++ lldb/source/Target/TraceCursor.cpp
@@ -0,0 +1,15 @@
+//===-- TraceCursor.cpp -*- 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 "lldb/Target/TraceCursor.h"
+
+#include "lldb/Target/Trace.h"
+
+using namespace lldb_private;
+
+bool TraceCursor::IsStale() { return m_stop_id != m_trace_sp->GetStopID(); }
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -473,3 +473,8 @@
 
   DoRefreshLiveProcessState(std::move(live_process_state));
 }
+
+uint32_t Trace::GetStopID() {
+  RefreshLiveProcessState();
+  return m_stop_id;
+}
Index: lldb/source/Target/CMakeLists.txt
===
--- lldb/source/Target/CMakeLists.txt
+++ lldb/source/Target/CMakeLists.txt
@@ -68,6 +68,7 @@
   ThreadSpec.cpp
   ThreadPostMortemTrace.cpp
   Trace.cpp
+  TraceCursor.cpp
   TraceSessionFileParser.cpp
   UnixSignals.cpp
   UnwindAssembly.cpp
Index: lldb/include/lldb/lldb-forward.h
===
--- lldb/include/lldb/lldb-forward.h
+++ lldb/include/lldb/lldb-forward.h
@@ -229,6 +229,7 @@
 class ThreadSpec;
 class ThreadPostMortemTrace;
 class Trace;
+class TraceCursor;
 class TraceSessionFileParser;
 class Type;
 class TypeAndOrName;
@@ -441,6 +442,7 @@
 typedef std::weak_ptr ThreadPlanWP;
 typedef std::shared_ptr ThreadPlanTracerSP;
 typedef std::shared_ptr TraceSP;
+typedef std::unique_ptr TraceCursorUP;
 typedef std::shared_ptr TypeSP;
 typedef std::weak_ptr TypeWP;
 typedef std::shared_ptr TypeCategoryImplSP;
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -958,6 +958,25 @@
   eExpressionEvaluationComplete
 };
 
+/// Architecture-agnostic categorization of instructions for traversing the
+/// control flow of a trace.
+///
+/// A single instruction can match one or more of these categories.
+FLAGS_ENUM(TraceInstructionControlFlowType){
+/// Any instruction.
+eTraceInstructionControlFlowTypeInstruction = (1u << 1),
+/// A conditional or unconditional branch/jump.
+eTraceInstructionControlFlowTypeBranch = (1u << 2),
+/// A conditional or unconditional branch/jump that changed
+/// the control flow of the program.
+eTraceInstructionControlFlowTypeTakenBranch = (1u << 3),
+/// A call to a function.
+eTraceInstructionControlFlowTypeCall = (1u << 4),
+/// A return from a function.
+eTraceInstructionControlFlowTypeReturn = (1u << 5)};
+
+LLDB_MARK_AS_BITMASK_ENUM(TraceInstructionControlFlowType)
+
 /// Watchpoint Kind.
 ///
 /// Indicates what types of events cause the watchpoint to fire. Used by Native
Index: lldb/include/lldb/Target/TraceCursor.h
===
--- /dev/null
+++ lldb/include/lldb/Target/TraceCursor.h
@@ -0,0 +1,136 @@
+//===-- TraceCursor.h ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_TRACE_CURSOR_H
+#define LLDB_TARGET_TRACE_CURSOR_H
+
+#include "lldb/lldb-private.h"
+
+namespace lldb_private {
+
+/// Class used for iterating over the instructions of a thread's trace.
+///
+/// This class attempts to be a generic interface for accessing the instructions
+/// of the trace so that each Trace plug-in can rec

[Lldb-commits] [PATCH] D104423: [WebAssembly] Rename event to tag

[Heejin Ahn via Phabricator via lldb-commits]

aheejin updated this revision to Diff 352903.
aheejin edited the summary of this revision.
aheejin added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104423

Files:
  lld/include/lld/Common/LLVM.h
  lld/test/wasm/Inputs/event-section1.ll
  lld/test/wasm/Inputs/event-section2.ll
  lld/test/wasm/Inputs/tag-section1.ll
  lld/test/wasm/Inputs/tag-section2.ll
  lld/test/wasm/event-section.ll
  lld/test/wasm/tag-section.ll
  lld/wasm/InputChunks.cpp
  lld/wasm/InputElement.h
  lld/wasm/InputFiles.cpp
  lld/wasm/InputFiles.h
  lld/wasm/MarkLive.cpp
  lld/wasm/OutputSections.cpp
  lld/wasm/SymbolTable.cpp
  lld/wasm/SymbolTable.h
  lld/wasm/Symbols.cpp
  lld/wasm/Symbols.h
  lld/wasm/SyntheticSections.cpp
  lld/wasm/SyntheticSections.h
  lld/wasm/Writer.cpp
  lld/wasm/WriterUtils.cpp
  lld/wasm/WriterUtils.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  llvm/include/llvm/BinaryFormat/Wasm.h
  llvm/include/llvm/BinaryFormat/WasmRelocs.def
  llvm/include/llvm/CodeGen/WasmEHFuncInfo.h
  llvm/include/llvm/MC/MCSymbolWasm.h
  llvm/include/llvm/Object/Wasm.h
  llvm/include/llvm/ObjectYAML/WasmYAML.h
  llvm/lib/BinaryFormat/Wasm.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Object/WasmObjectFile.cpp
  llvm/lib/ObjectYAML/WasmEmitter.cpp
  llvm/lib/ObjectYAML/WasmYAML.cpp
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyWasmObjectWriter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
  llvm/test/CodeGen/WebAssembly/exception.ll
  llvm/test/CodeGen/WebAssembly/null-streamer.ll
  llvm/test/MC/WebAssembly/annotations.s
  llvm/test/MC/WebAssembly/basic-assembly.s
  llvm/test/MC/WebAssembly/event-section-decoding.ll
  llvm/test/MC/WebAssembly/event-section.ll
  llvm/test/MC/WebAssembly/tag-section-decoding.ll
  llvm/test/MC/WebAssembly/tag-section.ll
  llvm/test/ObjectYAML/wasm/event_section.yaml
  llvm/tools/llvm-readobj/WasmDumper.cpp
  llvm/tools/obj2yaml/wasm2yaml.cpp

Index: llvm/tools/obj2yaml/wasm2yaml.cpp
===
--- llvm/tools/obj2yaml/wasm2yaml.cpp
+++ llvm/tools/obj2yaml/wasm2yaml.cpp
@@ -132,7 +132,7 @@
   case wasm::WASM_SYMBOL_TYPE_FUNCTION:
   case wasm::WASM_SYMBOL_TYPE_GLOBAL:
   case wasm::WASM_SYMBOL_TYPE_TABLE:
-  case wasm::WASM_SYMBOL_TYPE_EVENT:
+  case wasm::WASM_SYMBOL_TYPE_TAG:
 Info.ElementIndex = Symbol.ElementIndex;
 break;
   case wasm::WASM_SYMBOL_TYPE_SECTION:
@@ -238,9 +238,9 @@
   Im.GlobalImport.Type = Import.Global.Type;
   Im.GlobalImport.Mutable = Import.Global.Mutable;
   break;
-case wasm::WASM_EXTERNAL_EVENT:
-  Im.EventImport.Attribute = Import.Event.Attribute;
-  Im.EventImport.SigIndex = Import.Event.SigIndex;
+case wasm::WASM_EXTERNAL_TAG:
+  Im.TagImport.Attribute = Import.Tag.Attribute;
+  Im.TagImport.SigIndex = Import.Tag.SigIndex;
   break;
 case wasm::WASM_EXTERNAL_TABLE:
   // FIXME: Currently we always output an index of 0 for any imported
@@ -280,16 +280,16 @@
   S = std::move(MemorySec);
   break;
 }
-case wasm::WASM_SEC_EVENT: {
-  auto EventSec = std::make_unique();
-  for (auto &Event : Obj.events()) {
-WasmYAML::Event E;
-E.Index = Event.Index;
-E.Attribute = Event.Type.Attribute;
-E.SigIndex = Event.Type.SigIndex;
-EventSec->Events.push_back(E);
+case wasm::WASM_SEC_TAG: {
+  auto TagSec = std::make_unique();
+  for (auto &Tag : Obj.tags()) {
+WasmYAML::Tag T;
+T.Index = Tag.Index;
+T.Attribute = Tag.Type.Attribute;
+T.SigIndex = Tag.Type.SigIndex;
+TagSec->Tags.push_back(T);
   }
-  S = std::move(EventSec);
+  S = std::move(TagSec);
   break;
 }
 case wasm::WASM_SEC_GLOBAL: {
Index: llvm/tools/llvm-readobj/WasmDumper.cpp
===
--- llvm/tools/llvm-readobj/WasmDumper.cpp
+++ llvm/tools/llvm-readobj/WasmDumper.cpp
@@ -23,8 +23,8 @@
 static const EnumEntry WasmSymbolTypes[] = {
 #define ENUM_ENTRY(X)  \
   { #X, wasm::WASM_SYMBOL_TYPE_##X }
-ENUM_ENTRY(FU

[Lldb-commits] [lldb] 1d891d4 - [WebAssembly] Rename event to tag

[Heejin Ahn via lldb-commits]

Author: Heejin Ahn
Date: 2021-06-17T20:34:19-07:00
New Revision: 1d891d44f33f99c55e779acaeac4628e4ac9aaaf

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

LOG: [WebAssembly] Rename event to tag

We recently decided to change 'event' to 'tag', and 'event section' to
'tag section', out of the rationale that the section contains a
generalized tag that references a type, which may be used for something
other than exceptions, and the name 'event' can be confusing in the web
context.

See
- 
https://github.com/WebAssembly/exception-handling/issues/159#issuecomment-857910130
- https://github.com/WebAssembly/exception-handling/pull/161

Reviewed By: tlively

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

Added: 
lld/test/wasm/Inputs/tag-section1.ll
lld/test/wasm/Inputs/tag-section2.ll
lld/test/wasm/tag-section.ll
llvm/test/MC/WebAssembly/tag-section-decoding.ll
llvm/test/MC/WebAssembly/tag-section.ll

Modified: 
lld/include/lld/Common/LLVM.h
lld/wasm/InputChunks.cpp
lld/wasm/InputElement.h
lld/wasm/InputFiles.cpp
lld/wasm/InputFiles.h
lld/wasm/MarkLive.cpp
lld/wasm/OutputSections.cpp
lld/wasm/SymbolTable.cpp
lld/wasm/SymbolTable.h
lld/wasm/Symbols.cpp
lld/wasm/Symbols.h
lld/wasm/SyntheticSections.cpp
lld/wasm/SyntheticSections.h
lld/wasm/Writer.cpp
lld/wasm/WriterUtils.cpp
lld/wasm/WriterUtils.h
lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
llvm/include/llvm/BinaryFormat/Wasm.h
llvm/include/llvm/BinaryFormat/WasmRelocs.def
llvm/include/llvm/CodeGen/WasmEHFuncInfo.h
llvm/include/llvm/MC/MCSymbolWasm.h
llvm/include/llvm/Object/Wasm.h
llvm/include/llvm/ObjectYAML/WasmYAML.h
llvm/lib/BinaryFormat/Wasm.cpp
llvm/lib/MC/WasmObjectWriter.cpp
llvm/lib/Object/RelocationResolver.cpp
llvm/lib/Object/WasmObjectFile.cpp
llvm/lib/ObjectYAML/WasmEmitter.cpp
llvm/lib/ObjectYAML/WasmYAML.cpp
llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp
llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp
llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp
llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyWasmObjectWriter.cpp
llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
llvm/test/CodeGen/WebAssembly/exception.ll
llvm/test/CodeGen/WebAssembly/null-streamer.ll
llvm/test/MC/WebAssembly/annotations.s
llvm/test/MC/WebAssembly/basic-assembly.s
llvm/test/ObjectYAML/wasm/event_section.yaml
llvm/tools/llvm-readobj/WasmDumper.cpp
llvm/tools/obj2yaml/wasm2yaml.cpp

Removed: 
lld/test/wasm/Inputs/event-section1.ll
lld/test/wasm/Inputs/event-section2.ll
lld/test/wasm/event-section.ll
llvm/test/MC/WebAssembly/event-section-decoding.ll
llvm/test/MC/WebAssembly/event-section.ll



diff  --git a/lld/include/lld/Common/LLVM.h b/lld/include/lld/Common/LLVM.h
index 2328521246b48..c19364ad9f6ce 100644
--- a/lld/include/lld/Common/LLVM.h
+++ b/lld/include/lld/Common/LLVM.h
@@ -44,8 +44,8 @@ class WasmSymbol;
 } // namespace object
 
 namespace wasm {
-struct WasmEvent;
-struct WasmEventType;
+struct WasmTag;
+struct WasmTagType;
 struct WasmFunction;
 struct WasmGlobal;
 struct WasmGlobalType;
@@ -87,8 +87,6 @@ using llvm::object::WasmObjectFile;
 using llvm::object::WasmSection;
 using llvm::object::WasmSegment;
 using llvm::object::WasmSymbol;
-using llvm::wasm::WasmEvent;
-using llvm::wasm::WasmEventType;
 using llvm::wasm::WasmFunction;
 using llvm::wasm::WasmGlobal;
 using llvm::wasm::WasmGlobalType;
@@ -98,6 +96,8 @@ using llvm::wasm::WasmRelocation;
 using llvm::wasm::WasmSignature;
 using llvm::wasm::WasmTable;
 using llvm::wasm::WasmTableType;
+using llvm::wasm::WasmTag;
+using llvm::wasm::WasmTagType;
 } // end namespace lld.
 
 namespace std {

diff  --git a/lld/test/wasm/Inputs/event-section1.ll 
b/lld/test/wasm/Inputs/tag-section1.ll
similarity index 100%
rename from lld/test/wasm/Inputs/event-section1.ll
rename to lld/test/wasm/Inputs/tag-section1.ll

diff  --git a/lld/test/wasm/Inputs/event-section2.ll 
b/lld/test/wasm/Inputs/tag-section2.ll
similarity index 100%
rename from lld/test/wasm/Inputs/event-section2.ll
rename to lld/test/wasm/Inputs/tag-section2.ll

diff  --git a/lld/test/wasm/event-section.ll b/lld/test/wasm/tag-section.ll
similarity index 84%
rename from lld/t

[Lldb-commits] [PATCH] D104423: [WebAssembly] Rename event to tag

[Heejin Ahn via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d891d44f33f: [WebAssembly] Rename event to tag (authored by 
aheejin).

Changed prior to commit:
  https://reviews.llvm.org/D104423?vs=352903&id=352907#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104423

Files:
  lld/include/lld/Common/LLVM.h
  lld/test/wasm/Inputs/event-section1.ll
  lld/test/wasm/Inputs/event-section2.ll
  lld/test/wasm/Inputs/tag-section1.ll
  lld/test/wasm/Inputs/tag-section2.ll
  lld/test/wasm/event-section.ll
  lld/test/wasm/tag-section.ll
  lld/wasm/InputChunks.cpp
  lld/wasm/InputElement.h
  lld/wasm/InputFiles.cpp
  lld/wasm/InputFiles.h
  lld/wasm/MarkLive.cpp
  lld/wasm/OutputSections.cpp
  lld/wasm/SymbolTable.cpp
  lld/wasm/SymbolTable.h
  lld/wasm/Symbols.cpp
  lld/wasm/Symbols.h
  lld/wasm/SyntheticSections.cpp
  lld/wasm/SyntheticSections.h
  lld/wasm/Writer.cpp
  lld/wasm/WriterUtils.cpp
  lld/wasm/WriterUtils.h
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  llvm/include/llvm/BinaryFormat/Wasm.h
  llvm/include/llvm/BinaryFormat/WasmRelocs.def
  llvm/include/llvm/CodeGen/WasmEHFuncInfo.h
  llvm/include/llvm/MC/MCSymbolWasm.h
  llvm/include/llvm/Object/Wasm.h
  llvm/include/llvm/ObjectYAML/WasmYAML.h
  llvm/lib/BinaryFormat/Wasm.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Object/WasmObjectFile.cpp
  llvm/lib/ObjectYAML/WasmEmitter.cpp
  llvm/lib/ObjectYAML/WasmYAML.cpp
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyWasmObjectWriter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
  llvm/test/CodeGen/WebAssembly/exception.ll
  llvm/test/CodeGen/WebAssembly/null-streamer.ll
  llvm/test/MC/WebAssembly/annotations.s
  llvm/test/MC/WebAssembly/basic-assembly.s
  llvm/test/MC/WebAssembly/event-section-decoding.ll
  llvm/test/MC/WebAssembly/event-section.ll
  llvm/test/MC/WebAssembly/tag-section-decoding.ll
  llvm/test/MC/WebAssembly/tag-section.ll
  llvm/test/ObjectYAML/wasm/event_section.yaml
  llvm/tools/llvm-readobj/WasmDumper.cpp
  llvm/tools/obj2yaml/wasm2yaml.cpp

Index: llvm/tools/obj2yaml/wasm2yaml.cpp
===
--- llvm/tools/obj2yaml/wasm2yaml.cpp
+++ llvm/tools/obj2yaml/wasm2yaml.cpp
@@ -132,7 +132,7 @@
   case wasm::WASM_SYMBOL_TYPE_FUNCTION:
   case wasm::WASM_SYMBOL_TYPE_GLOBAL:
   case wasm::WASM_SYMBOL_TYPE_TABLE:
-  case wasm::WASM_SYMBOL_TYPE_EVENT:
+  case wasm::WASM_SYMBOL_TYPE_TAG:
 Info.ElementIndex = Symbol.ElementIndex;
 break;
   case wasm::WASM_SYMBOL_TYPE_SECTION:
@@ -238,9 +238,9 @@
   Im.GlobalImport.Type = Import.Global.Type;
   Im.GlobalImport.Mutable = Import.Global.Mutable;
   break;
-case wasm::WASM_EXTERNAL_EVENT:
-  Im.EventImport.Attribute = Import.Event.Attribute;
-  Im.EventImport.SigIndex = Import.Event.SigIndex;
+case wasm::WASM_EXTERNAL_TAG:
+  Im.TagImport.Attribute = Import.Tag.Attribute;
+  Im.TagImport.SigIndex = Import.Tag.SigIndex;
   break;
 case wasm::WASM_EXTERNAL_TABLE:
   // FIXME: Currently we always output an index of 0 for any imported
@@ -280,16 +280,16 @@
   S = std::move(MemorySec);
   break;
 }
-case wasm::WASM_SEC_EVENT: {
-  auto EventSec = std::make_unique();
-  for (auto &Event : Obj.events()) {
-WasmYAML::Event E;
-E.Index = Event.Index;
-E.Attribute = Event.Type.Attribute;
-E.SigIndex = Event.Type.SigIndex;
-EventSec->Events.push_back(E);
+case wasm::WASM_SEC_TAG: {
+  auto TagSec = std::make_unique();
+  for (auto &Tag : Obj.tags()) {
+WasmYAML::Tag T;
+T.Index = Tag.Index;
+T.Attribute = Tag.Type.Attribute;
+T.SigIndex = Tag.Type.SigIndex;
+TagSec->Tags.push_back(T);
   }
-  S = std::move(EventSec);
+  S = std::move(TagSec);
   break;
 }
 case wasm::WASM_SEC_GLOBAL: {
Index: llvm/tools/llvm-readobj/WasmDumper.cpp
===
--- llvm/tools/llvm-readobj/WasmDumper.cpp
+++ llvm/tools/llvm-readobj/WasmDumper.cpp
@@ -23,8 +23,8 @@
 static const