[Lldb-commits] [PATCH] D124370: [lldb] Fix PR54761

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: shafik, clayborg.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

The issue is somewhat hard to describe (I recommend reading the bug),
but what roughly happens is that we have a type with no full definition
(-flimit-debug-info is active), but various compile units still contain
definitions of some methods of that type.

In this situation, we could run into a problem if we started to parse a
method of that type (this is for example needed to establish a
declaration context for any types defined inside that method), and the
class itself was already parsed in another compile unit. In this case,
we would hit a piece of code which would try to merge/copy the two sets
of methods.

The precise purpose of this code is not clear to me, but if it
definitely not doing the right thing in this case (and I'm having
trouble imagining a case where it would be doing the right thing). There
are several ways to to fix this problem, but the simplest one seems to
be to change the condition guarding the problematic code from:

  if (class_opaque_type.IsBeingDefined() || alternate_defn)

to plain:

  if (class_opaque_type.IsBeingDefined())

Since adding a method to a class that is not currently being defined
(i.e., we either having started defining it, or we've already completed
it) is not a good idea, any code path that has this condition false (as
ours did) is most likely erroneous.

This also ensures that the first method of such class is treated in the
same was as the second one -- the first method is not being added
either, although it ends up going through a completely different code
path.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124370

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/incomplete-types/members/Makefile
  
lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py
  lldb/test/API/lang/cpp/incomplete-types/members/a.h
  lldb/test/API/lang/cpp/incomplete-types/members/f.cpp
  lldb/test/API/lang/cpp/incomplete-types/members/g.cpp
  lldb/test/API/lang/cpp/incomplete-types/members/main.cpp

