[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

2022-10-20 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1528
+DWARFASTParserClang::GetTemplateParametersString(const DWARFDIE &die) {
+  if (llvm::StringRef(die.GetName()).contains("<"))
+return std::string();

reverse the condition and use early exit?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1541
+  llvm::raw_string_ostream os(template_name);
+  arg.dump(os);
+  if (!template_name.empty()) {

`dump` is a "debugging aid". I guess you should call `print` and pass the 
appropriate `PrintingPolicy`. It might just be the default, though maybe we 
could also try to make an effort to match the output in the 
non-simplified-names case.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1551
+}
+if (!all_template_names.empty()) {
+  all_template_names.append(">");

When can this be empty? Should we still include the `<>` in those cases?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2497
+  // templates parameters, try again with the template parameters stripped 
since
+  // with -gsimple-template-names the DT_name may only contain the base name.
+  if (types.Empty()) {

AT_name ?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2498
+  // with -gsimple-template-names the DT_name may only contain the base name.
+  if (types.Empty()) {
+const llvm::StringRef nameRef = name.GetStringRef();

I don't think this can be keyed off of `Empty()` for two reasons:
- I believe the user can pass a non-empty type list into this function, 
expecting it to append to it
- just because you found one templated type above, it doesn't mean that there 
can't be another non-templated type somewhere (if only a part of the binary was 
built with simplified names).

I think we should just unconditionally (possibly guarded by a max_matches 
check) try both.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2500
+const llvm::StringRef nameRef = name.GetStringRef();
+auto it = nameRef.find('<');
+if (it != llvm::StringRef::npos) {

Michael137 wrote:
> Is there some other way to determine whether the debug-info was generated 
> from `-gsimple-template-names`? Then we wouldn't have to check the existence 
> of a `<` in the name in multiple places and wouldn't have to do this lookup 
> speculatively. With this change, if we fail to find a template type in the 
> index, we would do the lookup all over again, only to not find it again. 
> Could that get expensive? Just trying to figure out if we can avoid doing 
> this `GetTypes` call twice.
> 
> There have been [talks](https://github.com/llvm/llvm-project/issues/58362) 
> about doing a base-name search by default followed by fuzzy matching on 
> template parameters (though in the context of function names). The 
> `simple-template-names` came up as a good motivator/facilitator for doing so. 
> But for that idea to work here we'd have to be able to retrieve from the 
> index by basename, so perhaps out of the scope of what we're trying to do here
> 
> tagging people involved in that discussion: @dblaikie @aprantl @jingham 
> @labath
> 
> Other than this, generally LGTM
Negative matches should be fast, and since typically only one of the branches 
will produce any results, I think this should be fine. Filtering matches in the 
simplified case would be slower, since you'd have build the full name of all 
potential candidates (and that could be thousands of instantiations), but I 
don't know how slow. But that's the price we pay for better filtering (which we 
don't do right now, but we could start doing afterwards).



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2577
 
   ConstString name = pattern.back().name;
 

Do we need to do something similar in this FindTypes overload as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

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

[Lldb-commits] [PATCH] D136295: Fix exception description in lldb-vscode

2022-10-20 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

If you're explicitly checking for SIGABRT, then you might as well raise that 
signal directly (`raise(SIGABRT)`) instead of relying on the runtime to do it 
(in response to an unhandled exception). Or, if you want this to work on 
windows, then you'll need to adjust the expectation for the fact that this will 
be reported differently there (I don't know how, but it will definitely not be 
SIGABRT).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136295

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

[Lldb-commits] [PATCH] D136209: [LLDB][NativePDB] Fix parameter size for member functions LF_MFUNCTION

2022-10-20 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

Given what Reid said, would it be possible to make a test case that fails in an 
asserts build? Or was it maybe failing already there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136209

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

[Lldb-commits] [PATCH] D136306: [WIP][lldb][CPlusPlus] Add abi_tag support to the CPlusPlusNameParser

2022-10-20 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 planned changes to this revision.
Michael137 added a comment.

Wonder if we should skip the tags rather than consume them. Depends on what 
DW_AT_name says. Also I suspect there are cases where we would want to include 
tags and others where we dont

Will need to handle special characters in the tags too (currently it just 
handles simple identifiers)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136306

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

[Lldb-commits] [PATCH] D136207: [lldb] Fix breakpoint setting so it always works when there is a line entry in a compile unit's line table.

2022-10-20 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lldb/source/Symbol/CompileUnit.cpp:336
 } else {
   line_entry.range.GetBaseAddress().CalculateSymbolContext(&sc,
resolve_scope);

I think this would be more robustly written as something like
```
line_entry.range.GetBaseAddress().CalculateSymbolContext(&new_sc, 
resolve_scope);
if (new_sc.comp_unit == this)
  sc_list.Append(new_sc);
else
  sc_list.Append(sc);
```
-- basically using a new symbol context for the resolution, instead of 
twiddling the old one back and forth.



Comment at: 
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py:109
 
+@skipIf(oslist=["windows"])
+@no_debug_info_test

I would hope that the yaml-based test can run on windows as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136207

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

[Lldb-commits] [PATCH] D136006: [LLDB][NativePDB] Improve ParseDeclsForContext time.

2022-10-20 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Okay, sounds good then.




Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1413-1414
   if (context.isTranslationUnit()) {
-ParseAllNamespacesPlusChildrenOf(llvm::None);
+ParseAllTypes();
+ParseAllFunctionsAndNonLocalVars();
 return;

I have a feeling this is still doing more work than it would be necessary. I 
haven't checked, but I'd expect that here it should be sufficient to parse only 
the top level namespace names (not their contents), and create forward 
declarations for the classes in the global namespace. I suspect this is doing 
much more than that.
(Of course, if PDB makes it hard to parse just this information, then it might 
actually be better to parse everything -- I just don't know)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136006

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

[Lldb-commits] [PATCH] D135031: [lldb] [llgs] Move client-server communication into a separate thread (WIP)

2022-10-20 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

FWIW, I tried the to run the benchmark on an apple machine and got roughly the 
same results (although I never managed to get it to run to completion). I also 
don't know how much of a deal that is. I think someone from Apple ought to 
comment on that.

However, I have to say that this API feels very unergonomic. I still haven't 
managed to figure out how is it supposed to work, much less where the bug is. I 
think it's just operating at too low level. Instead of dealing with selects and 
callbacks, I'd say that the interface between the threads should be "this is 
the packet I want you to send, and here's is how I would like to get the reply".


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

https://reviews.llvm.org/D135031

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

[Lldb-commits] [PATCH] D136011: [lldb] Don't check environment default char signedness when creating clang type for "char"

2022-10-20 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

In D136011#3866854 , @aeubanks wrote:

> In D136011#3862150 , @labath wrote:
>
>> 
>
> Maybe looking for a "char" entry and setting `ast.getLangOpts().CharIsSigned` 
> from it would probably work in most cases, but not sure that it's worth the 
> effort. Rendering the wrong signedness for chars doesn't seem like the worst 
> thing, and this patch improves things (as the removed `expectedFailureAll` 
> show, plus this helps with some testing of lldb I've been doing). I can add a 
> TODO if you'd like.

It's not just a question of rendering. This can result in wrong/unexpected 
results of expressions as well. Something as simple as `p 
(int)unspecified_char_value` can return a different result depending on whether 
`unspecified_char_value` is considered signed or unsigned.
However, I am not convinced that we would be better off trying to detect this 
would, as that detection can fail as well, only in more unpredictable ways.

In D136011#3868755 , @dblaikie wrote:

> Yeah, this line of reasoning (non-homogenaeity) is one I'm on board with, 
> thanks for the framing. Basically I/we can think of the debugger's expression 
> context as some code that's compiled with the default char signedness always. 
> Since char signedness isn't part of the ABI, bits of the program can be built 
> with one and bits with the other - and the debugger is just a bit that's 
> built with the default.

Are you sure about the ABI part? I guess it may depend on how you define the 
ABI.. It definitely does not affect the calling conventions, but I'm pretty 
sure it would be an ODR violation if you even had a simple function like `int 
to_int(char c) { return c; }` in a header, and compiled it with both -fsigned 
and -funsigned-char. Not that this will stop people from doing it, but the most 
likely scenario for this is that your linking your application with the C 
library compiled in a different mode, where it is kinda ok, because the C 
library does not have many inline functions, and doesn't do much char 
manipulation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136011

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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

2022-10-20 Thread Aaron Ballman via Phabricator via lldb-commits 
aaron.ballman added a comment.

In D134878#3869947 , @mehdi_amini 
wrote:

> From what I saw when you posted the discourse thread initially, I understand 
> you're targeting user-visible features, and mostly from the "toolchain" side 
> of the project.
> However I find the language here to be potentially confusing for the API 
> surface of the libraries: that is how much of this applies to the LLVM C++ 
> libraries API surface?
> If this is out-of-scope, can this be called out more explicitly?

Sure! Would it help to add a paragraph before the bulleted list in `breaking` 
that says something along the lines of:

The C++ interfaces of individual library components of projects like LLVM or 
Clang are not intended to be stable interfaces. Potentially disruptive changes 
to such C++ interfaces do not constitute a breaking change unless the 
disruption is exposed via another mechanism such as a stable C API.

(Feel free to wordsmith it!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134878

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

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

2022-10-20 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

Hey, this patch still breaks TestArray.py. Could you please take another look?

https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/47748/testReport/lldb-api/commands_frame_diagnose_array/TestArray_py/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132734

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

[Lldb-commits] [PATCH] D136306: [WIP][lldb][CPlusPlus] Add abi_tag support to the CPlusPlusNameParser

2022-10-20 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 updated this revision to Diff 469267.
Michael137 added a comment.

- Support certain special tokens


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136306

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
  lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
  lldb/test/API/functionalities/step-avoids-regexp/main.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -120,7 +120,47 @@
"test_return_auto", "()", "const", "std::test_return_auto"},
   {"decltype(auto) std::test_return_auto(int) const", "std",
"test_return_auto", "(int)", "const",
-   "std::test_return_auto"}};
+   "std::test_return_auto"},
+  
+  // abi_tag on class
+  {"v1::v2::Dummy[abi:c1][abi:c2]> "
+   "v1::v2::Dummy[abi:c1][abi:c2]>"
+   "::method2>>(int, v1::v2::Dummy) const &&",
+   // Context
+   "v1::v2::Dummy[abi:c1][abi:c2]>",
+   // Basename
+   "method2>>", 
+   // Args, qualifiers
+   "(int, v1::v2::Dummy)", "const &&",
+   // Full scope-qualified name without args
+   "v1::v2::Dummy[abi:c1][abi:c2]>"
+   "::method2>>"},
+
+  // abi_tag on free function and template argument
+  {"v1::v2::Dummy[abi:c1][abi:c2]> "
+   "v1::v2::with_tag_in_ns[abi:f1][abi:f2]>>(int, v1::v2::Dummy) const &&",
+   // Context
+   "v1::v2",
+   // Basename
+   "with_tag_in_ns[abi:f1][abi:f2]>>",
+   // Args, qualifiers
+   "(int, v1::v2::Dummy)", "const &&",
+   // Full scope-qualified name without args
+   "v1::v2::with_tag_in_ns[abi:f1][abi:f2]>>"},
+
+  // abi_tag with special characters
+  {"auto ns::with_tag_in_ns[abi:special tag,0.0][abi:special tag,1.0]>"
+   "(float) const &&",
+   // Context
+   "ns",
+   // Basename
+   "with_tag_in_ns[abi:special tag,0.0][abi:special tag,1.0]>",
+   // Args, qualifiers
+   "(float)", "const &&",
+   // Full scope-qualified name without args
+   "ns::with_tag_in_ns[abi:special tag,0.0][abi:special tag,1.0]>"}
+  };
 
   for (const auto &test : test_cases) {
 CPlusPlusLanguage::MethodName method(ConstString(test.input));
Index: lldb/test/API/functionalities/step-avoids-regexp/main.cpp
===
--- lldb/test/API/functionalities/step-avoids-regexp/main.cpp
+++ lldb/test/API/functionalities/step-avoids-regexp/main.cpp
@@ -1,4 +1,6 @@
 namespace ignore {
+struct Dummy {};
+
 template  auto auto_ret(T x) { return 0; }
 [[gnu::abi_tag("test")]] int with_tag() { return 0; }
 template  [[gnu::abi_tag("test")]] int with_tag_template() {
@@ -8,9 +10,15 @@
 template  decltype(auto) decltype_auto_ret(T x) { return 0; }
 } // namespace ignore
 
+template 
+[[gnu::abi_tag("test")]] ignore::Dummy with_tag_template_returns_ignore(T x) {
+  return {};
+}
+
 int main() {
   auto v1 = ignore::auto_ret(5);
   auto v2 = ignore::with_tag();
   auto v3 = ignore::decltype_auto_ret(5);
   auto v4 = ignore::with_tag_template();
+  auto v5 = with_tag_template_returns_ignore(5);
 }
Index: lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
===
--- lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
+++ lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
@@ -37,13 +37,10 @@
 self.thread.StepInto()
 self.hit_correct_function("main")
 
-@skipIfWindows
-@expectedFailureAll(bugnumber="rdar://100645742")
-def test_step_avoid_regex_abi_tagged_template(self):
-"""Tests stepping into an ABI tagged function that matches the avoid regex"""
-self.build()
-(_, _, self.thread, _) = lldbutil.run_to_source_breakpoint(self, "with_tag_template", lldb.SBFileSpec('main.cpp'))
-
 # Try to step into ignore::with_tag_template
 self.thread.StepInto()
 self.hit_correct_function("main")
+
+# Step into with_tag_template_returns_ignore (outside 'ignore::' namespace)
+self.thread.StepInto()
+self.hit_correct_function("with_tag_template_returns_ignore")
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
@@ -117,6 +117,7 @@
   void Advance();
   void TakeBack();
   bool ConsumeToken(clang::tok::TokenKind kind);
+
   template  bool Consu

[Lldb-commits] [PATCH] D136306: [lldb][CPlusPlus] Add abi_tag support to the CPlusPlusNameParser

2022-10-20 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 updated this revision to Diff 469282.
Michael137 added a comment.

- Add test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136306

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
  lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
  lldb/test/API/functionalities/step-avoids-regexp/main.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -114,13 +114,55 @@
   {"llvm::Optional::operator*() const volatile &&",
"llvm::Optional", "operator*", "()",
"const volatile &&", "llvm::Optional::operator*"},
+  {"void foo>()",
+   "", "foo>", "()", "", "foo>"},
 
   // auto return type
   {"auto std::test_return_auto() const", "std",
"test_return_auto", "()", "const", "std::test_return_auto"},
   {"decltype(auto) std::test_return_auto(int) const", "std",
"test_return_auto", "(int)", "const",
-   "std::test_return_auto"}};
+   "std::test_return_auto"},
+  
+  // abi_tag on class
+  {"v1::v2::Dummy[abi:c1][abi:c2]> "
+   "v1::v2::Dummy[abi:c1][abi:c2]>"
+   "::method2>>(int, v1::v2::Dummy) const &&",
+   // Context
+   "v1::v2::Dummy[abi:c1][abi:c2]>",
+   // Basename
+   "method2>>", 
+   // Args, qualifiers
+   "(int, v1::v2::Dummy)", "const &&",
+   // Full scope-qualified name without args
+   "v1::v2::Dummy[abi:c1][abi:c2]>"
+   "::method2>>"},
+
+  // abi_tag on free function and template argument
+  {"v1::v2::Dummy[abi:c1][abi:c2]> "
+   "v1::v2::with_tag_in_ns[abi:f1][abi:f2]>>(int, v1::v2::Dummy) const &&",
+   // Context
+   "v1::v2",
+   // Basename
+   "with_tag_in_ns[abi:f1][abi:f2]>>",
+   // Args, qualifiers
+   "(int, v1::v2::Dummy)", "const &&",
+   // Full scope-qualified name without args
+   "v1::v2::with_tag_in_ns[abi:f1][abi:f2]>>"},
+
+  // abi_tag with special characters
+  {"auto ns::with_tag_in_ns[abi:special tag,0.0][abi:special tag,1.0]>"
+   "(float) const &&",
+   // Context
+   "ns",
+   // Basename
+   "with_tag_in_ns[abi:special tag,0.0][abi:special tag,1.0]>",
+   // Args, qualifiers
+   "(float)", "const &&",
+   // Full scope-qualified name without args
+   "ns::with_tag_in_ns[abi:special tag,0.0][abi:special tag,1.0]>"},
+  };
 
   for (const auto &test : test_cases) {
 CPlusPlusLanguage::MethodName method(ConstString(test.input));
Index: lldb/test/API/functionalities/step-avoids-regexp/main.cpp
===
--- lldb/test/API/functionalities/step-avoids-regexp/main.cpp
+++ lldb/test/API/functionalities/step-avoids-regexp/main.cpp
@@ -1,4 +1,6 @@
 namespace ignore {
+struct Dummy {};
+
 template  auto auto_ret(T x) { return 0; }
 [[gnu::abi_tag("test")]] int with_tag() { return 0; }
 template  [[gnu::abi_tag("test")]] int with_tag_template() {
@@ -8,9 +10,15 @@
 template  decltype(auto) decltype_auto_ret(T x) { return 0; }
 } // namespace ignore
 
+template 
+[[gnu::abi_tag("test")]] ignore::Dummy with_tag_template_returns_ignore(T x) {
+  return {};
+}
+
 int main() {
   auto v1 = ignore::auto_ret(5);
   auto v2 = ignore::with_tag();
   auto v3 = ignore::decltype_auto_ret(5);
   auto v4 = ignore::with_tag_template();
+  auto v5 = with_tag_template_returns_ignore(5);
 }
Index: lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
===
--- lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
+++ lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
@@ -37,13 +37,10 @@
 self.thread.StepInto()
 self.hit_correct_function("main")
 
-@skipIfWindows
-@expectedFailureAll(bugnumber="rdar://100645742")
-def test_step_avoid_regex_abi_tagged_template(self):
-"""Tests stepping into an ABI tagged function that matches the avoid regex"""
-self.build()
-(_, _, self.thread, _) = lldbutil.run_to_source_breakpoint(self, "with_tag_template", lldb.SBFileSpec('main.cpp'))
-
 # Try to step into ignore::with_tag_template
 self.thread.StepInto()
 self.hit_correct_function("main")
+
+# Step into with_tag_template_returns_ignore (outside 'ignore::' namespace)
+self.thread.StepInto()
+self.hit_correct_function("with_tag_template_returns_ignore")
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
==

[Lldb-commits] [PATCH] D136209: [LLDB][NativePDB] Fix parameter size for member functions LF_MFUNCTION

2022-10-20 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu added a comment.

In D136209#3866933 , @rnk wrote:

> Right, so `cantFail` must be an assertions-only check. That's not great.

With assertion enabled, lldb still silently deserialize member functions as 
non-member functions. My guess is that `TypeDeserializer::deserializeAs` 
doesn't return error in this case since `sizeof(MemberFunctionRecord)` > 
`sizeof(ProcedureRecord)`. We may need additional checking in 
TypeDeserializer::deserializeAs to verify that we are deserializing the correct 
type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136209

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

[Lldb-commits] [PATCH] D136371: [trace][intel pt] Correctly treat kernel CPUs as individual threads

2022-10-20 Thread Sujin Park via Phabricator via lldb-commits 
persona0220 created this revision.
persona0220 added reviewers: wallace, jj10306.
Herald added a project: All.
persona0220 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Resolved a bug in kernel decoding and correctly treat kernel CPUs as individual 
threads.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136371

Files:
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp


Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -89,7 +89,7 @@
   trace_sp->m_storage.tsc_conversion =
   bundle_description.tsc_perf_zero_conversion;
 
-  if (bundle_description.cpus) {
+  if (bundle_description.cpus && trace_mode == TraceMode::UserMode) {
 std::vector cpus;
 
 for (const JSONCpu &cpu : *bundle_description.cpus) {


Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -89,7 +89,7 @@
   trace_sp->m_storage.tsc_conversion =
   bundle_description.tsc_perf_zero_conversion;
 
-  if (bundle_description.cpus) {
+  if (bundle_description.cpus && trace_mode == TraceMode::UserMode) {
 std::vector cpus;
 
 for (const JSONCpu &cpu : *bundle_description.cpus) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136371: [trace][intel pt] Correctly treat kernel CPUs as individual threads

2022-10-20 Thread walter erquinigo via Phabricator via lldb-commits 
wallace accepted this revision.
wallace added a comment.
This revision is now accepted and ready to land.

nice. I expected something as simple as that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136371

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

[Lldb-commits] [PATCH] D136207: [lldb] Fix breakpoint setting so it always works when there is a line entry in a compile unit's line table.

2022-10-20 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg updated this revision to Diff 469342.
clayborg added a comment.

Create a new "resolved_sc" that gets used to resolve the line_entry's address. 
If the resolving goes to plan, append that symbol context, else append the 
original one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136207

Files:
  lldb/source/Symbol/CompileUnit.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/bad_aranges.yaml

Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/bad_aranges.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/bad_aranges.yaml
@@ -0,0 +1,445 @@
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x10C
+  cpusubtype:  0x0
+  filetype:0xA
+  ncmds:   7
+  sizeofcmds:  1240
+  flags:   0x0
+  reserved:0x0
+LoadCommands:
+  - cmd: LC_UUID
+cmdsize: 24
+uuid:030921EA-6D76-3A68-B515-386B9AF6D568
+  - cmd: LC_BUILD_VERSION
+cmdsize: 24
+platform:1
+minos:   786432
+sdk: 787200
+ntools:  0
+  - cmd: LC_SYMTAB
+cmdsize: 24
+symoff:  4096
+nsyms:   2
+stroff:  4128
+strsize: 28
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  16384
+fileoff: 0
+filesize:0
+maxprot: 5
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x13F94
+size:36
+offset:  0x0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x8400
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE0C010A000700D8041B00
+  - sectname:__unwind_info
+segname: __TEXT
+addr:0x13FB8
+size:72
+offset:  0x0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE0C010A000700D8041B001800030921EA6D763A68B515386B9AF6D5683200180001000C00
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __LINKEDIT
+vmaddr:  4294983680
+vmsize:  4096
+fileoff: 4096
+filesize:60
+maxprot: 1
+initprot:1
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 792
+segname: __DWARF
+vmaddr:  4294987776
+vmsize:  4096
+fileoff: 8192
+filesize:796
+maxprot: 7
+initprot:3
+nsects:  9
+flags:   0
+Sections:
+  - sectname:__debug_line
+segname: __DWARF
+addr:0x15000
+size:64
+offset:  0x2000
+align:   0
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+  - sectname:__debug_aranges
+segname: __DWARF
+addr:0x15040
+size:48
+offset:  0x2040
+align:   0
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+  - sectname:__debug_info
+segname: __DWARF
+addr:0x15070
+size:148
+offset:  0x2070
+align:   0
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+  - sectname:__debug_abbrev
+segname:

[Lldb-commits] [lldb] 20c7ec1 - [lldb][trace] Correctly treat kernel CPUs as individual threads

2022-10-20 Thread Sujin Park via lldb-commits 

Author: Sujin Park
Date: 2022-10-20T13:37:08-07:00
New Revision: 20c7ec12724059388e51e9d00f5c63983ad0e146

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

LOG: [lldb][trace] Correctly treat kernel CPUs as individual threads

Resolved a bug in kernel decoding and correctly treat kernel CPUs as
individual threads.

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

Added: 


Modified: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
index 0a3bc64098b50..1db34d8fe840d 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -89,7 +89,7 @@ TraceIntelPTSP TraceIntelPT::CreateInstanceForPostmortemTrace(
   trace_sp->m_storage.tsc_conversion =
   bundle_description.tsc_perf_zero_conversion;
 
-  if (bundle_description.cpus) {
+  if (bundle_description.cpus && trace_mode == TraceMode::UserMode) {
 std::vector cpus;
 
 for (const JSONCpu &cpu : *bundle_description.cpus) {



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

[Lldb-commits] [PATCH] D136371: [trace][intel pt] Correctly treat kernel CPUs as individual threads

2022-10-20 Thread Sujin Park via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG20c7ec127240: [lldb][trace] Correctly treat kernel CPUs as 
individual threads (authored by persona0220).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136371

Files:
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp


Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -89,7 +89,7 @@
   trace_sp->m_storage.tsc_conversion =
   bundle_description.tsc_perf_zero_conversion;
 
-  if (bundle_description.cpus) {
+  if (bundle_description.cpus && trace_mode == TraceMode::UserMode) {
 std::vector cpus;
 
 for (const JSONCpu &cpu : *bundle_description.cpus) {


Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -89,7 +89,7 @@
   trace_sp->m_storage.tsc_conversion =
   bundle_description.tsc_perf_zero_conversion;
 
-  if (bundle_description.cpus) {
+  if (bundle_description.cpus && trace_mode == TraceMode::UserMode) {
 std::vector cpus;
 
 for (const JSONCpu &cpu : *bundle_description.cpus) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136207: [lldb] Fix breakpoint setting so it always works when there is a line entry in a compile unit's line table.

2022-10-20 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg updated this revision to Diff 469344.
clayborg added a comment.

Also test on windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136207

Files:
  lldb/source/Symbol/CompileUnit.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/bad_aranges.yaml

Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/bad_aranges.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/bad_aranges.yaml
@@ -0,0 +1,445 @@
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x10C
+  cpusubtype:  0x0
+  filetype:0xA
+  ncmds:   7
+  sizeofcmds:  1240
+  flags:   0x0
+  reserved:0x0
+LoadCommands:
+  - cmd: LC_UUID
+cmdsize: 24
+uuid:030921EA-6D76-3A68-B515-386B9AF6D568
+  - cmd: LC_BUILD_VERSION
+cmdsize: 24
+platform:1
+minos:   786432
+sdk: 787200
+ntools:  0
+  - cmd: LC_SYMTAB
+cmdsize: 24
+symoff:  4096
+nsyms:   2
+stroff:  4128
+strsize: 28
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  16384
+fileoff: 0
+filesize:0
+maxprot: 5
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x13F94
+size:36
+offset:  0x0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x8400
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE0C010A000700D8041B00
+  - sectname:__unwind_info
+segname: __TEXT
+addr:0x13FB8
+size:72
+offset:  0x0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE0C010A000700D8041B001800030921EA6D763A68B515386B9AF6D5683200180001000C00
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __LINKEDIT
+vmaddr:  4294983680
+vmsize:  4096
+fileoff: 4096
+filesize:60
+maxprot: 1
+initprot:1
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 792
+segname: __DWARF
+vmaddr:  4294987776
+vmsize:  4096
+fileoff: 8192
+filesize:796
+maxprot: 7
+initprot:3
+nsects:  9
+flags:   0
+Sections:
+  - sectname:__debug_line
+segname: __DWARF
+addr:0x15000
+size:64
+offset:  0x2000
+align:   0
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+  - sectname:__debug_aranges
+segname: __DWARF
+addr:0x15040
+size:48
+offset:  0x2040
+align:   0
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+  - sectname:__debug_info
+segname: __DWARF
+addr:0x15070
+size:148
+offset:  0x2070
+align:   0
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+  - sectname:__debug_abbrev
+segname: __DWARF
+addr:0x15104
+size:90
+offset:  0x2104
+align:   0
+

[Lldb-commits] [PATCH] D136306: [lldb][CPlusPlus] Add abi_tag support to the CPlusPlusNameParser

2022-10-20 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

As much as I dislike expanding our hand-rolled parser, this does seem small and 
important enough to warrant doing it!




Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp:260
+  Bookmark start_position = SetBookmark();
+  if (!ConsumeToken(tok::l_square)) {
+return false;

nit: the function is inconsistent about whether to use curly braces on single 
statements. LLVM style says don't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136306

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

[Lldb-commits] [PATCH] D136006: [LLDB][NativePDB] Improve ParseDeclsForContext time.

2022-10-20 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu updated this revision to Diff 469350.
zequanwu edited the summary of this revision.
zequanwu added a comment.

Don't try to complete types in `PdbAstBuilder::ParseDeclsForContext`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136006

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h

Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
@@ -11,6 +11,7 @@
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Threading.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangASTImporter.h"
 
@@ -122,7 +123,9 @@
  TypeIndex func_ti, CompilerType func_ct,
  uint32_t param_count, clang::StorageClass func_storage,
  bool is_inline, clang::DeclContext *parent);
-  void ParseAllNamespacesPlusChildrenOf(llvm::Optional parent);
+  void ParseNamespace(clang::DeclContext &parent);
+  void ParseAllTypes();
+  void ParseAllFunctionsAndNonLocalVars();
   void ParseDeclsForSimpleContext(clang::DeclContext &context);
   void ParseBlockChildren(PdbCompilandSymId block_id);
 
@@ -135,7 +138,8 @@
   TypeSystemClang &m_clang;
 
   ClangASTImporter m_importer;
-
+  llvm::once_flag m_parse_functions_and_non_local_vars;
+  llvm::once_flag m_parse_all_types;
   llvm::DenseMap m_decl_to_status;
   llvm::DenseMap m_uid_to_decl;
   llvm::DenseMap m_uid_to_type;
@@ -145,6 +149,7 @@
   llvm::DenseMap, 8>>
   m_cxx_record_map;
+  llvm::DenseSet m_parsed_namespaces;
 };
 
 } // namespace npdb
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -21,7 +21,6 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
-
 #include "PdbUtil.h"
 #include "UdtRecordCompleter.h"
 #include "SymbolFileNativePDB.h"
@@ -1232,8 +1231,11 @@
   return llvm::isa(&context);
 }
 
-void PdbAstBuilder::ParseAllNamespacesPlusChildrenOf(
-llvm::Optional parent) {
+void PdbAstBuilder::ParseNamespace(clang::DeclContext &context) {
+  clang::NamespaceDecl *ns = llvm::dyn_cast(&context);
+  if (m_parsed_namespaces.contains(ns))
+return;
+  std::string qname = ns->getQualifiedNameAsString();
   SymbolFileNativePDB *pdb = static_cast(
   m_clang.GetSymbolFile()->GetBackingSymbolFile());
   PdbIndex &index = pdb->GetIndex();
@@ -1247,12 +1249,6 @@
 
 CVTagRecord tag = CVTagRecord::create(cvt);
 
-if (!parent) {
-  clang::QualType qt = GetOrCreateType(tid);
-  CompleteType(qt);
-  continue;
-}
-
 // Call CreateDeclInfoForType unconditionally so that the namespace info
 // gets created.  But only call CreateRecordType if the namespace name
 // matches.
@@ -1263,41 +1259,68 @@
   continue;
 
 clang::NamespaceDecl *ns = llvm::cast(context);
-std::string actual_ns = ns->getQualifiedNameAsString();
-if (llvm::StringRef(actual_ns).startswith(*parent)) {
-  clang::QualType qt = GetOrCreateType(tid);
-  CompleteType(qt);
-  continue;
+llvm::StringRef ns_name = ns->getName();
+if (ns_name.startswith(qname)) {
+  ns_name = ns_name.drop_front(qname.size());
+  if (ns_name.startswith("::"))
+GetOrCreateType(tid);
 }
   }
+  ParseAllFunctionsAndNonLocalVars();
+  m_parsed_namespaces.insert(ns);
+}
 
-  uint32_t module_count = index.dbi().modules().getModuleCount();
-  for (uint16_t modi = 0; modi < module_count; ++modi) {
-CompilandIndexItem &cii = index.compilands().GetOrCreateCompiland(modi);
-const CVSymbolArray &symbols = cii.m_debug_stream.getSymbolArray();
-auto iter = symbols.begin();
-while (iter != symbols.end()) {
-  PdbCompilandSymId sym_id{modi, iter.offset()};
-
-  switch (iter->kind()) {
-  case S_GPROC32:
-  case S_LPROC32:
-GetOrCreateFunctionDecl(sym_id);
-iter = symbols.at(getScopeEndOffset(*iter));
-break;
-  case S_GDATA32:
-  case S_GTHREAD32:
-  case S_LDATA32:
-  case S_LTHREAD32:
-GetOrCreateVariableDecl(PdbCompilandSymId(modi, 0), sym_id);
-++iter;
-break;
-  default:
-++iter;
+void PdbAstBuilder::ParseAllTypes() {
+  llvm::call_once(m_parse_all_types, [this]() {
+SymbolFileNativePDB *pdb = static_cast(
+m_clang.GetSymbolFile()->GetBackingSymbolFile());
+PdbIndex &index = pdb->GetIndex();
+TypeIndex ti{index.tpi().TypeIndexBegin()};
+for (const CVType &cvt : index.tpi().typeArray()) {
+  PdbTypeSymId tid

[Lldb-commits] [PATCH] D136006: [LLDB][NativePDB] Improve ParseDeclsForContext time.

2022-10-20 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1413-1414
   if (context.isTranslationUnit()) {
-ParseAllNamespacesPlusChildrenOf(llvm::None);
+ParseAllTypes();
+ParseAllFunctionsAndNonLocalVars();
 return;

labath wrote:
> I have a feeling this is still doing more work than it would be necessary. I 
> haven't checked, but I'd expect that here it should be sufficient to parse 
> only the top level namespace names (not their contents), and create forward 
> declarations for the classes in the global namespace. I suspect this is doing 
> much more than that.
> (Of course, if PDB makes it hard to parse just this information, then it 
> might actually be better to parse everything -- I just don't know)
Yeah, we can do that. 
Update to don't try to complete types at all in 
`PdbAstBuilder::ParseDeclsForContext`. For a chrome crash report I'm looking 
at, the time for evaluating a unknown identifier(which will have search scope 
being a TU) drops from 287s -> 160s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136006

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

[Lldb-commits] [lldb] ba8ded6 - [lldb] Don't check environment default char signedness when creating clang type for "char"

2022-10-20 Thread Arthur Eubanks via lldb-commits 

Author: Arthur Eubanks
Date: 2022-10-20T15:03:36-07:00
New Revision: ba8ded6820fa610c7460fe86cd1f41f1df4bcc6c

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

LOG: [lldb] Don't check environment default char signedness when creating clang 
type for "char"

With -f(un)signed-char, the die corresponding to "char" may be the opposite 
DW_ATE_(un)signed_char from the default platform signedness.
Ultimately we should determine whether a type is the unspecified signedness 
char by looking if its name is "char" (as opposed to "signed char"/"unsigned 
char") and not care about DW_ATE_(un)signed_char matching the platform default.

Fixes #23443

Reviewed By: labath

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

Added: 


Modified: 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/test/API/commands/expression/char/TestExprsChar.py
lldb/test/API/commands/expression/char/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 0185bf8f052d8..3ebd42c996bdc 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1063,7 +1063,7 @@ CompilerType 
TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
 break;
 
   case DW_ATE_signed_char:
-if (ast.getLangOpts().CharIsSigned && type_name == "char") {
+if (type_name == "char") {
   if (QualTypeMatchesBitSize(bit_size, ast, ast.CharTy))
 return GetType(ast.CharTy);
 }
@@ -1115,7 +1115,7 @@ CompilerType 
TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
 break;
 
   case DW_ATE_unsigned_char:
-if (!ast.getLangOpts().CharIsSigned && type_name == "char") {
+if (type_name == "char") {
   if (QualTypeMatchesBitSize(bit_size, ast, ast.CharTy))
 return GetType(ast.CharTy);
 }

diff  --git a/lldb/test/API/commands/expression/char/TestExprsChar.py 
b/lldb/test/API/commands/expression/char/TestExprsChar.py
index 849615ef91453..02a2e8c6d31a7 100644
--- a/lldb/test/API/commands/expression/char/TestExprsChar.py
+++ b/lldb/test/API/commands/expression/char/TestExprsChar.py
@@ -14,30 +14,15 @@ def do_test(self, dictionary=None):
 self.expect_expr("foo(c)", result_value="1")
 self.expect_expr("foo(sc)", result_value="2")
 self.expect_expr("foo(uc)", result_value="3")
+self.expect_expr("g", result_type="char")
+self.expect_expr("gs", result_type="signed char")
+self.expect_expr("gu", result_type="unsigned char")
 
 def test_default_char(self):
 self.do_test()
 
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"], 
bugnumber="llvm.org/pr23069")
-@expectedFailureAll(
-archs=[
-"powerpc64le",
-"s390x"],
-bugnumber="llvm.org/pr23069")
 def test_signed_char(self):
 self.do_test(dictionary={'CFLAGS_EXTRAS': '-fsigned-char'})
 
-@expectedFailureAll(
-archs=[
-"i[3-6]86",
-"x86_64",
-"arm64",
-'arm64e',
-'armv7',
-'armv7k',
-'arm64_32'],
-bugnumber="llvm.org/pr23069, ")
-@expectedFailureAll(triple='mips*', bugnumber="llvm.org/pr23069")
-@expectedFailureAll(oslist=['windows'], archs=['aarch64'], 
bugnumber="llvm.org/pr23069")
 def test_unsigned_char(self):
 self.do_test(dictionary={'CFLAGS_EXTRAS': '-funsigned-char'})

diff  --git a/lldb/test/API/commands/expression/char/main.cpp 
b/lldb/test/API/commands/expression/char/main.cpp
index 9ff4436d88a08..23eb554e6ad66 100644
--- a/lldb/test/API/commands/expression/char/main.cpp
+++ b/lldb/test/API/commands/expression/char/main.cpp
@@ -1,5 +1,9 @@
 #include 
 
+char g = 0;
+signed char gs = 0;
+unsigned char gu = 0;
+
 int foo(char c) { return 1; }
 int foo(signed char c) { return 2; }
 int foo(unsigned char c) { return 3; }



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

[Lldb-commits] [PATCH] D136011: [lldb] Don't check environment default char signedness when creating clang type for "char"

2022-10-20 Thread Arthur Eubanks via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGba8ded6820fa: [lldb] Don't check environment default 
char signedness when creating clang type… (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136011

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/commands/expression/char/TestExprsChar.py
  lldb/test/API/commands/expression/char/main.cpp


Index: lldb/test/API/commands/expression/char/main.cpp
===
--- lldb/test/API/commands/expression/char/main.cpp
+++ lldb/test/API/commands/expression/char/main.cpp
@@ -1,5 +1,9 @@
 #include 
 
+char g = 0;
+signed char gs = 0;
+unsigned char gu = 0;
+
 int foo(char c) { return 1; }
 int foo(signed char c) { return 2; }
 int foo(unsigned char c) { return 3; }
Index: lldb/test/API/commands/expression/char/TestExprsChar.py
===
--- lldb/test/API/commands/expression/char/TestExprsChar.py
+++ lldb/test/API/commands/expression/char/TestExprsChar.py
@@ -14,30 +14,15 @@
 self.expect_expr("foo(c)", result_value="1")
 self.expect_expr("foo(sc)", result_value="2")
 self.expect_expr("foo(uc)", result_value="3")
+self.expect_expr("g", result_type="char")
+self.expect_expr("gs", result_type="signed char")
+self.expect_expr("gu", result_type="unsigned char")
 
 def test_default_char(self):
 self.do_test()
 
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"], 
bugnumber="llvm.org/pr23069")
-@expectedFailureAll(
-archs=[
-"powerpc64le",
-"s390x"],
-bugnumber="llvm.org/pr23069")
 def test_signed_char(self):
 self.do_test(dictionary={'CFLAGS_EXTRAS': '-fsigned-char'})
 
-@expectedFailureAll(
-archs=[
-"i[3-6]86",
-"x86_64",
-"arm64",
-'arm64e',
-'armv7',
-'armv7k',
-'arm64_32'],
-bugnumber="llvm.org/pr23069, ")
-@expectedFailureAll(triple='mips*', bugnumber="llvm.org/pr23069")
-@expectedFailureAll(oslist=['windows'], archs=['aarch64'], 
bugnumber="llvm.org/pr23069")
 def test_unsigned_char(self):
 self.do_test(dictionary={'CFLAGS_EXTRAS': '-funsigned-char'})
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1063,7 +1063,7 @@
 break;
 
   case DW_ATE_signed_char:
-if (ast.getLangOpts().CharIsSigned && type_name == "char") {
+if (type_name == "char") {
   if (QualTypeMatchesBitSize(bit_size, ast, ast.CharTy))
 return GetType(ast.CharTy);
 }
@@ -1115,7 +1115,7 @@
 break;
 
   case DW_ATE_unsigned_char:
-if (!ast.getLangOpts().CharIsSigned && type_name == "char") {
+if (type_name == "char") {
   if (QualTypeMatchesBitSize(bit_size, ast, ast.CharTy))
 return GetType(ast.CharTy);
 }


Index: lldb/test/API/commands/expression/char/main.cpp
===
--- lldb/test/API/commands/expression/char/main.cpp
+++ lldb/test/API/commands/expression/char/main.cpp
@@ -1,5 +1,9 @@
 #include 
 
+char g = 0;
+signed char gs = 0;
+unsigned char gu = 0;
+
 int foo(char c) { return 1; }
 int foo(signed char c) { return 2; }
 int foo(unsigned char c) { return 3; }
Index: lldb/test/API/commands/expression/char/TestExprsChar.py
===
--- lldb/test/API/commands/expression/char/TestExprsChar.py
+++ lldb/test/API/commands/expression/char/TestExprsChar.py
@@ -14,30 +14,15 @@
 self.expect_expr("foo(c)", result_value="1")
 self.expect_expr("foo(sc)", result_value="2")
 self.expect_expr("foo(uc)", result_value="3")
+self.expect_expr("g", result_type="char")
+self.expect_expr("gs", result_type="signed char")
+self.expect_expr("gu", result_type="unsigned char")
 
 def test_default_char(self):
 self.do_test()
 
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"], bugnumber="llvm.org/pr23069")
-@expectedFailureAll(
-archs=[
-"powerpc64le",
-"s390x"],
-bugnumber="llvm.org/pr23069")
 def test_signed_char(self):
 self.do_test(dictionary={'CFLAGS_EXTRAS': '-fsigned-char'})
 
-@expectedFailureAll(
-archs=[
-"i[3-6]86",
-"x86_64",
-"arm64",
-'arm64e',
-'armv7',
-'armv7k',
-'arm64_32'],
-bugnumber="llvm.org/pr23069, ")
-@expectedFailureAll(triple='mips*', bugnumber="llvm.org/pr23069")
-

[Lldb-commits] [PATCH] D136295: Fix exception description in lldb-vscode

2022-10-20 Thread jeffrey tan via Phabricator via lldb-commits 
yinghuitan updated this revision to Diff 469374.
yinghuitan added a comment.

Directly raise signal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136295

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/test/API/tools/lldb-vscode/exception/Makefile
  lldb/test/API/tools/lldb-vscode/exception/TestVSCode_exception.py
  lldb/test/API/tools/lldb-vscode/exception/main.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -930,7 +930,7 @@
   // If no description has been set, then set it to the default thread stopped
   // description. If we have breakpoints that get hit and shouldn't be reported
   // as breakpoints, then they will set the description above.
-  if (ObjectContainsKey(body, "description")) {
+  if (!ObjectContainsKey(body, "description")) {
 char description[1024];
 if (thread.GetStopDescription(description, sizeof(description))) {
   EmplaceSafeString(body, "description", std::string(description));
Index: lldb/test/API/tools/lldb-vscode/exception/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/exception/main.cpp
@@ -0,0 +1,6 @@
+#include 
+
+int main() {
+  raise(SIGABRT);
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/exception/TestVSCode_exception.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/exception/TestVSCode_exception.py
@@ -0,0 +1,23 @@
+"""
+Test stop hooks
+"""
+
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbvscode_testcase
+
+
+class TestVSCode_exception(lldbvscode_testcase.VSCodeTestCaseBase):
+
+@skipIfWindows
+def test_stopped_description(self):
+'''
+TODO
+'''
+program = self.getBuildArtifact("a.out")
+print("test_stopped_description called", flush=True)
+self.build_and_launch(program)
+
+self.vscode.request_continue()
+self.assertTrue(self.verify_stop_exception_info("signal SIGABRT"))
Index: lldb/test/API/tools/lldb-vscode/exception/Makefile
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/exception/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -93,10 +93,10 @@
 return
 self.assertTrue(False, "breakpoint not hit")
 
-def verify_exception_breakpoint_hit(self, filter_label):
+def verify_stop_exception_info(self, expected_description):
 '''Wait for the process we are debugging to stop, and verify the stop
reason is 'exception' and that the description matches
-   'filter_label'
+   'expected_description'
 '''
 stopped_events = self.vscode.wait_for_stopped()
 for stopped_event in stopped_events:
@@ -109,7 +109,7 @@
 if 'description' not in body:
 continue
 description = body['description']
-if filter_label == description:
+if expected_description == description:
 return True
 return False
 
@@ -236,7 +236,7 @@
 
 def continue_to_exception_breakpoint(self, filter_label):
 self.vscode.request_continue()
-self.assertTrue(self.verify_exception_breakpoint_hit(filter_label),
+self.assertTrue(self.verify_stop_exception_info(filter_label),
 'verify we got "%s"' % (filter_label))
 
 def continue_to_exit(self, exitCode=0):


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -930,7 +930,7 @@
   // If no description has been set, then set it to the default thread stopped
   // description. If we have breakpoints that get hit and shouldn't be reported
   // as breakpoints, then they will set the description above.
-  if (ObjectContainsKey(body, "description")) {
+  if (!ObjectContainsKey(body, "description")) {
 char description[1024];
 if (thread.GetStopDescription(description, sizeof(description))) {
   EmplaceSafeString(body, "description", std::string(description));
Index: lldb/test/API/tools/lldb-vscode/exception/main.cpp
===

[Lldb-commits] [PATCH] D136295: Fix exception description in lldb-vscode

2022-10-20 Thread jeffrey tan via Phabricator via lldb-commits 
yinghuitan updated this revision to Diff 469376.
yinghuitan added a comment.

Fix comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136295

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/test/API/tools/lldb-vscode/exception/Makefile
  lldb/test/API/tools/lldb-vscode/exception/TestVSCode_exception.py
  lldb/test/API/tools/lldb-vscode/exception/main.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -930,7 +930,7 @@
   // If no description has been set, then set it to the default thread stopped
   // description. If we have breakpoints that get hit and shouldn't be reported
   // as breakpoints, then they will set the description above.
-  if (ObjectContainsKey(body, "description")) {
+  if (!ObjectContainsKey(body, "description")) {
 char description[1024];
 if (thread.GetStopDescription(description, sizeof(description))) {
   EmplaceSafeString(body, "description", std::string(description));
Index: lldb/test/API/tools/lldb-vscode/exception/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/exception/main.cpp
@@ -0,0 +1,6 @@
+#include 
+
+int main() {
+  raise(SIGABRT);
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/exception/TestVSCode_exception.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/exception/TestVSCode_exception.py
@@ -0,0 +1,24 @@
+"""
+Test exception behavior in VSCode
+"""
+
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbvscode_testcase
+
+
+class TestVSCode_exception(lldbvscode_testcase.VSCodeTestCaseBase):
+
+@skipIfWindows
+def test_stopped_description(self):
+'''
+Test that exception description is shown correctly in stopped
+event.
+'''
+program = self.getBuildArtifact("a.out")
+print("test_stopped_description called", flush=True)
+self.build_and_launch(program)
+
+self.vscode.request_continue()
+self.assertTrue(self.verify_stop_exception_info("signal SIGABRT"))
Index: lldb/test/API/tools/lldb-vscode/exception/Makefile
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/exception/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -93,10 +93,10 @@
 return
 self.assertTrue(False, "breakpoint not hit")
 
-def verify_exception_breakpoint_hit(self, filter_label):
+def verify_stop_exception_info(self, expected_description):
 '''Wait for the process we are debugging to stop, and verify the stop
reason is 'exception' and that the description matches
-   'filter_label'
+   'expected_description'
 '''
 stopped_events = self.vscode.wait_for_stopped()
 for stopped_event in stopped_events:
@@ -109,7 +109,7 @@
 if 'description' not in body:
 continue
 description = body['description']
-if filter_label == description:
+if expected_description == description:
 return True
 return False
 
@@ -236,7 +236,7 @@
 
 def continue_to_exception_breakpoint(self, filter_label):
 self.vscode.request_continue()
-self.assertTrue(self.verify_exception_breakpoint_hit(filter_label),
+self.assertTrue(self.verify_stop_exception_info(filter_label),
 'verify we got "%s"' % (filter_label))
 
 def continue_to_exit(self, exitCode=0):


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -930,7 +930,7 @@
   // If no description has been set, then set it to the default thread stopped
   // description. If we have breakpoints that get hit and shouldn't be reported
   // as breakpoints, then they will set the description above.
-  if (ObjectContainsKey(body, "description")) {
+  if (!ObjectContainsKey(body, "description")) {
 char description[1024];
 if (thread.GetStopDescription(description, sizeof(description))) {
   EmplaceSafeString(body, "description", std::string(description));
Index: ll

[Lldb-commits] [lldb] 8c7a1f8 - Revert "[lldb] Fix member access in GetExpressionPath"

2022-10-20 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-10-20T15:21:59-07:00
New Revision: 8c7a1f87617067bc23c2e0733fe5e3202e1d6e81

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

LOG: Revert "[lldb] Fix member access in GetExpressionPath"

This reverts commit 0205aa4a02570dfeda5807f66756ebdbb102744b because it
breaks TestArray.py:

  a->c = 

I decided to revert instead of disable the test because it looks like a
legitimate issue with the patch.

Added: 


Modified: 
lldb/source/Core/ValueObject.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-synth/TestDataFormatterSynth.py

Removed: 
lldb/test/API/python_api/expression_path/Makefile
lldb/test/API/python_api/expression_path/TestExpressionPath.py
lldb/test/API/python_api/expression_path/main.cpp



diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 226a2c3f690f2..19d86bee40e1f 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -273,6 +273,8 @@ CompilerType ValueObject::MaybeCalculateCompleteType() {
   return compiler_type;
 }
 
+
+
 DataExtractor &ValueObject::GetDataExtractor() {
   UpdateValueIfNeeded(false);
   return m_data;
@@ -407,9 +409,8 @@ ValueObject::GetChildAtIndexPath(llvm::ArrayRef 
idxs,
   return root;
 }
 
-lldb::ValueObjectSP
-ValueObject::GetChildAtIndexPath(llvm::ArrayRef> idxs,
- size_t *index_of_error) {
+lldb::ValueObjectSP ValueObject::GetChildAtIndexPath(
+  llvm::ArrayRef> idxs, size_t *index_of_error) {
   if (idxs.size() == 0)
 return GetSP();
   ValueObjectSP root(GetSP());
@@ -1184,10 +1185,9 @@ bool ValueObject::DumpPrintableRepresentation(
   {
 Status error;
 lldb::WritableDataBufferSP buffer_sp;
-std::pair read_string =
-ReadPointedString(buffer_sp, error, 0,
-  (custom_format == eFormatVectorOfChar) ||
-  (custom_format == eFormatCharArray));
+std::pair read_string = ReadPointedString(
+buffer_sp, error, 0, (custom_format == eFormatVectorOfChar) ||
+ (custom_format == eFormatCharArray));
 lldb_private::formatters::StringPrinter::
 ReadBufferAndDumpToStreamOptions options(*this);
 options.SetData(DataExtractor(
@@ -1552,7 +1552,8 @@ bool ValueObject::GetDeclaration(Declaration &decl) {
   return false;
 }
 
-void ValueObject::AddSyntheticChild(ConstString key, ValueObject *valobj) {
+void ValueObject::AddSyntheticChild(ConstString key,
+ValueObject *valobj) {
   m_synthetic_children[key] = valobj;
 }
 
@@ -1923,96 +1924,64 @@ void ValueObject::GetExpressionPath(Stream &s,
 return;
   }
 
-  // Checks whether a value is dereference of a non-reference parent.
-  // This is used to determine whether to print a dereference operation (*).
-  auto is_deref_of_non_reference = [](ValueObject *value) {
-if (value == nullptr)
-  return false;
-ValueObject *value_parent = value->GetParent();
-if (value_parent) {
-  CompilerType parent_compiler_type = value_parent->GetCompilerType();
-  if (parent_compiler_type) {
-const uint32_t parent_type_info = parent_compiler_type.GetTypeInfo();
-if (parent_type_info & eTypeIsReference)
-  return false;
-  }
-}
-return value->IsDereferenceOfParent();
-  };
-
-  ValueObject *parent = GetParent();
+  const bool is_deref_of_parent = IsDereferenceOfParent();
 
-  if (is_deref_of_non_reference(this) &&
+  if (is_deref_of_parent &&
   epformat == eGetExpressionPathFormatDereferencePointers) {
 // this is the original format of GetExpressionPath() producing code like
 // *(a_ptr).memberName, which is entirely fine, until you put this into
 // StackFrame::GetValueForVariableExpressionPath() which prefers to see
-// a_ptr->memberName. The eHonorPointers mode is meant to produce strings
-// in this latter format.
-s.PutChar('*');
-if (parent)
-  parent->GetExpressionPath(s, epformat);
-return;
+// a_ptr->memberName. the eHonorPointers mode is meant to produce strings
+// in this latter format
+s.PutCString("*(");
   }
 
-  const bool is_deref_of_parent = IsDereferenceOfParent();
-  bool is_parent_deref_of_non_reference = false;
-  bool print_obj_access = false;
-  bool print_ptr_access = false;
-
-  if (!IsBaseClass() && !is_deref_of_parent) {
-ValueObject *non_base_class_parent = GetNonBaseClassParent();
-if (non_base_class_parent && !non_base_class_parent->GetName().IsEmpty()) {
-  CompilerType non_base_class_parent_compiler_type =
-  non_base_c

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

2022-10-20 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 469399.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.

Split off the statistics


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

https://reviews.llvm.org/D134991

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/lldb/include/lldb/Utility/Diagnostics.h
  lldb/lldb/source/Utility/Diagnostics.cpp
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Initialization/SystemInitializerCommon.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/Diagnostics.cpp
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -788,6 +788,10 @@
<< '\n';
 return 1;
   }
+
+  // Setup LLDB signal handlers once the debugger has been initialized.
+  SBDebugger::PrintDiagnosticsOnError();
+
   SBHostOS::ThreadCreated("");
 
   signal(SIGINT, sigint_handler);
Index: lldb/source/Utility/Diagnostics.cpp
===
--- /dev/null
+++ lldb/source/Utility/Diagnostics.cpp
@@ -0,0 +1,74 @@
+//===-- Diagnostics.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Utility/Diagnostics.h"
+#include "lldb/Utility/LLDBAssert.h"
+
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace lldb_private;
+using namespace lldb;
+using namespace llvm;
+
+void Diagnostics::Initialize() {
+  lldbassert(!InstanceImpl() && "Already initialized.");
+  InstanceImpl().emplace();
+}
+
+void Diagnostics::Terminate() {
+  lldbassert(InstanceImpl() && "Already terminated.");
+  InstanceImpl().reset();
+}
+
+Optional &Diagnostics::InstanceImpl() {
+  static Optional g_diagnostics;
+  return g_diagnostics;
+}
+
+Diagnostics &Diagnostics::Instance() { return *InstanceImpl(); }
+
+Diagnostics::Diagnostics() {}
+
+Diagnostics::~Diagnostics() {}
+
+void Diagnostics::AddCallback(Callback callback) {
+  std::lock_guard guard(m_callbacks_mutex);
+  m_callbacks.push_back(callback);
+}
+
+bool Diagnostics::Dump(raw_ostream &stream) {
+  SmallString<128> diagnostics_dir;
+  std::error_code ec =
+  sys::fs::createUniqueDirectory("diagnostics", diagnostics_dir);
+  if (ec) {
+stream << "unable to create diagnostic dir: "
+   << toString(errorCodeToError(ec)) << '\n';
+return false;
+  }
+
+  stream << "LLDB diagnostics written to " << diagnostics_dir << "\n";
+  stream << "Please include the directory content when filing a bug report\n";
+
+  Error error = Create(FileSpec(diagnostics_dir.str()));
+  if (error) {
+stream << toString(std::move(error)) << '\n';
+return false;
+  }
+
+  return true;
+}
+
+Error Diagnostics::Create(const FileSpec &dir) {
+  for (Callback c : m_callbacks) {
+if (Error err = c(dir))
+  return err;
+  }
+  return Error::success();
+}
Index: lldb/source/Utility/CMakeLists.txt
===
--- lldb/source/Utility/CMakeLists.txt
+++ lldb/source/Utility/CMakeLists.txt
@@ -35,6 +35,7 @@
   DataBufferLLVM.cpp
   DataEncoder.cpp
   DataExtractor.cpp
+  Diagnostics.cpp
   Environment.cpp
   Event.cpp
   FileSpec.cpp
Index: lldb/source/Initialization/SystemInitializerCommon.cpp
===
--- lldb/source/Initialization/SystemInitializerCommon.cpp
+++ lldb/source/Initialization/SystemInitializerCommon.cpp
@@ -12,6 +12,8 @@
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/Socket.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Utility/Diagnostics.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/Version/Version.h"
@@ -63,6 +65,7 @@
 
   InitializeLldbChannel();
 
+  Diagnostics::Initialize();
   FileSystem::Initialize();
   HostInfo::Initialize(m_shlib_dir_helper);
 
@@ -95,4 +98,5 @@
   HostInfo::Terminate();
   Log::DisableAllLogChannels();
   FileSystem::Terminate();
+  Diagnostics::Terminate();
 }
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -44,6 +44,7 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Utility/AnsiTerminal.h"
+#include "lldb/Utility/Diagnostics.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Listener.h"
Index: lldb/source