Index: lldb/test/API/lang/cpp/incomplete-types/members/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/main.cpp
@@ -0,0 +1,9 @@
+#include "a.h"
+
+A::A() = default;
+void A::anchor() {}
+
+int main() {
+  A().f();
+  A().g();
+}
Index: lldb/test/API/lang/cpp/incomplete-types/members/g.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/g.cpp
@@ -0,0 +1,8 @@
+#include "a.h"
+
+int A::g() {
+  struct Ag {
+int a, b;
+  } ag{47, 42};
+  return ag.a + ag.b; // break here
+}
Index: lldb/test/API/lang/cpp/incomplete-types/members/f.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/f.cpp
@@ -0,0 +1,8 @@
+#include "a.h"
+
+int A::f() {
+  struct Af {
+int x, y;
+  } af{42, 47};
+  return af.x + af.y; // break here
+}
Index: lldb/test/API/lang/cpp/incomplete-types/members/a.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/a.h
@@ -0,0 +1,14 @@
+#ifndef A_H
+#define A_H
+
+class A {
+public:
+  A();
+  virtual void anchor();
+  int f();
+  int g();
+
+  int member = 47;
+};
+
+#endif
Index: lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py
@@ -0,0 +1,32 @@
+"""
+Test situations where we don't have a definition for a type, but we have (some)
+of its member functions.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCppIncompleteTypeMembers(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("f.cpp"))
+
+# Sanity check that we really have to debug info for this type.
+this = self.expect_var_path("this", type="A *")
+self.assertEquals(this.GetType().GetPointeeType().GetNumberOfFields(),
+0, str(this))
+
+self.expect_var_path("af.x", value='42')
+
+lldbutil.run_break_set_by_source_regexp(self, "// break here",
+extra_options="-f g.cpp")
+self.runCmd("continue")
+
+self.expect_var_path("ag.a", value='47')
Index: lldb/test/API/lang/cpp/incomplete-types/members/Makefile
===
--- /dev/null
+++ lldb/t

[Lldb-commits] [PATCH] D124000: [lldb] Add FixAnyAddress to ABI plugins

[Muhammad Omair Javaid via Phabricator via lldb-commits]

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

Looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124000

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

[Lldb-commits] [PATCH] D122411: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

Very corner case but Is there a possibility of masks changing positions during 
the life-time of an inferior ? So this makes me think that code/data masks are 
per process and cant be changes once set. Should we worry about it?AFAIK this 
is fine but just asking to clear any ambiguities.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122411

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

[Lldb-commits] [PATCH] D122411: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

The current assumption is that once you've got a valid (currently anything 
non-zero) mask then that's it, it's not going to change. I think a future 
extension could change the meaning of the existing non-address bits at runtime, 
but that wouldn't be a problem unless you're something like the tagging 
commands. 99% of the time we want to remove all non-address bits no matter what 
they mean.

Changing the virtual address size is the real problem if it ever happens but 
given that the stack is assumed to be at the top of memory, it would be 
difficult. Shrinking means cutting off/moving the stack down (with some kind of 
position independent stack?) and growing it means you can't make top of stack 
== end of memory assumptions. You could do it with some kind of fork or restart 
but I think lldb would just handle that fine like it normally does.

Side note: the "0 means not read yet" pattern seems like it could lead to some 
waste with a target that genuinely has no mask but that's not relevant to this 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122411

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

[Lldb-commits] [PATCH] D122411: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

There's some argument that the thing asking for the mask shouldn't ask so early 
but I think the logic here is still an improvement regardless. If we don't have 
a thread yet we can't have a mask.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122411

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

[Lldb-commits] [PATCH] D122411: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> If we don't have a thread yet we can't have a mask.

Though this isn't a watertight argument since I think Mach corefiles can have a 
note that tells you the number of addressable bits. In our case there's nothing 
like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122411

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

[Lldb-commits] [PATCH] D123500: [lldb][NFC] Add more tests for GenerateOptionsUsage

[David Spickett via Phabricator via lldb-commits]

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

I realised that in theory the tests could still pass if your ordering
just happend to land in the right way.

For the short option `[-ABCabc]` there's not a lot I can do about that,
so I added another check for a different command. More chance to catch
non-determinism. I don't think any commands have enough options to make
it 100% sure.

For the detailed options I switched from a regex to cutting up the
output with python. Then once we've got all the short names we can
check the whole order.

(and I checked, when it fails you do get a list specific comparison output)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123500

Files:
  lldb/test/API/commands/help/TestHelp.py


Index: lldb/test/API/commands/help/TestHelp.py
===
--- lldb/test/API/commands/help/TestHelp.py
+++ lldb/test/API/commands/help/TestHelp.py
@@ -243,3 +243,63 @@
 "-f  ( --format  )", "The format to use for 
each of the value to be written.",
 "-s  ( --size  )", "The size in bytes to 
write from input file or each value."])
 
+@no_debug_info_test
+def test_help_shows_optional_short_options(self):
+"""Test that optional short options are printed and that they are in
+   alphabetical order with upper case options first."""
+self.expect("help memory read",
+substrs=["memory read [-br]", "memory read [-AFLORTr]"])
+self.expect("help target modules lookup",
+substrs=["target modules lookup [-Airv]"])
+
+@no_debug_info_test
+def test_help_shows_command_options_usage(self):
+"""Test that we start the usage section with a specific line."""
+self.expect("help memory read", substrs=["Command Options Usage:\n  
memory read"])
+
+@no_debug_info_test
+def test_help_detailed_information_spacing(self):
+"""Test that we put a break between the usage and the options help 
lines,
+   and between the options themselves."""
+self.expect("help memory read", substrs=[
+"[]\n\n   --show-tags",
+# Starts with the end of the show-tags line
+"output).\n\n   -A"])
+
+@no_debug_info_test
+def test_help_detailed_information_ordering(self):
+"""Test that we order options alphabetically, upper case first."""
+# You could test this with a simple regex like:
+# .*.*.*
+# Except that that could pass sometimes even with shuffled output.
+# This makes sure that doesn't happen.
+
+self.runCmd("help memory read")
+got = self.res.GetOutput()
+_, options_lines = got.split("Command Options Usage:")
+options_lines = options_lines.lstrip().splitlines()
+
+# Skip over "memory read [-xyz] lines.
+while("memory read" in options_lines[0]):
+options_lines.pop(0)
+# Plus the newline after that.
+options_lines.pop(0)
+
+short_options = []
+for line in options_lines:
+# Ignore line breaks and descriptions.
+# (not stripping the line here in case some line of the 
descriptions
+# happens to start with "-")
+if not line or not line.startswith("   -"):
+continue
+# This apears at the end after the options.
+if "This command takes options and free form arguments." in line:
+break
+line = line.strip()
+# The order of -- only options is not enforced so ignore their 
position.
+if not line.startswith("--"):
+# Save its short char name.
+short_options.append(line[1])
+
+self.assertEqual(sorted(short_options), short_options,
+ "Short option help displayed in an incorrect order!")


Index: lldb/test/API/commands/help/TestHelp.py
===
--- lldb/test/API/commands/help/TestHelp.py
+++ lldb/test/API/commands/help/TestHelp.py
@@ -243,3 +243,63 @@
 "-f  ( --format  )", "The format to use for each of the value to be written.",
 "-s  ( --size  )", "The size in bytes to write from input file or each value."])
 
+@no_debug_info_test
+def test_help_shows_optional_short_options(self):
+"""Test that optional short options are printed and that they are in
+   alphabetical order with upper case options first."""
+self.expect("help memory read",
+substrs=["memory read [-br]", "memory read [-AFLORTr]"])
+self.expect("help target modules lookup",
+substrs=["target modules lookup [-Airv]"])
+
+@no_debug_info_test
+def test_help_shows_comm

[Lldb-commits] [PATCH] D124338: [lldb] Update online help text (consistency, typo)

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM with the inline comment addressed.

Do you need someone to commit this for you?




Comment at: lldb/source/Commands/Options.td:155
 Arg<"Selector">, Required,
-Desc<"Set the breakpoint by ObjC selector name. Can be repeated multiple "
-"times to make one breakpoint for multiple Selectors.">;
+Desc<"Set the breakpoint by Objective-C selector name. Can be repeated "
+"multiple times to make one breakpoint for multiple Selectors.">;

Seems like this line needs an extra space as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124338

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

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

[Nikolas Klauser 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 rG29c8c070a177: [libc++] Use bit field for checking if string 
is in long or short mode (authored by philnik).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123580

Files:
  libcxx/include/string
  libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
  libcxx/test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp
  libcxx/utils/gdb/libcxx/printers.py

Index: libcxx/utils/gdb/libcxx/printers.py
===
--- libcxx/utils/gdb/libcxx/printers.py
+++ libcxx/utils/gdb/libcxx/printers.py
@@ -192,26 +192,6 @@
 class StdStringPrinter(object):
 """Print a std::string."""
 
-def _get_short_size(self, short_field, short_size):
-"""Short size depends on both endianness and a compile-time define."""
-
-# If the padding field is present after all this indirection, then string
-# was compiled with _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT defined.
-field = short_field.type.fields()[1].type.fields()[0]
-libcpp_abi_alternate_string_layout = field.name and "__padding" in field.name
-
-# This logical structure closely follows the original code (which is clearer
-# in C++).  Keep them parallel to make them easier to compare.
-if libcpp_abi_alternate_string_layout:
-if _libcpp_big_endian:
-return short_size >> 1
-else:
-return short_size
-elif _libcpp_big_endian:
-return short_size
-else:
-return short_size >> 1
-
 def __init__(self, val):
 self.val = val
 
@@ -223,18 +203,13 @@
 short_size = short_field["__size_"]
 if short_size == 0:
 return ""
-short_mask = self.val["__short_mask"]
-# Counter intuitive to compare the size and short_mask to see if the string
-# is long, but that's the way the implementation does it. Note that
-# __is_long() doesn't use get_short_size in C++.
-is_long = short_size & short_mask
-if is_long:
+if short_field["__is_long_"]:
 long_field = value_field["__l"]
 data = long_field["__data_"]
 size = long_field["__size_"]
 else:
 data = short_field["__data_"]
-size = self._get_short_size(short_field, short_size)
+size = short_field["__size_"]
 return data.lazy_string(length=size)
 
 def display_hint(self):
Index: libcxx/test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp
===
--- libcxx/test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp
+++ libcxx/test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp
@@ -9,6 +9,11 @@
 // UNSUPPORTED: no-exceptions
 // XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11}}
 
+// Prior to http://llvm.org/D123580, there was a bug with how the max_size()
+// was calculated. That was inlined into some functions in the dylib, which leads
+// to failures when running this test against an older system dylib.
+// XFAIL: use_system_cxx_lib && target=arm64-apple-macosx{{11.0|12.0}}
+
 // 
 
 // size_type max_size() const;
Index: libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
===
--- /dev/null
+++ libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
@@ -0,0 +1,125 @@
+//===--===//
+//
+// 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
+//
+//===--===//
+
+// UNSUPPORTED:
+
+// 
+
+// This test ensures that the correct max_size() is returned depending on the platform.
+
+#include 
+#include 
+#include 
+#include 
+
+#include "test_macros.h"
+
+// alignment of the string heap buffer is hardcoded to 16
+static const size_t alignment = 16;
+
+void full_size() {
+  std::string str;
+  assert(str.max_size() == std::numeric_limits::max() - alignment);
+
+#ifndef TEST_HAS_NO_CHAR8_T
+  std::u8string u8str;
+  assert(u8str.max_size() == std::numeric_limits::max() - alignment);
+#endif
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  std::wstring wstr;
+  assert(wstr.max_size() == std::numeric_limits::max() / sizeof(wchar_t) - alignment);
+#endif
+
+#ifndef TEST_HAS_NO_UNICODE_CHARS
+  std::u16string u16str;
+  std::u32string u32str;
+  assert(u16str.max_size() == std::numeric_limits::max() / 2 - alignment);
+  asse

[Lldb-commits] [PATCH] D121876: [BOLT][DWARF] Implement monolithic DWARF5

[Maksim Panchenko via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121876

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

[Lldb-commits] [PATCH] D121876: [BOLT][DWARF] Implement monolithic DWARF5.

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added inline comments.



Comment at: 
lldb/test/Shell/SymbolFile/DWARF/x86/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s:17
 
-# RUN: %clang_host -o %t %s \
+# RUN: %clang_host -gdwarf-4 -o %t %s \
 # RUN:   
%S/Inputs/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu2.s

DavidSpickett wrote:
> Was this test just using whatever DWARF `%clang_host` defaulted to? (since I 
> don't see any changes outside of bolt other than this)
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121876

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

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

[Louis Dionne via Phabricator via lldb-commits]

ldionne added inline comments.



Comment at: libcxx/utils/gdb/libcxx/printers.py:192
 
 class StdStringPrinter(object):
 """Print a std::string."""

philnik wrote:
> labath wrote:
> > philnik wrote:
> > > dblaikie wrote:
> > > > philnik wrote:
> > > > > jgorbe wrote:
> > > > > > Mordante wrote:
> > > > > > > philnik wrote:
> > > > > > > > Mordante wrote:
> > > > > > > > > philnik wrote:
> > > > > > > > > > Mordante wrote:
> > > > > > > > > > > Does this also break the LLDB pretty printer?
> > > > > > > > > > Probably. Would be nice to have a test runner for that.
> > > > > > > > > I already planned to look into that, D97044#3440904 ;-)
> > > > > > > > Do you know where I would have to look to know what the LLDB 
> > > > > > > > pretty printers do?
> > > > > > > Unfortunately no. @jingham seems to be the Data formatter code 
> > > > > > > owner.
> > > > > > There was a recent lldb change fixing prettyprinters after a 
> > > > > > similar change to string: 
> > > > > > https://github.com/llvm/llvm-project/commit/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe
> > > > > > 
> > > > > > If the gdb prettyprinter needed fixing for this change, chances are 
> > > > > > that lldb will need a similar update too.
> > > > > Could someone from #lldb help me figure out what to change in the 
> > > > > pretty printers? I looked at the file, but I don't really understand 
> > > > > how it works and TBH I don't really feel like spending a lot of time 
> > > > > figuring it out. If nobody says anything I'll land this in a week.
> > > > > 
> > > > > As a side note: it would be really nice if there were a few more 
> > > > > comments inside `LibCxx.cpp` to explain what happens there. That 
> > > > > would make fixing the pretty printer a lot easier. The code is 
> > > > > probably not very hard (at least it doesn't look like it), but I am 
> > > > > looking for a few things that I can't find and I have no idea what 
> > > > > some of the things mean.
> > > > Looks like something around 
> > > > https://github.com/llvm/llvm-project/blob/2e6ac54cf48aa04f7b05c382c33135b16d3f01ea/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp#L597
> > > >  (& the similar masking in the `else` block a few lines down) - I guess 
> > > > a specific lookup for the new field would be needed, rather than the 
> > > > bitmasking.
> > > Yes, but what do the numbers in `size_mode_locations` mean? Why is there 
> > > no checking if it's big or little endian? Is it unsupported maybe? Does 
> > > it work because of something else? Is there a reason that `g_data_name` 
> > > exists instead of comparing directly? Should I add another layout style 
> > > or should I just update the code for the new layout?
> > > I don't know anything about the LLDB codebase, so I don't understand the 
> > > code and I don't know how I should change it.
> > I don't think there's been any official policy decision either way, but 
> > historically we haven't been asking libc++ authors to update lldb pretty 
> > printers -- we would just fix them up on the lldb side when we noticed the 
> > change. The thing that has changed recently is that google started relying 
> > (and testing) more on lldb, which considerably shortened the time it takes 
> > to notice this change, and also makes it difficult for some people to make 
> > progress while we are in this state. But I don't think that means that 
> > updating the pretty printer is suddenly your responsibility.
> > 
> > As for your questions, I'll try to answer them as best as I can:
> > > what do the numbers in size_mode_locations mean?
> > These are the indexes of fields in the string object. For some reason 
> > (unknown to me), the pretty printer uses indexes rather than field names 
> > for its work. Prompted by the previous patch, I've been trying to change 
> > that, but I haven't done it yet, as I was trying to improve the testing 
> > story (more on that later).
> > > Why is there no checking if it's big or little endian? Is it unsupported 
> > > maybe?
> > Most likely yes. Although most parts of lldb support big endian, I am not 
> > aware of anyone testing it on a regular basis, so it's quite likely that a 
> > lot of things are in fact broken.
> > > Is there a reason that g_data_name exists instead of comparing directly?
> > LLDB uses a global string pool, so this is an attempt to reduce the number 
> > of string pool queries. The pattern is not consistently used everywhere, 
> > and overall, I wouldn't be too worried about it.
> > > Should I add another layout style or should I just update the code for 
> > > the new layout?
> > As the pretty printers ship with lldb, they are expected to support not 
> > just the current format, but also the past ones (within reason). This is 
> > what makes adding a new format (or just refactoring the existing code) 
> > difficult, and it's why I was trying to come up with better tests for this 
> > (it remains to be seen if I am successful).
> > 
> > A

[Lldb-commits] [PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

[Fangrui Song via Phabricator via lldb-commits]

MaskRay closed this revision.
MaskRay added a comment.

a749e3295df4aee18a0ad723875a6501f30ac744 
 pushed by 
Aaron does not have a `Differential Revision:` line. Manual closing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121078

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

[Lldb-commits] [PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

[Tanya Lattner via Phabricator via lldb-commits]

tonic added a comment.

In D121078#3402469 , @aaron.ballman 
wrote:

> In D121078#3400810 , @SimplyDanny 
> wrote:
>
>> @tonic, can you please have another look?
>
> If @tonic has any additional feedback, we can handle it post-commit at this 
> point. I've gone ahead and committed this on your behalf in 
> a749e3295df4aee18a0ad723875a6501f30ac744 
> .
>
> (Note, I can't close this review because of the "changes requested" state for 
> tonic.)

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121078

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

[Lldb-commits] [PATCH] D124370: [lldb] Fix PR54761

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

This mostly makes sense, the purpose of the `|| alternate_defn` was not clear 
to me either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124370

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

[Lldb-commits] [PATCH] D124370: [lldb] Fix PR54761

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

So I believe the reason we need to be able to add methods to a class is for 
templates. Templates in DWARF are really poorly emitted all in the name of 
saving space. There is no full definition of template classes that get emitted, 
just a class definition with _only_ the methods that the user used in the 
current compile unit. DWARF doesn't really support emitting a templatized 
definition of a class in terms of a type T, it will always emit instantiations 
of the type T directly. So in LLDB we must create a template class like 
"std::vector" and say it has no member functions, and then add each member 
function as a type specific specialization due to how the types must be created 
in the clang::ASTContext.

in one DWARF compile unit you end up having say "std::vector::clear()" and 
in another you would have "std::vector::erase()". In order to make the 
most complete definition of template types we need to find all 
"std::vector" definitions and manually add any method that we haven't 
already seen, otherwise we end up not being able to call methods that might 
exist. Of course calling template functions is already fraught with danger 
since most are inlined if they come from the STL, but if the user asks for the 
class definition for "std::vector", we should show all that we can. Can 
you make sure this patch doesn't hurt that functionality? We must have a test 
for it.

So we can't just say we will accept just one class definition if we have 
template types. Given this information, let me know what you think of your 
current patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124370

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

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

LGTM. Anyone else have any issues?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

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

[Lldb-commits] [PATCH] D124409: Filter non-external static members from SBType::GetFieldAtIndex.

[Sigurður Ásgeirsson via Phabricator via lldb-commits]

siggi-alpheus created this revision.
siggi-alpheus added a reviewer: labath.
siggi-alpheus added a project: LLDB.
Herald added subscribers: JDevlieghere, arphaman.
Herald added a reviewer: shafik.
Herald added a project: All.
siggi-alpheus requested review of this revision.
Herald added a subscriber: lldb-commits.

See issue 55040  where 
static members of classes declared in the anonymous namespace are incorrectly 
returned as member fields from lldb::SBType::GetFieldAtIndex(). It appears that 
attrs.member_byte_offset contains a sentinel value for members that don't have 
a DW_AT_data_member_location.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124409

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2669,7 +2669,7 @@
 attrs.accessibility = eAccessNone;
 
   // Handle static members
-  if (attrs.is_external && attrs.member_byte_offset == UINT32_MAX) {
+  if (attrs.member_byte_offset == UINT32_MAX) {
 Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
 
 if (var_type) {


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2669,7 +2669,7 @@
 attrs.accessibility = eAccessNone;
 
   // Handle static members
-  if (attrs.is_external && attrs.member_byte_offset == UINT32_MAX) {
+  if (attrs.member_byte_offset == UINT32_MAX) {
 Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
 
 if (var_type) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D124370: [lldb] Fix PR54761

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

I take it no tests fail upon removing this condition? Any hints in version 
history about its purpose?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124370

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

[Lldb-commits] [PATCH] D123982: [trace][intel pt] Support events

[Jakob Johnson via Phabricator via lldb-commits]

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

Thanks for making those changes - lgtm!




Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:258
   /// first one. Otherwise, this map will be empty.
   std::map m_instruction_timestamps;
   /// This is the chronologically last TSC that has been added.

wallace wrote:
> jj10306 wrote:
> > wallace wrote:
> > > jj10306 wrote:
> > > > jj10306 wrote:
> > > > > I know this isn't related to these changes, but this should be 
> > > > > updated to be consistent with the other `instruction_index -> XXX` 
> > > > > maps in this class.
> > > > 
> > > in this case we can't do that, because when doing random accesses in the 
> > > trace, we need to quickly find the most recent TSC for the new 
> > > instruction, which is done with binary search using a map. This is 
> > > impossible in a hash map like DenseMap
> > I know this is a pedantic discussion, but isn't lookup in hash tables 
> > amortized O(1) (assuming a good hash that doesn't take too long to execute 
> > or produce the same hash for different values) whereas in an ordered map 
> > it's O(logn)?
> > IIUC, you should only use an ordered map when you rely on the order 
> > property of it.
> > 
> > In any case, we should use `uint64_t` here instead of `size_t` to be 
> > consistent with the type for instruction index used in the other maps.
> > In any case, we should use uint64_t here instead of size_t to be consistent 
> > with the type for instruction index used in the other maps.
> 
> you are right with this one.
> 
> > IIUC, you should only use an ordered map when you rely on the order 
> > property of it.
> 
> yes, and that's what we use for the timestamps. As there are a small number 
> of unique timestamps, we are keeping this map 'insn_index -> timestamp'. We 
> need its order property because when we do GoToId(insn_id) we need to look 
> for most recent entry in the map that has a timestamp, and we use that 
> timestamp for our new instruction. We use map.lower_bound for that. With a 
> hashmap we couldn't do that.
>We use map.lower_bound for that. With a hashmap we couldn't do that.
Ahhh yes, that makes sense - I forgot we were using that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123982

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

[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo updated this revision to Diff 425001.
ayermolo added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

addressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122988

Files:
  cross-project-tests/lit.cfg.py
  lldb/test/API/lit.cfg.py
  lldb/test/Shell/helper/toolchain.py


Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -140,7 +140,7 @@
 
 # Facebook T92898286
 if config.llvm_test_bolt:
-host_flags += ['--post-link-optimize', '-fdebug-default-version=4']
+host_flags += ['--post-link-optimize']
 # End Facebook T92898286
 
 host_flags = ' '.join(host_flags)
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -205,8 +205,7 @@
 
 # Facebook T92898286
 if is_configured("llvm_test_bolt"):
-dotest_cmd += ['-E', '"--post-link-optimize"',
-   '-E', '"-fdebug-default-version=4"']
+dotest_cmd += ['-E', '"--post-link-optimize"']
 # End Facebook T92898286
 
 if 'lldb-repro-capture' in config.available_features or \
Index: cross-project-tests/lit.cfg.py
===
--- cross-project-tests/lit.cfg.py
+++ cross-project-tests/lit.cfg.py
@@ -77,7 +77,7 @@
 # Facebook T92898286
 should_test_bolt = get_required_attr(config, "llvm_test_bolt")
 if should_test_bolt:
-llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects), 
additional_flags=['--post-link-optimize', '-fdebug-default-version=4'])
+llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects), 
additional_flags=['--post-link-optimize'])
 else:
 llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects))
 # End Facebook T92898286


Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -140,7 +140,7 @@
 
 # Facebook T92898286
 if config.llvm_test_bolt:
-host_flags += ['--post-link-optimize', '-fdebug-default-version=4']
+host_flags += ['--post-link-optimize']
 # End Facebook T92898286
 
 host_flags = ' '.join(host_flags)
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -205,8 +205,7 @@
 
 # Facebook T92898286
 if is_configured("llvm_test_bolt"):
-dotest_cmd += ['-E', '"--post-link-optimize"',
-   '-E', '"-fdebug-default-version=4"']
+dotest_cmd += ['-E', '"--post-link-optimize"']
 # End Facebook T92898286
 
 if 'lldb-repro-capture' in config.available_features or \
Index: cross-project-tests/lit.cfg.py
===
--- cross-project-tests/lit.cfg.py
+++ cross-project-tests/lit.cfg.py
@@ -77,7 +77,7 @@
 # Facebook T92898286
 should_test_bolt = get_required_attr(config, "llvm_test_bolt")
 if should_test_bolt:
-llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects), additional_flags=['--post-link-optimize', '-fdebug-default-version=4'])
+llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects), additional_flags=['--post-link-optimize'])
 else:
 llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects))
 # End Facebook T92898286
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D124110: Refactor protected virtual functions from SymbolFile into new SymbolFileActual class.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

Before I read the comments I was going to make the same suggestion as Greg, but 
this makes sense to limit churn. LGMT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124110

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

[Lldb-commits] [PATCH] D124338: [lldb] Update online help text (consistency, typo)

[Will Hawkins via Phabricator via lldb-commits]

hawkinsw updated this revision to Diff 425032.
hawkinsw added a comment.

Updating thanks to @jdevlieghere feedback!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124338

Files:
  lldb/source/Commands/Options.td


Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -95,7 +95,7 @@
   def breakpoint_modify_command : Option<"command", "C">, Group<4>,
 Arg<"Command">,
 Desc<"A command to run when the breakpoint is hit, can be provided more "
-"than once, the commands will get run in order left to right.">;
+"than once, the commands will be run in left-to-right order.">;
 }
 
 let Command = "breakpoint dummy" in {
@@ -132,8 +132,8 @@
 "no matter where the binary eventually loads.  Alternately, if you also "
 "specify the module - with the -s option - then the address will be "
 "treated as a file address in that module, and resolved accordingly.  "
-"Again, this will allow lldb to track that offset on subsequent reloads. "
-" The module need not have been loaded at the time you specify this "
+"Again, this will allow lldb to track that offset on subsequent reloads.  "
+"The module need not have been loaded at the time you specify this "
 "breakpoint, and will get resolved when the module is loaded.">;
   def breakpoint_set_name : Option<"name", "n">, Group<3>, Arg<"FunctionName">,
 Completion<"Symbol">, Required,
@@ -152,8 +152,8 @@
 " to make one breakpoint for multiple names.">;
   def breakpoint_set_selector : Option<"selector", "S">, Group<5>,
 Arg<"Selector">, Required,
-Desc<"Set the breakpoint by ObjC selector name. Can be repeated multiple "
-"times to make one breakpoint for multiple Selectors.">;
+Desc<"Set the breakpoint by Objective-C selector name.  Can be repeated "
+"multiple times to make one breakpoint for multiple Selectors.">;
   def breakpoint_set_method : Option<"method", "M">, Group<6>, Arg<"Method">,
 Required, Desc<"Set the breakpoint by C++ method names.  Can be repeated "
 "multiple times to make one breakpoint for multiple methods.">;


Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -95,7 +95,7 @@
   def breakpoint_modify_command : Option<"command", "C">, Group<4>,
 Arg<"Command">,
 Desc<"A command to run when the breakpoint is hit, can be provided more "
-"than once, the commands will get run in order left to right.">;
+"than once, the commands will be run in left-to-right order.">;
 }
 
 let Command = "breakpoint dummy" in {
@@ -132,8 +132,8 @@
 "no matter where the binary eventually loads.  Alternately, if you also "
 "specify the module - with the -s option - then the address will be "
 "treated as a file address in that module, and resolved accordingly.  "
-"Again, this will allow lldb to track that offset on subsequent reloads. "
-" The module need not have been loaded at the time you specify this "
+"Again, this will allow lldb to track that offset on subsequent reloads.  "
+"The module need not have been loaded at the time you specify this "
 "breakpoint, and will get resolved when the module is loaded.">;
   def breakpoint_set_name : Option<"name", "n">, Group<3>, Arg<"FunctionName">,
 Completion<"Symbol">, Required,
@@ -152,8 +152,8 @@
 " to make one breakpoint for multiple names.">;
   def breakpoint_set_selector : Option<"selector", "S">, Group<5>,
 Arg<"Selector">, Required,
-Desc<"Set the breakpoint by ObjC selector name. Can be repeated multiple "
-"times to make one breakpoint for multiple Selectors.">;
+Desc<"Set the breakpoint by Objective-C selector name.  Can be repeated "
+"multiple times to make one breakpoint for multiple Selectors.">;
   def breakpoint_set_method : Option<"method", "M">, Group<6>, Arg<"Method">,
 Required, Desc<"Set the breakpoint by C++ method names.  Can be repeated "
 "multiple times to make one breakpoint for multiple methods.">;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D124338: [lldb] Update online help text (consistency, typo)

[Will Hawkins via Phabricator via lldb-commits]

hawkinsw added a comment.

Added that extra space, as requested! And, yes, I will need someone to land 
this one for me. I am going to ask for commit access, but so far have not! 
Thanks for your help, @JDevlieghere !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124338

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

[Lldb-commits] [PATCH] D111209: Don't push null ExecutionContext on CommandInterpreter exectx stack

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.
Herald added a project: All.

Hi all, getting back to this one after six months, hah.  Reading through Jim's 
thoughts on this, I think we're OK to add this refinement.  Tatyana also 
thought that removing this altogether would be an option, but this has been 
sitting unfixed for a while and I'd like to land the fix to it.  Jim wishes for 
a world where HandleCommand requires an SBExecutionContext, or one in which we 
had an established way of deprecating old SB API.  Given that we can't address 
that at the moment, I'm going to land this patch finally so we're not carrying 
the problem around for the foreseeable future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111209

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

[Lldb-commits] [lldb] 827ff1e - [LLDB][NativePDB] Fix incorrect file index of inlinees introduced by f00cd23caed5f920495bcae2055f4c478d8383d6

[Zequan Wu via lldb-commits]

Author: Zequan Wu
Date: 2022-04-25T16:07:04-07:00
New Revision: 827ff1e576f71fd3b81c4b012bf852eb6f46f808

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

LOG: [LLDB][NativePDB] Fix incorrect file index of inlinees introduced by 
f00cd23caed5f920495bcae2055f4c478d8383d6

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 00d4422bc979e..57b3cca47daa8 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1225,9 +1225,6 @@ bool SymbolFileNativePDB::ParseDebugMacros(CompileUnit 
&comp_unit) {
 llvm::Expected
 SymbolFileNativePDB::GetFileIndex(const CompilandIndexItem &cii,
   uint32_t file_id) {
-  auto index_iter = m_file_indexes.find(file_id);
-  if (index_iter != m_file_indexes.end())
-return index_iter->getSecond();
   const auto &checksums = cii.m_strings.checksums().getArray();
   const auto &strings = cii.m_strings.strings();
   // Indices in this structure are actually offsets of records in the
@@ -1244,9 +1241,9 @@ SymbolFileNativePDB::GetFileIndex(const 
CompilandIndexItem &cii,
 
   // LLDB wants the index of the file in the list of support files.
   auto fn_iter = llvm::find(cii.m_file_list, *efn);
-  lldbassert(fn_iter != cii.m_file_list.end());
-  m_file_indexes[file_id] = std::distance(cii.m_file_list.begin(), fn_iter);
-  return m_file_indexes[file_id];
+  if (fn_iter != cii.m_file_list.end())
+return std::distance(cii.m_file_list.begin(), fn_iter);
+  return llvm::make_error(raw_error_code::no_entry);
 }
 
 bool SymbolFileNativePDB::ParseSupportFiles(CompileUnit &comp_unit,

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
index f1b6e34ca346b..d626daf4e95d3 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -272,8 +272,6 @@ class SymbolFileNativePDB : public SymbolFile {
   llvm::DenseMap m_compilands;
   llvm::DenseMap m_types;
   llvm::DenseMap> m_inline_sites;
-  // A map from file id in records to file index in support files.
-  llvm::DenseMap m_file_indexes;
 };
 
 } // namespace npdb



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

[Lldb-commits] [lldb] e07c092 - [lldb] Update online help text (consistency, typo)

[Jonas Devlieghere via lldb-commits]

Author: Will Hawkins
Date: 2022-04-25T16:31:26-07:00
New Revision: e07c092b8529c6ef0f7ba722fd7188b1bc731d8c

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

LOG: [lldb] Update online help text (consistency, typo)

Update the online help text for breakpoint set to be
consistent with respect to the spelling of Objective-C
and fix a few space-related typos.

Differential revision: https://reviews.llvm.org/D124338

Added: 


Modified: 
lldb/source/Commands/Options.td

Removed: 




diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 35924ad6f558d..bea4ff510b2ca 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -95,7 +95,7 @@ let Command = "breakpoint modify" in {
   def breakpoint_modify_command : Option<"command", "C">, Group<4>,
 Arg<"Command">,
 Desc<"A command to run when the breakpoint is hit, can be provided more "
-"than once, the commands will get run in order left to right.">;
+"than once, the commands will be run in left-to-right order.">;
 }
 
 let Command = "breakpoint dummy" in {
@@ -132,8 +132,8 @@ let Command = "breakpoint set" in {
 "no matter where the binary eventually loads.  Alternately, if you also "
 "specify the module - with the -s option - then the address will be "
 "treated as a file address in that module, and resolved accordingly.  "
-"Again, this will allow lldb to track that offset on subsequent reloads. "
-" The module need not have been loaded at the time you specify this "
+"Again, this will allow lldb to track that offset on subsequent reloads.  "
+"The module need not have been loaded at the time you specify this "
 "breakpoint, and will get resolved when the module is loaded.">;
   def breakpoint_set_name : Option<"name", "n">, Group<3>, Arg<"FunctionName">,
 Completion<"Symbol">, Required,
@@ -152,8 +152,8 @@ let Command = "breakpoint set" in {
 " to make one breakpoint for multiple names.">;
   def breakpoint_set_selector : Option<"selector", "S">, Group<5>,
 Arg<"Selector">, Required,
-Desc<"Set the breakpoint by ObjC selector name. Can be repeated multiple "
-"times to make one breakpoint for multiple Selectors.">;
+Desc<"Set the breakpoint by Objective-C selector name.  Can be repeated "
+"multiple times to make one breakpoint for multiple Selectors.">;
   def breakpoint_set_method : Option<"method", "M">, Group<6>, Arg<"Method">,
 Required, Desc<"Set the breakpoint by C++ method names.  Can be repeated "
 "multiple times to make one breakpoint for multiple methods.">;



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

[Lldb-commits] [PATCH] D124338: [lldb] Update online help text (consistency, typo)

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe07c092b8529: [lldb] Update online help text (consistency, 
typo) (authored by hawkinsw, committed by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124338

Files:
  lldb/source/Commands/Options.td


Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -95,7 +95,7 @@
   def breakpoint_modify_command : Option<"command", "C">, Group<4>,
 Arg<"Command">,
 Desc<"A command to run when the breakpoint is hit, can be provided more "
-"than once, the commands will get run in order left to right.">;
+"than once, the commands will be run in left-to-right order.">;
 }
 
 let Command = "breakpoint dummy" in {
@@ -132,8 +132,8 @@
 "no matter where the binary eventually loads.  Alternately, if you also "
 "specify the module - with the -s option - then the address will be "
 "treated as a file address in that module, and resolved accordingly.  "
-"Again, this will allow lldb to track that offset on subsequent reloads. "
-" The module need not have been loaded at the time you specify this "
+"Again, this will allow lldb to track that offset on subsequent reloads.  "
+"The module need not have been loaded at the time you specify this "
 "breakpoint, and will get resolved when the module is loaded.">;
   def breakpoint_set_name : Option<"name", "n">, Group<3>, Arg<"FunctionName">,
 Completion<"Symbol">, Required,
@@ -152,8 +152,8 @@
 " to make one breakpoint for multiple names.">;
   def breakpoint_set_selector : Option<"selector", "S">, Group<5>,
 Arg<"Selector">, Required,
-Desc<"Set the breakpoint by ObjC selector name. Can be repeated multiple "
-"times to make one breakpoint for multiple Selectors.">;
+Desc<"Set the breakpoint by Objective-C selector name.  Can be repeated "
+"multiple times to make one breakpoint for multiple Selectors.">;
   def breakpoint_set_method : Option<"method", "M">, Group<6>, Arg<"Method">,
 Required, Desc<"Set the breakpoint by C++ method names.  Can be repeated "
 "multiple times to make one breakpoint for multiple methods.">;


Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -95,7 +95,7 @@
   def breakpoint_modify_command : Option<"command", "C">, Group<4>,
 Arg<"Command">,
 Desc<"A command to run when the breakpoint is hit, can be provided more "
-"than once, the commands will get run in order left to right.">;
+"than once, the commands will be run in left-to-right order.">;
 }
 
 let Command = "breakpoint dummy" in {
@@ -132,8 +132,8 @@
 "no matter where the binary eventually loads.  Alternately, if you also "
 "specify the module - with the -s option - then the address will be "
 "treated as a file address in that module, and resolved accordingly.  "
-"Again, this will allow lldb to track that offset on subsequent reloads. "
-" The module need not have been loaded at the time you specify this "
+"Again, this will allow lldb to track that offset on subsequent reloads.  "
+"The module need not have been loaded at the time you specify this "
 "breakpoint, and will get resolved when the module is loaded.">;
   def breakpoint_set_name : Option<"name", "n">, Group<3>, Arg<"FunctionName">,
 Completion<"Symbol">, Required,
@@ -152,8 +152,8 @@
 " to make one breakpoint for multiple names.">;
   def breakpoint_set_selector : Option<"selector", "S">, Group<5>,
 Arg<"Selector">, Required,
-Desc<"Set the breakpoint by ObjC selector name. Can be repeated multiple "
-"times to make one breakpoint for multiple Selectors.">;
+Desc<"Set the breakpoint by Objective-C selector name.  Can be repeated "
+"multiple times to make one breakpoint for multiple Selectors.">;
   def breakpoint_set_method : Option<"method", "M">, Group<6>, Arg<"Method">,
 Required, Desc<"Set the breakpoint by C++ method names.  Can be repeated "
 "multiple times to make one breakpoint for multiple methods.">;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5cbf516 - Refactor protected virtual functions from SymbolFile into new SymbolFileCommon class.

[Jeffrey Tan via lldb-commits]

Author: Jeffrey Tan
Date: 2022-04-25T18:33:47-07:00
New Revision: 5cbf516cb79fa27395dabb33002ab20243b1ee5d

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

LOG: Refactor protected virtual functions from SymbolFile into new 
SymbolFileCommon class.

This is a preparatory patch for https://reviews.llvm.org/D121631.
It refactors protected virtual members of SymbolFile
into a new SymbolFileCommon class per suggestion in:
https://reviews.llvm.org/D121631

This will avoid the friendship declaration in that patch.

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

Added: 


Modified: 
lldb/include/lldb/Symbol/SymbolFile.h
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
lldb/source/Symbol/SymbolFile.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/SymbolFile.h 
b/lldb/include/lldb/Symbol/SymbolFile.h
index c7bb6c16f0398..c9be5dbd2b256 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -36,6 +36,12 @@
 
 namespace lldb_private {
 
+/// Provides public interface for all SymbolFiles. Any protected
+/// virtual members should go into SymbolFileCommon; most SymbolFile
+/// implementations should inherit from SymbolFileCommon to override
+/// the behaviors except SymbolFileOnDemand which inherits
+/// public interfaces from SymbolFile and forward to underlying concrete
+/// SymbolFile implementation.
 class SymbolFile : public PluginInterface {
   /// LLVM RTTI support.
   static char ID;
@@ -67,8 +73,7 @@ class SymbolFile : public PluginInterface {
   static SymbolFile *FindPlugin(lldb::ObjectFileSP objfile_sp);
 
   // Constructors and Destructors
-  SymbolFile(lldb::ObjectFileSP objfile_sp)
-  : m_objfile_sp(std::move(objfile_sp)) {}
+  SymbolFile() = default;
 
   ~SymbolFile() override = default;
 
@@ -99,15 +104,7 @@ class SymbolFile : public PluginInterface {
   /// A uint32_t mask containing bits from the SymbolFile::Abilities
   /// enumeration. Any bits that are set represent an ability that
   /// this symbol plug-in can parse from the object file.
-  uint32_t GetAbilities() {
-if (!m_calculated_abilities) {
-  m_abilities = CalculateAbilities();
-  m_calculated_abilities = true;
-}
-
-return m_abilities;
-  }
-
+  virtual uint32_t GetAbilities() = 0;
   virtual uint32_t CalculateAbilities() = 0;
 
   /// Symbols file subclasses should override this to return the Module that
@@ -125,10 +122,10 @@ class SymbolFile : public PluginInterface {
 
   // Compile Unit function calls
   // Approach 1 - iterator
-  uint32_t GetNumCompileUnits();
-  lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx);
+  virtual uint32_t GetNumCompileUnits() = 0;
+  virtual lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx) = 0;
 
-  Symtab *GetSymtab();
+  virtual Symtab *GetSymtab() = 0;
 
   virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
   /// Return the Xcode SDK comp_unit was compiled against.
@@ -256,16 +253,16 @@ class SymbolFile : public PluginInterface {
   virtual void PreloadSymbols();
 
   virtual llvm::Expected
-  GetTypeSystemForLanguage(lldb::LanguageType language);
+  GetTypeSystemForLanguage(lldb::LanguageType language) = 0;
 
   virtual CompilerDeclContext
   FindNamespace(ConstString name, const CompilerDeclContext &parent_decl_ctx) {
 return CompilerDeclContext();
   }
 
-  ObjectFile *GetObjectFile() { return m_objfile_sp.get(); }
-  const ObjectFile *GetObjectFile() const { return m_objfile_sp.get(); }
-  ObjectFile *GetMainObjectFile();
+  virtual ObjectFile *GetObjectFile() = 0;
+  virtual const ObjectFile *GetObjectFile() const = 0;
+  virtual ObjectFile *GetMainObjectFile() = 0;
 
   virtual std::vector>
   ParseCallEdgesInFunction(UserID func_id) {
@@ -276,7 +273,7 @@ class SymbolFile : public PluginInterface {
 
   /// Notify the SymbolFile that the file addresses in the Sections
   /// for this module have been changed.
-  virtual void SectionFileAddressesChanged();
+  virtual void SectionFileAddressesChanged() = 0;
 
   struct RegisterInfoResolver {
 virtual ~RegisterInfoResolver(); // anchor
@@ -297,7 +294,7 

[Lldb-commits] [PATCH] D124110: Refactor protected virtual functions from SymbolFile into new SymbolFileActual class.

[jeffrey tan via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5cbf516cb79f: Refactor protected virtual functions from 
SymbolFile into new SymbolFileCommon… (authored by yinghuitan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124110

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  lldb/source/Symbol/SymbolFile.cpp

Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -25,6 +25,7 @@
 using namespace lldb;
 
 char SymbolFile::ID;
+char SymbolFileCommon::ID;
 
 void SymbolFile::PreloadSymbols() {
   // No-op for most implementations.
@@ -33,9 +34,6 @@
 std::recursive_mutex &SymbolFile::GetModuleMutex() const {
   return GetObjectFile()->GetModule()->GetMutex();
 }
-ObjectFile *SymbolFile::GetMainObjectFile() {
-  return m_objfile_sp->GetModule()->GetObjectFile();
-}
 
 SymbolFile *SymbolFile::FindPlugin(ObjectFileSP objfile_sp) {
   std::unique_ptr best_symfile_up;
@@ -87,16 +85,6 @@
   return best_symfile_up.release();
 }
 
-llvm::Expected
-SymbolFile::GetTypeSystemForLanguage(lldb::LanguageType language) {
-  auto type_system_or_err =
-  m_objfile_sp->GetModule()->GetTypeSystemForLanguage(language);
-  if (type_system_or_err) {
-type_system_or_err->SetSymbolFile(this);
-  }
-  return type_system_or_err;
-}
-
 uint32_t
 SymbolFile::ResolveSymbolContext(const SourceLocationSpec &src_location_spec,
  lldb::SymbolContextItem resolve_scope,
@@ -154,7 +142,37 @@
 #endif
 }
 
-uint32_t SymbolFile::GetNumCompileUnits() {
+SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
+
+Symtab *SymbolFileCommon::GetSymtab() {
+  std::lock_guard guard(GetModuleMutex());
+  if (m_symtab)
+return m_symtab;
+
+  // Fetch the symtab from the main object file.
+  m_symtab = GetMainObjectFile()->GetSymtab();
+
+  // Then add our symbols to it.
+  if (m_symtab)
+AddSymbols(*m_symtab);
+
+  return m_symtab;
+}
+
+ObjectFile *SymbolFileCommon::GetMainObjectFile() {
+  return m_objfile_sp->GetModule()->GetObjectFile();
+}
+
+void SymbolFileCommon::SectionFileAddressesChanged() {
+  ObjectFile *module_objfile = GetMainObjectFile();
+  ObjectFile *symfile_objfile = GetObjectFile();
+  if (symfile_objfile != module_objfile)
+symfile_objfile->SectionFileAddressesChanged();
+  if (m_symtab)
+m_symtab->SectionFileAddressesChanged();
+}
+
+uint32_t SymbolFileCommon::GetNumCompileUnits() {
   std::lock_guard guard(GetModuleMutex());
   if (!m_compile_units) {
 // Create an array of compile unit shared pointers -- which will each
@@ -164,7 +182,7 @@
   return m_compile_units->size();
 }
 
-CompUnitSP SymbolFile::GetCompileUnitAtIndex(uint32_t idx) {
+CompUnitSP SymbolFileCommon::GetCompileUnitAtIndex(uint32_t idx) {
   std::lock_guard guard(GetModuleMutex());
   uint32_t num = GetNumCompileUnits();
   if (idx >= num)
@@ -175,7 +193,8 @@
   return cu_sp;
 }
 
-void SymbolFile::SetCompileUnitAtIndex(uint32_t idx, const CompUnitSP &cu_sp) {
+void SymbolFileCommon::SetCompileUnitAtIndex(uint32_t idx,
+ const CompUnitSP &cu_sp) {
   std::lock_guard guard(GetModuleMutex());
   const size_t num_compile_units = GetNumCompileUnits();
   assert(idx < num_compile_units);
@@ -190,31 +209,29 @@
   (*m_compile_units)[idx] = cu_sp;
 }
 
-Symtab *SymbolFile::GetSymtab() {
-  std::lock_guard guard(GetModuleMutex());
-  if (m_symtab)
-return m_symtab;
-
-  // Fetch the symtab from the main object file.
-  m_symtab = GetMainObjectFile()->GetSymtab();
-
-  // Then add our symbols to it.
-  if (m_symtab)
-AddSymbols(*m_symtab);
-
-  return m_symtab;
+llvm::Expected
+SymbolFileCommon::GetTypeSystemForLanguage(lldb::LanguageType language) {
+  auto type_system_or_err =
+  m_objfile_sp->GetModule()->GetTypeSystemForLanguage(language);
+  if (type_system_or_err) {
+type_system_or_err->SetSymbolFile(this);
+  }
+  return type_system_or_err;
 }
 
-void SymbolFile::SectionFileAddressesChanged() {
-  ObjectFile *module_objfile = GetMainObjectFile();
-  ObjectFile *symfile_objfile = GetObjectFile();
-  if (symfile_objfile != modul

[Lldb-commits] [PATCH] D123982: [trace][intel pt] Support events

[Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG059f39d2f445: [trace][intel pt] Support events (authored by 
Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123982

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Target/TraceCursor.cpp
  lldb/source/Target/TraceInstructionDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpInfo.py
  lldb/test/API/commands/trace/TestTraceEvents.py
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -40,12 +40,17 @@
 
   Memory usage:
 Raw trace size: 4 KiB
-Total approximate memory usage (excluding raw trace): 0.27 KiB
-Average memory usage per instruction (excluding raw trace): 13.00 bytes
+Total approximate memory usage (excluding raw trace): 1.27 KiB
+Average memory usage per instruction (excluding raw trace): 61.76 bytes
 
   Timing:
 Decoding instructions: ''', '''s
 
+  Events:
+Number of instructions with events: 1
+Number of individual events: 1
+  paused: 1
+
   Errors:
 Number of TSC decoding errors: 0'''])
 
Index: lldb/test/API/commands/trace/TestTraceEvents.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceEvents.py
@@ -0,0 +1,82 @@
+import lldb
+from intelpt_testcase import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceEvents(TraceIntelPTTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@testSBAPIAndCommands
+def testPauseEvents(self):
+  '''
+Everytime the target stops running on the CPU, a 'disabled' event will
+be emitted, which is represented by the TraceCursor API as a 'paused'
+event.
+  '''
+  self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace-multi-file", "a.out"))
+  self.expect("b 12")
+  self.expect("r")
+  self.traceStartThread()
+  self.expect("n")
+  self.expect("n")
+  self.expect("si")
+  self.expect("si")
+  self.expect("si")
+  # We ensure that the paused events are printed correctly forward
+  self.expect("thread trace dump instructions -e -f",
+patterns=[f'''thread #1: tid = .*
+  a.out`main \+ 23 at main.cpp:12
+0: {ADDRESS_REGEX}movl .*
+  \[paused\]
+1: {ADDRESS_REGEX}addl .*
+2: {ADDRESS_REGEX}movl .*
+  \[paused\]
+  a.out`main \+ 34 \[inlined\] inline_function\(\) at main.cpp:4
+3: {ADDRESS_REGEX}movl .*
+  a.out`main \+ 41 \[inlined\] inline_function\(\) \+ 7 at main.cpp:5
+4: {ADDRESS_REGEX}movl .*
+5: {ADDRESS_REGEX}addl .*
+6: {ADDRESS_REGEX}movl .*
+  a.out`main \+ 52 \[inlined\] inline_function\(\) \+ 18 at main.cpp:6
+7: {ADDRESS_REGEX}movl .*
+  a.out`main \+ 55 at main.cpp:14
+8: {ADDRESS_REGEX}movl .*
+9: {ADDRESS_REGEX}addl .*
+10: {ADDRESS_REGEX}movl .*
+  \[paused\]
+  a.out`main \+ 63 at main.cpp:16
+11: {ADDRESS_REGEX}callq  .* ; symbol stub for: foo\(\)
+  \[paused\]
+  a.out`symbol stub for: foo\(\)
+12: {ADDRESS_REGEX}jmpq'''])
+
+  # We ensure that the paused events are printed correctly backward
+  self.expect("thread trace dump instructions -e --id 12",
+patterns=[f'''thread #1: tid = .*
+  a.out`symbol stub for: foo\(\)
+12: {ADDRESS_REGEX}jmpq .*
+  \[paused\]
+  a.out`main \+ 63 at main.cpp:16
+11: {ADDRESS_REGEX}callq  .* ; symbol stub for: foo\(\)
+  \[paused\]
+  a.out`main \+ 60 at main.cpp:14
+10: {ADDRESS_REGEX}movl .*
+9: {ADDRESS_REGEX}addl .*
+8: {ADDRESS_REGEX}movl .*
+  a.out`main \+ 52 \[inlined\] inline_function\(\) \+ 18 at main.cpp:6
+7: {ADDRESS_REGEX}movl .*
+  a.out`main \+ 49 \[inlined\] inline_function\(\) \+ 15 at main.cpp:5
+6: {ADDRESS_REGEX}movl .*
+5: {ADDRESS_REGEX}addl .*
+4: {ADDRESS_REGEX}movl .*
+  a.out`main \+ 34 \[inlined\] inline_function\(\) at main.cpp:4
+3: {ADDRESS_REGEX}movl .*

[Lldb-commits] [lldb] 059f39d - [trace][intel pt] Support events

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-04-25T19:01:23-07:00
New Revision: 059f39d2f44503862cb9c752c28a3a77275b0e51

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

LOG: [trace][intel pt] Support events

A trace might contain events traced during the target's execution. For
example, a thread might be paused for some period of time due to context
switches or breakpoints, which actually force a context switch. Not only
that, a trace might be paused because the CPU decides to trace only a
specific part of the target, like the address filtering provided by
intel pt, which will cause pause events. Besides this case, other kinds
of events might exist.

This patch adds the method `TraceCursor::GetEvents()`` that returns the
list of events that happened right before the instruction being pointed
at by the cursor. Some refactors were done to make this change simpler.

Besides this new API, the instruction dumper now supports the -e flag
which shows pause events, like in the following example, where pauses
happened due to breakpoints.

```
thread #1: tid = 2717361
  a.out`main + 20 at main.cpp:27:20
0: 0x004023d9leaq   -0x1200(%rbp), %rax
  [paused]
1: 0x004023e0movq   %rax, %rdi
  [paused]
2: 0x004023e3callq  0x403a62  ; 
std::vector >::vector at stl_vector.h:391:7
  a.out`std::vector >::vector() at stl_vector.h:391:7
3: 0x00403a62pushq  %rbp
4: 0x00403a63movq   %rsp, %rbp
```

The `dump info` command has also been updated and now it shows the
number of instructions that have associated events.

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

Added: 
lldb/test/API/commands/trace/TestTraceEvents.py

Modified: 
lldb/include/lldb/Target/TraceCursor.h
lldb/include/lldb/Target/TraceInstructionDumper.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/Options.td
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Target/TraceCursor.cpp
lldb/source/Target/TraceInstructionDumper.cpp
lldb/test/API/commands/trace/TestTraceDumpInfo.py
lldb/test/API/commands/trace/TestTraceLoad.py

Removed: 




diff  --git a/lldb/include/lldb/Target/TraceCursor.h 
b/lldb/include/lldb/Target/TraceCursor.h
index 710bb963a4aa9..606f6886964a7 100644
--- a/lldb/include/lldb/Target/TraceCursor.h
+++ b/lldb/include/lldb/Target/TraceCursor.h
@@ -234,9 +234,17 @@ class TraceCursor {
   /// \param[in] counter_type
   ///The counter type.
   /// \return
-  /// The value of the counter or \b llvm::None if not available.
+  ///The value of the counter or \b llvm::None if not available.
   virtual llvm::Optional GetCounter(lldb::TraceCounter counter_type) 
= 0;
 
+  /// Get a bitmask with a list of events that happened chronologically right
+  /// before the current instruction or error, but after the previous
+  /// instruction.
+  ///
+  /// \return
+  ///   The bitmask of events.
+  virtual lldb::TraceEvents GetEvents() = 0;
+
   /// \return
   /// The \a lldb::TraceInstructionControlFlowType categories the
   /// instruction the cursor is pointing at falls into. If the cursor 
points
@@ -254,6 +262,29 @@ class TraceCursor {
   bool m_forwards = false;
 };
 
+namespace trace_event_utils {
+/// Convert an individual event to a display string.
+///
+/// \param[in] event
+/// An individual event.
+///
+/// \return
+/// A display string for that event, or nullptr if wrong data is passed
+/// in.
+const char *EventToDisplayString(lldb::TraceEvents event);
+
+/// Invoke the given callback for each individual event of the given events
+/// bitmask.
+///
+/// \param[in] events
+/// A list of events to inspect.
+///
+/// \param[in] callback
+/// The callback to invoke for each event.
+void ForEachEvent(lldb::TraceEvents events,
+  std::function callback);
+} // namespace trace_event_utils
+
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_TRACE_CURSOR_H

diff  --git a/lldb/include/lldb/Target/TraceInstructionDumper.h 
b/lldb/include/lldb/Target/TraceInstructionDumper.h
index f66509e462dd4..9dc8e4c50f428 100644
--- a/lldb/include/lldb/Target/TraceInstructionDumper.h
+++ b/lldb/include/lldb/Target/TraceInstructionDumper.h
@@ -26,6 +26,8 @@ struct TraceInstructionDumperOptions {
   /// For each instruction, print the corresponding time

[Lldb-commits] [PATCH] D124064: [NFC][trace] simplify the instruction dumper

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:26
+  lldb::InstructionSP instruction;
+  lldb_private::ExecutionContext exe_ctx;
+};

jj10306 wrote:
> It shouldn't be an issue now because this struct is never stored any where, 
> but in the future we should be aware that if this struct is ever stored "long 
> term" (ie in another class), we should instead use `ExececutionContextRef`
> From the documentation in `ExecutionContext.h`
> ```
> /// exist during a function that requires the objects. ExecutionContext
> /// objects should NOT be used for long term storage since they will keep
> /// objects alive with extra shared pointer references to these  objects
> ```
Yes, you are right :) 



Comment at: lldb/source/Target/TraceInstructionDumper.cpp:199
+/// instruction's disassembler when possible.
+std::tuple
+CalculateDisass(const InstructionSymbolInfo &insn_info,

jj10306 wrote:
> should this be a static function?
goot catch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124064

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

[Lldb-commits] [PATCH] D124064: [NFC][trace] simplify the instruction dumper

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 425109.
wallace added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124064

Files:
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/source/Target/TraceInstructionDumper.cpp

Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -47,8 +47,6 @@
   }
 }
 
-/// \return
-/// Return \b true if the cursor could move one step.
 bool TraceInstructionDumper::TryMoveOneStep() {
   if (!m_cursor_up->Next()) {
 SetNoMoreData();
@@ -57,17 +55,6 @@
   return true;
 }
 
-/// Helper struct that holds symbol, disassembly and address information of an
-/// instruction.
-struct InstructionSymbolInfo {
-  SymbolContext sc;
-  Address address;
-  lldb::addr_t load_address;
-  lldb::DisassemblerSP disassembler;
-  lldb::InstructionSP instruction;
-  lldb_private::ExecutionContext exe_ctx;
-};
-
 // This custom LineEntry validator is neded because some line_entries have
 // 0 as line, which is meaningless. Notice that LineEntry::IsValid only
 // checks that line is not LLDB_INVALID_LINE_NUMBER, i.e. UINT32_MAX.
@@ -122,46 +109,35 @@
   return curr_line_valid == prev_line_valid;
 }
 
-/// Dump the symbol context of the given instruction address if it's different
-/// from the symbol context of the previous instruction in the trace.
-///
-/// \param[in] prev_sc
-/// The symbol context of the previous instruction in the trace.
-///
-/// \param[in] address
-/// The address whose symbol information will be dumped.
-///
-/// \return
-/// The symbol context of the current address, which might differ from the
-/// previous one.
-static void
-DumpInstructionSymbolContext(Stream &s,
- Optional prev_insn,
- InstructionSymbolInfo &insn) {
+void TraceInstructionDumper::DumpInstructionSymbolContext(
+const Optional &prev_insn,
+const InstructionSymbolInfo &insn) {
   if (prev_insn && IsSameInstructionSymbolContext(*prev_insn, insn))
 return;
 
-  s.Printf("  ");
+  m_s << "  ";
 
   if (!insn.sc.module_sp)
-s.Printf("(none)");
+m_s << "(none)";
   else if (!insn.sc.function && !insn.sc.symbol)
-s.Printf("%s`(none)",
- insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
+m_s.Format("{0}`(none)",
+   insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
   else
-insn.sc.DumpStopContext(&s, insn.exe_ctx.GetTargetPtr(), insn.address,
+insn.sc.DumpStopContext(&m_s, insn.exe_ctx.GetTargetPtr(), insn.address,
 /*show_fullpaths=*/false,
 /*show_module=*/true, /*show_inlined_frames=*/false,
 /*show_function_arguments=*/true,
 /*show_function_name=*/true);
-  s.Printf("\n");
+  m_s << "\n";
 }
 
-static void DumpInstructionDisassembly(Stream &s, InstructionSymbolInfo &insn) {
+void TraceInstructionDumper::DumpInstructionDisassembly(
+const InstructionSymbolInfo &insn) {
   if (!insn.instruction)
 return;
-  s.Printf("");
-  insn.instruction->Dump(&s, /*max_opcode_byte_size=*/0, /*show_address=*/false,
+  m_s << "";
+  insn.instruction->Dump(&m_s, /*max_opcode_byte_size=*/0,
+ /*show_address=*/false,
  /*show_bytes=*/false, &insn.exe_ctx, &insn.sc,
  /*prev_sym_ctx=*/nullptr,
  /*disassembly_addr_format=*/nullptr,
@@ -172,6 +148,26 @@
 
 bool TraceInstructionDumper::HasMoreData() { return !m_no_more_data; }
 
+void TraceInstructionDumper::PrintMissingInstructionsMessage() {
+  m_s << "...missing instructions\n";
+}
+
+void TraceInstructionDumper::PrintInstructionHeader() {
+  m_s.Format("{0}: ", m_cursor_up->GetId());
+
+  if (m_options.show_tsc) {
+m_s << "[tsc=";
+
+if (Optional timestamp =
+m_cursor_up->GetCounter(lldb::eTraceCounterTSC))
+  m_s.Format("{0:x+16}", *timestamp);
+else
+  m_s << "unavailable";
+
+m_s << "] ";
+  }
+}
+
 void TraceInstructionDumper::PrintEvents() {
   if (!m_options.show_events)
 return;
@@ -182,90 +178,76 @@
   });
 }
 
-Optional TraceInstructionDumper::DumpInstructions(size_t count) {
+/// Find the symbol context for the given address reusing the previous
+/// instruction's symbol context when possible.
+static SymbolContext
+CalculateSymbolContext(const Address &address,
+   const InstructionSymbolInfo &prev_insn_info) {
+  AddressRange range;
+  if (prev_insn_info.sc.GetAddressRange(eSymbolContextEverything, 0,
+/*inline_block_range*/ false, range) &&
+  range.Contains(address))
+return prev_insn_in

[Lldb-commits] [lldb] 35e60f5 - [NFC][trace] simplify the instruction dumper

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-04-25T20:02:48-07:00
New Revision: 35e60f5de180aea55ed478298f4b40f04dcc57d1

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

LOG: [NFC][trace] simplify the instruction dumper

TraceInstructionDumper::DumpInstructions was becoming too big, so I'm
refactoring it into smaller functions. I also made some static methods proper
instance methods to simplify calls. Other minor improvements are also done.

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

Added: 


Modified: 
lldb/include/lldb/Target/TraceInstructionDumper.h
lldb/source/Target/TraceInstructionDumper.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/TraceInstructionDumper.h 
b/lldb/include/lldb/Target/TraceInstructionDumper.h
index 9dc8e4c50f428..53a1a03ee1ce2 100644
--- a/lldb/include/lldb/Target/TraceInstructionDumper.h
+++ b/lldb/include/lldb/Target/TraceInstructionDumper.h
@@ -8,11 +8,24 @@
 
 #include "lldb/Target/TraceCursor.h"
 
+#include "lldb/Symbol/SymbolContext.h"
+
 #ifndef LLDB_TARGET_TRACE_INSTRUCTION_DUMPER_H
 #define LLDB_TARGET_TRACE_INSTRUCTION_DUMPER_H
 
 namespace lldb_private {
 
+/// Helper struct that holds symbol, disassembly and address information of an
+/// instruction.
+struct InstructionSymbolInfo {
+  SymbolContext sc;
+  Address address;
+  lldb::addr_t load_address;
+  lldb::DisassemblerSP disassembler;
+  lldb::InstructionSP instruction;
+  lldb_private::ExecutionContext exe_ctx;
+};
+
 /// Class that holds the configuration used by \a TraceInstructionDumper for
 /// traversing and dumping instructions.
 struct TraceInstructionDumperOptions {
@@ -83,6 +96,28 @@ class TraceInstructionDumper {
 
   void PrintEvents();
 
+  void PrintMissingInstructionsMessage();
+
+  void PrintInstructionHeader();
+
+  void DumpInstructionDisassembly(const InstructionSymbolInfo &insn);
+
+  /// Dump the symbol context of the given instruction address if it's 
diff erent
+  /// from the symbol context of the previous instruction in the trace.
+  ///
+  /// \param[in] prev_sc
+  /// The symbol context of the previous instruction in the trace.
+  ///
+  /// \param[in] address
+  /// The address whose symbol information will be dumped.
+  ///
+  /// \return
+  /// The symbol context of the current address, which might 
diff er from the
+  /// previous one.
+  void DumpInstructionSymbolContext(
+  const llvm::Optional &prev_insn,
+  const InstructionSymbolInfo &insn);
+
   lldb::TraceCursorUP m_cursor_up;
   TraceInstructionDumperOptions m_options;
   Stream &m_s;

diff  --git a/lldb/source/Target/TraceInstructionDumper.cpp 
b/lldb/source/Target/TraceInstructionDumper.cpp
index 7473e84e356f1..fd809f263ea25 100644
--- a/lldb/source/Target/TraceInstructionDumper.cpp
+++ b/lldb/source/Target/TraceInstructionDumper.cpp
@@ -47,8 +47,6 @@ TraceInstructionDumper::TraceInstructionDumper(
   }
 }
 
-/// \return
-/// Return \b true if the cursor could move one step.
 bool TraceInstructionDumper::TryMoveOneStep() {
   if (!m_cursor_up->Next()) {
 SetNoMoreData();
@@ -57,17 +55,6 @@ bool TraceInstructionDumper::TryMoveOneStep() {
   return true;
 }
 
-/// Helper struct that holds symbol, disassembly and address information of an
-/// instruction.
-struct InstructionSymbolInfo {
-  SymbolContext sc;
-  Address address;
-  lldb::addr_t load_address;
-  lldb::DisassemblerSP disassembler;
-  lldb::InstructionSP instruction;
-  lldb_private::ExecutionContext exe_ctx;
-};
-
 // This custom LineEntry validator is neded because some line_entries have
 // 0 as line, which is meaningless. Notice that LineEntry::IsValid only
 // checks that line is not LLDB_INVALID_LINE_NUMBER, i.e. UINT32_MAX.
@@ -122,46 +109,35 @@ IsSameInstructionSymbolContext(const 
InstructionSymbolInfo &prev_insn,
   return curr_line_valid == prev_line_valid;
 }
 
-/// Dump the symbol context of the given instruction address if it's 
diff erent
-/// from the symbol context of the previous instruction in the trace.
-///
-/// \param[in] prev_sc
-/// The symbol context of the previous instruction in the trace.
-///
-/// \param[in] address
-/// The address whose symbol information will be dumped.
-///
-/// \return
-/// The symbol context of the current address, which might 
diff er from the
-/// previous one.
-static void
-DumpInstructionSymbolContext(Stream &s,
- Optional prev_insn,
- InstructionSymbolInfo &insn) {
+void TraceInstructionDumper::DumpInstructionSymbolContext(
+const Optional &prev_insn,
+const InstructionSymbolInfo &insn) {
   if (prev_insn && IsSameInstructionSymbolContext(*prev_insn, insn))
 return;
 
-  s.Printf("  ");
+  m_s << "  ";
 
   if (!insn.sc.module_sp)
-s.Printf("

[Lldb-commits] [PATCH] D124064: [NFC][trace] simplify the instruction dumper

[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 rG35e60f5de180: [NFC][trace] simplify the instruction dumper 
(authored by Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124064

Files:
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/source/Target/TraceInstructionDumper.cpp

Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -47,8 +47,6 @@
   }
 }
 
-/// \return
-/// Return \b true if the cursor could move one step.
 bool TraceInstructionDumper::TryMoveOneStep() {
   if (!m_cursor_up->Next()) {
 SetNoMoreData();
@@ -57,17 +55,6 @@
   return true;
 }
 
-/// Helper struct that holds symbol, disassembly and address information of an
-/// instruction.
-struct InstructionSymbolInfo {
-  SymbolContext sc;
-  Address address;
-  lldb::addr_t load_address;
-  lldb::DisassemblerSP disassembler;
-  lldb::InstructionSP instruction;
-  lldb_private::ExecutionContext exe_ctx;
-};
-
 // This custom LineEntry validator is neded because some line_entries have
 // 0 as line, which is meaningless. Notice that LineEntry::IsValid only
 // checks that line is not LLDB_INVALID_LINE_NUMBER, i.e. UINT32_MAX.
@@ -122,46 +109,35 @@
   return curr_line_valid == prev_line_valid;
 }
 
-/// Dump the symbol context of the given instruction address if it's different
-/// from the symbol context of the previous instruction in the trace.
-///
-/// \param[in] prev_sc
-/// The symbol context of the previous instruction in the trace.
-///
-/// \param[in] address
-/// The address whose symbol information will be dumped.
-///
-/// \return
-/// The symbol context of the current address, which might differ from the
-/// previous one.
-static void
-DumpInstructionSymbolContext(Stream &s,
- Optional prev_insn,
- InstructionSymbolInfo &insn) {
+void TraceInstructionDumper::DumpInstructionSymbolContext(
+const Optional &prev_insn,
+const InstructionSymbolInfo &insn) {
   if (prev_insn && IsSameInstructionSymbolContext(*prev_insn, insn))
 return;
 
-  s.Printf("  ");
+  m_s << "  ";
 
   if (!insn.sc.module_sp)
-s.Printf("(none)");
+m_s << "(none)";
   else if (!insn.sc.function && !insn.sc.symbol)
-s.Printf("%s`(none)",
- insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
+m_s.Format("{0}`(none)",
+   insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
   else
-insn.sc.DumpStopContext(&s, insn.exe_ctx.GetTargetPtr(), insn.address,
+insn.sc.DumpStopContext(&m_s, insn.exe_ctx.GetTargetPtr(), insn.address,
 /*show_fullpaths=*/false,
 /*show_module=*/true, /*show_inlined_frames=*/false,
 /*show_function_arguments=*/true,
 /*show_function_name=*/true);
-  s.Printf("\n");
+  m_s << "\n";
 }
 
-static void DumpInstructionDisassembly(Stream &s, InstructionSymbolInfo &insn) {
+void TraceInstructionDumper::DumpInstructionDisassembly(
+const InstructionSymbolInfo &insn) {
   if (!insn.instruction)
 return;
-  s.Printf("");
-  insn.instruction->Dump(&s, /*max_opcode_byte_size=*/0, /*show_address=*/false,
+  m_s << "";
+  insn.instruction->Dump(&m_s, /*max_opcode_byte_size=*/0,
+ /*show_address=*/false,
  /*show_bytes=*/false, &insn.exe_ctx, &insn.sc,
  /*prev_sym_ctx=*/nullptr,
  /*disassembly_addr_format=*/nullptr,
@@ -172,6 +148,26 @@
 
 bool TraceInstructionDumper::HasMoreData() { return !m_no_more_data; }
 
+void TraceInstructionDumper::PrintMissingInstructionsMessage() {
+  m_s << "...missing instructions\n";
+}
+
+void TraceInstructionDumper::PrintInstructionHeader() {
+  m_s.Format("{0}: ", m_cursor_up->GetId());
+
+  if (m_options.show_tsc) {
+m_s << "[tsc=";
+
+if (Optional timestamp =
+m_cursor_up->GetCounter(lldb::eTraceCounterTSC))
+  m_s.Format("{0:x+16}", *timestamp);
+else
+  m_s << "unavailable";
+
+m_s << "] ";
+  }
+}
+
 void TraceInstructionDumper::PrintEvents() {
   if (!m_options.show_events)
 return;
@@ -182,90 +178,76 @@
   });
 }
 
-Optional TraceInstructionDumper::DumpInstructions(size_t count) {
+/// Find the symbol context for the given address reusing the previous
+/// instruction's symbol context when possible.
+static SymbolContext
+CalculateSymbolContext(const Address &address,
+   const InstructionSymbolInfo &prev_insn_info) {
+  AddressRange range;
+  if (prev_insn_info.sc.GetAddressRang

[Lldb-commits] [PATCH] D124429: [lldb] Remove Python 2 support from the ScriptInterpreter plugin

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, clayborg, lanza.
Herald added a project: All.
JDevlieghere requested review of this revision.

We dropped downstream support for Python 2 in the previous release. Now that we 
have branched for the next release the window where this kind of change could 
introduce conflicts is closing too. Start by getting rid of Python 2 support in 
the Script Interpreter plugin.


https://reviews.llvm.org/D124429

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -51,11 +51,7 @@
 // callbacks. Because they're defined in libLLDB which we cannot link for the
 // unit test, we have a 'default' implementation here.
 
-#if PY_MAJOR_VERSION >= 3
 extern "C" PyObject *PyInit__lldb(void) { return nullptr; }
-#else
-extern "C" void init_lldb(void) {}
-#endif
 
 llvm::Expected lldb_private::LLDBSwigPythonBreakpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -164,18 +164,6 @@
 TEST_F(PythonDataObjectsTest, TestPythonInteger) {
   // Test that integers behave correctly when wrapped by a PythonInteger.
 
-#if PY_MAJOR_VERSION < 3
-  // Verify that `PythonInt` works correctly when given a PyInt object.
-  // Note that PyInt doesn't exist in Python 3.x, so this is only for 2.x
-  PyObject *py_int = PyInt_FromLong(12);
-  EXPECT_TRUE(PythonInteger::Check(py_int));
-  PythonInteger python_int(PyRefType::Owned, py_int);
-
-  EXPECT_EQ(PyObjectType::Integer, python_int.GetObjectType());
-  auto python_int_value = As(python_int);
-  EXPECT_THAT_EXPECTED(python_int_value, llvm::HasValue(12));
-#endif
-
   // Verify that `PythonInteger` works correctly when given a PyLong object.
   PyObject *py_long = PyLong_FromLong(12);
   EXPECT_TRUE(PythonInteger::Check(py_long));
@@ -225,13 +213,8 @@
   EXPECT_TRUE(PythonBytes::Check(py_bytes));
   PythonBytes python_bytes(PyRefType::Owned, py_bytes);
 
-#if PY_MAJOR_VERSION < 3
-  EXPECT_TRUE(PythonString::Check(py_bytes));
-  EXPECT_EQ(PyObjectType::String, python_bytes.GetObjectType());
-#else
   EXPECT_FALSE(PythonString::Check(py_bytes));
   EXPECT_EQ(PyObjectType::Bytes, python_bytes.GetObjectType());
-#endif
 
   llvm::ArrayRef bytes = python_bytes.GetBytes();
   EXPECT_EQ(bytes.size(), strlen(test_bytes));
@@ -258,23 +241,12 @@
   static const char *test_string = "PythonDataObjectsTest::TestPythonString1";
   static const char *test_string2 = "PythonDataObjectsTest::TestPythonString2";
 
-#if PY_MAJOR_VERSION < 3
-  // Verify that `PythonString` works correctly when given a PyString object.
-  // Note that PyString doesn't exist in Python 3.x, so this is only for 2.x
-  PyObject *py_string = PyString_FromString(test_string);
-  EXPECT_TRUE(PythonString::Check(py_string));
-  PythonString python_string(PyRefType::Owned, py_string);
-
-  EXPECT_EQ(PyObjectType::String, python_string.GetObjectType());
-  EXPECT_STREQ(test_string, python_string.GetString().data());
-#else
   // Verify that `PythonString` works correctly when given a PyUnicode object.
   PyObject *py_unicode = PyUnicode_FromString(test_string);
   EXPECT_TRUE(PythonString::Check(py_unicode));
   PythonString python_unicode(PyRefType::Owned, py_unicode);
   EXPECT_EQ(PyObjectType::String, python_unicode.GetObjectType());
   EXPECT_STREQ(test_string, python_unicode.GetString().data());
-#endif
 
   // Test that creating a `PythonString` object works correctly with the
   // string constructor
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -60,17 +60,10 @@
 LLDB_PLUGIN_DEFINE(ScriptInterpreterPython)
 
 // Defined in the SWIG source file
-#if PY_MAJOR_VERSION >= 3
 extern "C" PyObject *PyInit__lldb(void);
 
 #define LLDBSwigPyInit PyInit__lldb
 
-#else
-extern "C" void init_lldb(void);
-
-#define LLDBSwigPyInit init_lldb
-#endif
-
 #if defined(_WIN32)
 // Don't

[Lldb-commits] [PATCH] D124430: [lldb] Remove Python 2 checks from the test suite

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, clayborg, lanza.
Herald added a project: All.
JDevlieghere requested review of this revision.
Herald added a project: LLDB.

Remove Python 2 checks from the test suite


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124430

Files:
  lldb/test/API/functionalities/step_scripted/TestStepScripted.py
  lldb/test/API/lldbtest.py
  lldb/test/API/macosx/nslog/TestDarwinNSLogOutput.py
  lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
  lldb/test/API/python_api/file_handle/TestFileHandle.py
  lldb/test/API/terminal/TestSTTYBeforeAndAfter.py

Index: lldb/test/API/terminal/TestSTTYBeforeAndAfter.py
===
--- lldb/test/API/terminal/TestSTTYBeforeAndAfter.py
+++ lldb/test/API/terminal/TestSTTYBeforeAndAfter.py
@@ -7,6 +7,7 @@
 
 import lldb
 import six
+import sys
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -41,11 +42,7 @@
 lldb_prompt = "(lldb) "
 
 # So that the child gets torn down after the test.
-import sys
-if sys.version_info.major == 3:
-  self.child = pexpect.spawnu('expect')
-else:
-  self.child = pexpect.spawn('expect')
+self.child = pexpect.spawnu('expect')
 child = self.child
 
 child.expect(expect_prompt)
Index: lldb/test/API/python_api/file_handle/TestFileHandle.py
===
--- lldb/test/API/python_api/file_handle/TestFileHandle.py
+++ lldb/test/API/python_api/file_handle/TestFileHandle.py
@@ -605,18 +605,15 @@
 def test_exceptions(self):
 self.assertRaises(Exception, lldb.SBFile, None)
 self.assertRaises(Exception, lldb.SBFile, "ham sandwich")
-if sys.version_info[0] < 3:
-self.assertRaises(Exception, lldb.SBFile, ReallyBadIO())
-else:
-self.assertRaises(OhNoe, lldb.SBFile, ReallyBadIO())
-error, n = lldb.SBFile(BadIO()).Write(b"FOO")
-self.assertEqual(n, 0)
-self.assertTrue(error.Fail())
-self.assertIn('OH NOE', error.GetCString())
-error, n = lldb.SBFile(BadIO()).Read(bytearray(100))
-self.assertEqual(n, 0)
-self.assertTrue(error.Fail())
-self.assertIn('OH NOE', error.GetCString())
+self.assertRaises(OhNoe, lldb.SBFile, ReallyBadIO())
+error, n = lldb.SBFile(BadIO()).Write(b"FOO")
+self.assertEqual(n, 0)
+self.assertTrue(error.Fail())
+self.assertIn('OH NOE', error.GetCString())
+error, n = lldb.SBFile(BadIO()).Read(bytearray(100))
+self.assertEqual(n, 0)
+self.assertTrue(error.Fail())
+self.assertIn('OH NOE', error.GetCString())
 
 
 @skipIf(py_version=['<', (3,)])
Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
@@ -49,19 +49,17 @@
 for i in insts:
 print("Disassembled %s" % str(i))
 
-if sys.version_info.major >= 3:
-sio = StringIO()
-insts.Print(sio)
-self.assertEqual(split(assembly), split(sio.getvalue()))
+sio = StringIO()
+insts.Print(sio)
+self.assertEqual(split(assembly), split(sio.getvalue()))
 
 self.assertEqual(insts.GetSize(), len(split(assembly)))
 
-if sys.version_info.major >= 3:
-for i,asm in enumerate(split(assembly)):
-inst = insts.GetInstructionAtIndex(i)
-sio = StringIO()
-inst.Print(sio)
-self.assertEqual(asm, sio.getvalue().strip())
+for i,asm in enumerate(split(assembly)):
+inst = insts.GetInstructionAtIndex(i)
+sio = StringIO()
+inst.Print(sio)
+self.assertEqual(asm, sio.getvalue().strip())
 
 raw_bytes = bytearray([0x04, 0xf9, 0xed, 0x82])
 
Index: lldb/test/API/macosx/nslog/TestDarwinNSLogOutput.py
===
--- lldb/test/API/macosx/nslog/TestDarwinNSLogOutput.py
+++ lldb/test/API/macosx/nslog/TestDarwinNSLogOutput.py
@@ -10,6 +10,7 @@
 import lldb
 import platform
 import re
+import sys
 
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -57,13 +58,8 @@
 
 # So that the child gets torn down after the test.
 import pexpect
-import sys
-if sys.version_info.major == 3:
-  self.child = pexpect.spawnu('%s %s %s' % (lldbtest_config.lldbExec,
-self.lldbOption, exe))
-else:
-  

[Lldb-commits] [PATCH] D124430: [lldb] Remove Python 2 checks from the test suite

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 425113.

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

https://reviews.llvm.org/D124430

Files:
  lldb/test/API/functionalities/step_scripted/TestStepScripted.py
  lldb/test/API/lldbtest.py
  lldb/test/API/macosx/nslog/TestDarwinNSLogOutput.py
  lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
  lldb/test/API/python_api/file_handle/TestFileHandle.py
  lldb/test/API/terminal/TestSTTYBeforeAndAfter.py

Index: lldb/test/API/terminal/TestSTTYBeforeAndAfter.py
===
--- lldb/test/API/terminal/TestSTTYBeforeAndAfter.py
+++ lldb/test/API/terminal/TestSTTYBeforeAndAfter.py
@@ -7,6 +7,7 @@
 
 import lldb
 import six
+import sys
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -41,11 +42,7 @@
 lldb_prompt = "(lldb) "
 
 # So that the child gets torn down after the test.
-import sys
-if sys.version_info.major == 3:
-  self.child = pexpect.spawnu('expect')
-else:
-  self.child = pexpect.spawn('expect')
+self.child = pexpect.spawnu('expect')
 child = self.child
 
 child.expect(expect_prompt)
Index: lldb/test/API/python_api/file_handle/TestFileHandle.py
===
--- lldb/test/API/python_api/file_handle/TestFileHandle.py
+++ lldb/test/API/python_api/file_handle/TestFileHandle.py
@@ -276,7 +276,6 @@
 self.assertTrue(re.search(r'QUUX', output))
 
 
-@skipIf(py_version=['<', (3,)])
 def test_immediate_string(self):
 f = io.StringIO()
 ret = lldb.SBCommandReturnObject()
@@ -291,7 +290,6 @@
 self.assertTrue(re.search(r'QUUX', output))
 
 
-@skipIf(py_version=['<', (3,)])
 def test_immediate_sbfile_string(self):
 f = io.StringIO()
 ret = lldb.SBCommandReturnObject()
@@ -361,7 +359,6 @@
 self.assertIn('Show a list of all debugger commands', output)
 
 
-@skipIf(py_version=['<', (3,)])
 def test_string_inout(self):
 inf = io.StringIO("help help\np/x ~0\n")
 outf = io.StringIO()
@@ -377,7 +374,6 @@
 self.assertIn('0xfff', output)
 
 
-@skipIf(py_version=['<', (3,)])
 def test_bytes_inout(self):
 inf = io.BytesIO(b"help help\nhelp b\n")
 outf = io.BytesIO()
@@ -447,7 +443,6 @@
 
 
 
-@skipIf(py_version=['<', (3,)])
 def test_sbfile_write_forced(self):
 with open(self.out_filename, 'w') as f:
 written = MutableBool(False)
@@ -467,7 +462,6 @@
 self.assertEqual(f.read().strip(), 'FOO')
 
 
-@skipIf(py_version=['<', (3,)])
 def test_sbfile_write_forced_borrowed(self):
 with open(self.out_filename, 'w') as f:
 written = MutableBool(False)
@@ -487,7 +481,6 @@
 self.assertEqual(f.read().strip(), 'FOO')
 
 
-@skipIf(py_version=['<', (3,)])
 def test_sbfile_write_string(self):
 f = io.StringIO()
 sbf = lldb.SBFile(f)
@@ -499,7 +492,6 @@
 self.assertTrue(f.closed)
 
 
-@skipIf(py_version=['<', (3,)])
 def test_string_out(self):
 f = io.StringIO()
 status = self.dbg.SetOutputFile(f)
@@ -508,7 +500,6 @@
 self.assertEqual(f.getvalue().strip(), "'foobar'")
 
 
-@skipIf(py_version=['<', (3,)])
 def test_string_error(self):
 f = io.StringIO()
 status = self.dbg.SetErrorFile(f)
@@ -518,7 +509,6 @@
 self.assertTrue(re.search(r'error:.*lolwut', errors))
 
 
-@skipIf(py_version=['<', (3,)])
 def test_sbfile_write_bytes(self):
 f = io.BytesIO()
 sbf = lldb.SBFile(f)
@@ -529,7 +519,6 @@
 sbf.Close()
 self.assertTrue(f.closed)
 
-@skipIf(py_version=['<', (3,)])
 def test_sbfile_read_string(self):
 f = io.StringIO('zork')
 sbf = lldb.SBFile(f)
@@ -539,7 +528,6 @@
 self.assertEqual(buf[:n], b'zork')
 
 
-@skipIf(py_version=['<', (3,)])
 def test_sbfile_read_string_one_byte(self):
 f = io.StringIO('z')
 sbf = lldb.SBFile(f)
@@ -550,7 +538,6 @@
 self.assertEqual(e.GetCString(), "can't read less than 6 bytes from a utf8 text stream")
 
 
-@skipIf(py_version=['<', (3,)])
 def test_sbfile_read_bytes(self):
 f = io.BytesIO(b'zork')
 sbf = lldb.SBFile(f)
@@ -560,7 +547,6 @@
 self.assertEqual(buf[:n], b'zork')
 
 
-@skipIf(py_version=['<', (3,)])
 def test_sbfile_out(self):
 with open(self.out_filename, 'w') as f:
 sbf = lldb.SBFile(f)
@@ -571,7 +557,6 @@
 self.assertEqual(f.read().strip(), '4')
 
 
-@skipIf(py_version=['<', (3,)])
 def test_file_out(self):
 with open(self.out_filename, 'w') as f:
 status = self.dbg.SetOutputFile(f)
@@ -605,21 +590,17 @@
 def test_exceptions(self):
 self.asse

[Lldb-commits] [PATCH] D124429: [lldb] Remove Python 2 support from the ScriptInterpreter plugin

[Pavel Labath via Phabricator via lldb-commits]

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

Woohoo


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

https://reviews.llvm.org/D124429

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