[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

2022-09-15 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
fdeazeve added a comment.

In D133858#3791290 , @mib wrote:

> I don't think it's necessary to add the `DoWillAttachToProcessWithID` method 
> and override it in each process plugin. I think you could have just reset the 
> `hit_counter` in the top-level `Process::{Attach,Launch}` class.

@jingham Thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133858

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


[Lldb-commits] [PATCH] D133906: [lldb] Generate lldb-forward with .def file

2022-09-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D133906#3791352 , @JDevlieghere 
wrote:

> In D133906#3791153 , @jingham wrote:
>
>> This patch makes me a little sad because it breaks the "Jump to Definition" 
>> chain in Xcode (and I bet it does in other IDE's that support this.)  You 
>> used to be able to do "Jump to Definition" on ProcessSP, then jump to 
>> definition on the class name in the shared pointer definition to jump to the 
>> class.  Now the first jump takes you to:
>>
>> LLDB_FORWARD_CLASS(Process)
>>
>> in the lldb-forward.def file, and you can't go any further because the IDE 
>> can't tell what to do with the contents of the .def file (it has no way to 
>> know how it was imported to create real definitions).  These .def insertions 
>> almost always make things harder to find in the actual code, so they aren't 
>> free.
>
> The alternative would be to have tablegen generate the header, but I think 
> that's overkill, and I'm not even sure Xcode would pick the generated header.

I have a feeling that the code would be simpler if you reversed the "iteration 
order", and it might even make the go-to definition command more useful. I'm 
thinking of something like

  // no .def file
  #define LLDB_FORWARD_CLASS(cls) \
namespace lldb_private { class cls; } \
namespace lldb {  using cls##SP = std::shared_ptr; } \
...
  
  LLDB_FORWARD_CLASS(Foo)
  LLDB_FORWARD_CLASS(Bar)
  ...

That said, my preferred solution would be something like `template 
using SP = std::shared_ptr`, and then replacing all `XyzSP` with `SP`. 
The two main benefits (besides simplifying the forward file) I see are:

- being able to write `SP`. With the current pattern, you'd have to 
introduce a whole new class of typedefs, or live with the fact that 
`shared_ptr` looks very different from XyzSP
- being compatible with the llvm naming scheme. XyzSP and xyz_sp would collide 
if they both used camel case. With this, they won't because one of them will 
not exist.


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

https://reviews.llvm.org/D133906

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


[Lldb-commits] [PATCH] D133944: [lldb][tests][gmodules] Test for expression evaluator crash for types with template base class

2022-09-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision.
Michael137 added reviewers: aprantl, martong.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The problem here is that the ASTImporter adds
the template base class member FieldDecl to
the DeclContext twice. This happens because
we don't construct a `LookupPtr` for decls
that originate from modules and thus the
ASTImporter never realizes that the FieldDecl
has already been imported. These duplicate
decls then break the assumption of the LayoutBuilder
which expects only a single member decl to
exist.

The test will be fixed by a follow-up revision
and is thus skipped for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133944

Files:
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/Makefile
  
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.cpp
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.h
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/main.cpp
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module.modulemap
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.cpp
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.h
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.cpp
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.h

Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.h
@@ -0,0 +1,10 @@
+#ifndef MOD2_H_IN
+#define MOD2_H_IN
+
+#include "base_module.h"
+
+struct ClassInMod2 {
+  ClassInMod3 VecInMod2;
+};
+
+#endif
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.cpp
@@ -0,0 +1,3 @@
+#include "module2.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.h
@@ -0,0 +1,10 @@
+#ifndef MOD1_H_IN
+#define MOD1_H_IN
+
+#include "base_module.h"
+
+struct ClassInMod1 {
+  ClassInMod3 VecInMod1;
+};
+
+#endif // _H_IN
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.cpp
@@ -0,0 +1,3 @@
+#include "module1.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module.modulemap
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module.modulemap
@@ -0,0 +1,14 @@
+module Module1 {
+  header "module1.h"
+  export *
+}
+
+module Module2 {
+  header "module2.h"
+  export *
+}
+
+module BaseModule {
+  header "base_module.h"
+  export *
+}
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/main.cpp
@@ -0,0 +1,15 @@
+#include "module1.h"
+#include "module2.h"
+
+#include 
+
+int main() {
+  ClassInMod1 FromMod1;
+  ClassInMod2 FromMod2;
+
+  FromMod1.VecInMod1.BaseMember = 137;
+  FromMod2.VecInMod2.BaseMember = 42;
+
+  std::puts("Break here");
+  return 0;
+}
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.h
@@ -0,0 +1,8 @@
+#ifndef MOD3_H_IN
+#define MOD3_H_IN
+
+template  struct ClassInMod3Base { int BaseMember = 0; };
+
+template  struct ClassInMod3 : public ClassInMod3Base {};
+
+#endif // _H_IN
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.cpp
@@ -0,0 +1,3 @@
+#include "base_module.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -0,0 +1,56 @@
+"""
+Tes

[Lldb-commits] [PATCH] D133945: [clang][ASTImporter] Continue with slow lookup in DeclContext::localUncachedLookup when regular lookup fails

2022-09-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision.
Michael137 added reviewers: aprantl, martong.
Herald added a subscriber: rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

The uncached lookup is mainly used in the ASTImporter/LLDB code-path
where we're not allowed to load from external storage. When importing
a FieldDecl with a DeclContext that had no external visible storage
(but came from a Clang module or PCH) the above call to `lookup(Name)`
the regular lookup fails because:

1. `DeclContext::buildLookup` doesn't set `LookupPtr` for decls that came from 
a module
2. LLDB doesn't use the `SharedImporterState`

In such a case but we would never go on to the "slow" path of iterating
through the decls in the DeclContext. In some cases this means that
`ASTNodeImporter::VisitFieldDecl` ends up importing a decl into the
`DeclContext` a second time.

The patch removes the short-circuit in the case where we don't find
any decls via the regular lookup.

**Tests**

- Un-skip the failing LLDB API tests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133945

Files:
  clang/lib/AST/DeclBase.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py


Index: 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.


Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   //

[Lldb-commits] [PATCH] D133945: [clang][ASTImporter] Continue with slow lookup in DeclContext::localUncachedLookup when regular lookup fails

2022-09-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: clang/lib/AST/DeclBase.cpp:1781
   if (Name && !hasLazyLocalLexicalLookups() &&
   !hasLazyExternalLexicalLookups()) {
 if (StoredDeclsMap *Map = LookupPtr) {

Could merge the two if-blocks now I suppose


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

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


[Lldb-commits] [PATCH] D133945: [clang][ASTImporter] Continue with slow lookup in DeclContext::localUncachedLookup when regular lookup fails

2022-09-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 460420.
Michael137 added a comment.

- Merge if-blocks
- Reword commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

Files:
  clang/lib/AST/DeclBase.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py


Index: 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,13 +1771,11 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
-  }
+if (!Results.empty())
+  return;
 
-  // If we have a lookup table, check there first. Maybe we'll get lucky.
-  // FIXME: Should we be checking these flags on the primary context?
-  if (Name && !hasLazyLocalLexicalLookups() &&
-  !hasLazyExternalLexicalLookups()) {
+// If we have a lookup table, check there first. Maybe we'll get lucky.
+// FIXME: Should we be checking these flags on the primary context?
 if (StoredDeclsMap *Map = LookupPtr) {
   StoredDeclsMap::iterator Pos = Map->find(Name);
   if (Pos != Map->end()) {


Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,13 +1771,11 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
-  }
+if (!Results.empty())
+  return;
 
-  // If we have a lookup table, check there first. Maybe we'll get lucky.
-  // FIXME: Should we be checking these flags on the primary context?
-  if (Name && !hasLazyLocalLexicalLookups() &&
-  !hasLazyExternalLexicalLookups()) {
+// If we have a lookup table, check there first. Maybe we'll get lucky.
+// FIXME: Should we be checking these flags on the primary context?
 if (StoredDeclsMap *Map = LookupPtr) {
   StoredDeclsMap::iterator Pos = Map->find(Name);
   if (Pos != Map->end()) {
___
ll

[Lldb-commits] [PATCH] D133945: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

2022-09-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 460429.
Michael137 added a comment.

- Undo incorrect previous change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

Files:
  clang/lib/AST/DeclBase.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py


Index: 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.


Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D133945: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

2022-09-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added inline comments.



Comment at: clang/lib/AST/DeclBase.cpp:1781
   if (Name && !hasLazyLocalLexicalLookups() &&
   !hasLazyExternalLexicalLookups()) {
 if (StoredDeclsMap *Map = LookupPtr) {

Michael137 wrote:
> Could merge the two if-blocks now I suppose
Nevermind, not true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

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


[Lldb-commits] [PATCH] D133906: [lldb] Generate lldb-forward with .def file

2022-09-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D133906#3792230 , @labath wrote:

> In D133906#3791352 , @JDevlieghere 
> wrote:
>
>> In D133906#3791153 , @jingham 
>> wrote:
>>
>>> This patch makes me a little sad because it breaks the "Jump to Definition" 
>>> chain in Xcode (and I bet it does in other IDE's that support this.)  You 
>>> used to be able to do "Jump to Definition" on ProcessSP, then jump to 
>>> definition on the class name in the shared pointer definition to jump to 
>>> the class.  Now the first jump takes you to:
>>>
>>> LLDB_FORWARD_CLASS(Process)
>>>
>>> in the lldb-forward.def file, and you can't go any further because the IDE 
>>> can't tell what to do with the contents of the .def file (it has no way to 
>>> know how it was imported to create real definitions).  These .def 
>>> insertions almost always make things harder to find in the actual code, so 
>>> they aren't free.
>>
>> The alternative would be to have tablegen generate the header, but I think 
>> that's overkill, and I'm not even sure Xcode would pick the generated header.
>
> I have a feeling that the code would be simpler if you reversed the 
> "iteration order", and it might even make the go-to definition command more 
> useful. I'm thinking of something like
>
>   // no .def file
>   #define LLDB_FORWARD_CLASS(cls) \
> namespace lldb_private { class cls; } \
> namespace lldb {  using cls##SP = std::shared_ptr; } \
> ...
>   
>   LLDB_FORWARD_CLASS(Foo)
>   LLDB_FORWARD_CLASS(Bar)
>   ...

Works for me, but I don't see how that would help with go-to definition. Xcode 
still won't show you the macro expansion so there's nothing to click through, 
which was Jim's complaint.

> That said, my preferred solution would be something like `template T> using SP = std::shared_ptr`, and then replacing all `XyzSP` with 
> `SP`. The two main benefits (besides simplifying the forward file) I see 
> are:
>
> - being able to write `SP`. With the current pattern, you'd have 
> to introduce a whole new class of typedefs, or live with the fact that 
> `shared_ptr` looks very different from XyzSP
> - being compatible with the llvm naming scheme. XyzSP and xyz_sp would 
> collide if they both used camel case. With this, they won't because one of 
> them will not exist.

This would address Jim's concern, but it's more churn. If everyone's okay with 
this approach I'm happy to do the mechanical find-and-replace.


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

https://reviews.llvm.org/D133906

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


[Lldb-commits] [PATCH] D133906: [lldb] Generate lldb-forward with .def file

2022-09-15 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment.

SP and UP templates seems fine to me.


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

https://reviews.llvm.org/D133906

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


[Lldb-commits] [PATCH] D133906: [lldb] Generate lldb-forward with .def file

2022-09-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

To some extent, the fact that we define an XxxSP for a class is saying "This is 
a thing we mostly pass around as a shared pointer" which would be lost if every 
time we make an SP or UP we use the same syntax.  But I'm not sure that that's 
a terribly strong objection.


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

https://reviews.llvm.org/D133906

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


[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-09-15 Thread Abramo Bagnara via Phabricator via lldb-commits
Abramo-Bagnara reopened this revision.
Abramo-Bagnara added a comment.

These changes introduce a subtle bug in template instantiation of newly added 
ElaboratedType around TypedefType:

bool refersInstantiatedDecl(QualType T) in SemaTemplateInstantiate.cpp does not 
handle this ElaboratedType correctly then inhibiting the proper instantiation 
of TypedefType whose declaration has been subject to instantiation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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


[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-09-15 Thread Matheus Izvekov via Phabricator via lldb-commits
mizvekov added a comment.

In D112374#3792825 , @Abramo-Bagnara 
wrote:

> These changes introduce a subtle bug in template instantiation of newly added 
> ElaboratedType around TypedefType:
>
> bool refersInstantiatedDecl(QualType T) in SemaTemplateInstantiate.cpp does 
> not handle this ElaboratedType correctly then inhibiting the proper 
> instantiation of TypedefType whose declaration has been subject to 
> instantiation.

I can't find that function, either in that file or anywhere else within `clang` 
sub-project.

Even more amazingly, a google search for that function name yields 0 hits.

Can you post a repro of the problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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


[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-09-15 Thread Abramo Bagnara via Phabricator via lldb-commits
Abramo-Bagnara added a comment.

I have to doubly apologize:

1. my reference to refersInstantiatedDecl is completely wrong and I have been 
mislead by an old patch on my machine.
2. the problem despite being very real is independent by your changes

If you are still interested this is the repro followed by the explaination:

abramo@igor:/tmp$ cat a.c
template 
void p() {

  typedef int x;
  sizeof(x);

}

int main() {

  p();

}
abramo@igor:/tmp$ clang++-16 -cc1 -ast-dump -xc++ a.c
a.c:4:3: warning: expression result unused [-Wunused-value]

  sizeof(x);
  ^

a.c:4:3: warning: expression result unused [-Wunused-value]

  sizeof(x);
  ^

a.c:8:3: note: in instantiation of function template specialization 'p' 
requested here

  p();
  ^

TranslationUnitDecl 0x55df356fc8c8 <> 

| -TypedefDecl 0x55df356fd130 <>  implicit 
__int128_t '__int128'   |
| `-BuiltinType 0x55df356fce90 '__int128'   
   |
| -TypedefDecl 0x55df356fd1a0 <>  implicit 
__uint128_t 'unsigned __int128' |
| `-BuiltinType 0x55df356fceb0 'unsigned __int128'  
   |
| -TypedefDecl 0x55df356fd518 <>  implicit 
__NSConstantString '__NSConstantString_tag' |
| `-RecordType 0x55df356fd290 '__NSConstantString_tag'  
   |
| `-CXXRecord 0x55df356fd1f8 '__NSConstantString_tag'   
   |
| -TypedefDecl 0x55df356fd5b0 <>  implicit 
__builtin_ms_va_list 'char *'   |
| `-PointerType 0x55df356fd570 'char *' 
   |
| `-BuiltinType 0x55df356fc970 'char'   
   |
| -TypedefDecl 0x55df35742aa8 <>  implicit 
__builtin_va_list '__va_list_tag[1]'|
| `-ConstantArrayType 0x55df35742a50 '__va_list_tag[1]' 1   
   |
| `-RecordType 0x55df356fd6a0 '__va_list_tag'   
   |
| `-CXXRecord 0x55df356fd608 '__va_list_tag'
   |
| -FunctionTemplateDecl 0x55df35742ca8  line:2:6 p   
   |
|   
   | -TemplateTypeParmDecl 0x55df35742b00 
 col:19 typename depth 0 index 0  |
|   
   | -FunctionDecl 0x55df35742c08  line:2:6 p 'void ()'|
|   
   | `-CompoundStmt 0x55df35742ed0   |
|   
   |
   | -DeclStmt 0x55df35742e30 
|
|   
   |
   | `-TypedefDecl 
0x55df35742dd8  col:15 referenced x 'int' |
|   
   |
   | `-BuiltinType 
0x55df356fc9d0 'int' |
|   
   | `-UnaryExprOrTypeTraitExpr 0x55df35742eb0 
 'unsigned long' sizeof 'x':'int' |
| `-FunctionDecl 0x55df357430e8  line:2:6 used p 'void ()'  
   |
|   
   | -TemplateArgument type 'int'   
   |
|   
   | `-BuiltinType 0x55df356fc9d0 'int' 
   |
| `-CompoundStmt 0x55df35743328   
   |
|   
   | -DeclStmt 0x55df35743310|
|   
   | `-TypedefDecl 0x55

[Lldb-commits] [PATCH] D133961: [lldb] Use SWIG_fail in python-typemaps.swig (NFC)

2022-09-15 Thread Dave Lee via Phabricator via lldb-commits
kastiglione created this revision.
kastiglione added a reviewer: JDevlieghere.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When attempting to use SWIG's `-builtin` flag, there were a few compile
failures caused by a mismatch between return type and return value. In those
cases, the return type was `int` but many of the type maps assume returning
`NULL`/`nullptr` (only the latter caused compile failures).

This fix abstracts failure paths to use the `SWIG_fail` macro, which performs
`goto fail;`. Each of the generated functions contain a `fail` label, which
performs any resource cleanup and returns the appropriate failure value.

This change isn't strictly necessary at this point, but seems like the right
thing to do, and for anyone who tries `-builtin` later, it resolves those
issues.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133961

Files:
  lldb/bindings/python/python-typemaps.swig

Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -18,7 +18,7 @@
   if (!py_str.IsAllocated()) {
 PyErr_SetString(PyExc_TypeError, "list must contain strings");
 free($1);
-return nullptr;
+SWIG_fail;
   }
 
   $1[i] = const_cast(py_str.GetString().data());
@@ -28,7 +28,7 @@
 $1 = NULL;
   } else {
 PyErr_SetString(PyExc_TypeError, "not a list");
-return NULL;
+SWIG_fail;
   }
 }
 
@@ -70,7 +70,7 @@
   PythonObject obj = Retain($input);
   lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
-return nullptr;
+SWIG_fail;
   $1 = value;
 }
 
@@ -79,10 +79,10 @@
   unsigned long long state_type_value =
   unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
-return nullptr;
+SWIG_fail;
   if (state_type_value > lldb::StateType::kLastStateType) {
 PyErr_SetString(PyExc_ValueError, "Not a valid StateType value");
-return nullptr;
+SWIG_fail;
   }
   $1 = static_cast(state_type_value);
 }
@@ -93,12 +93,12 @@
 %typemap(in) (char *dst, size_t dst_len) {
   if (!PyInt_Check($input)) {
 PyErr_SetString(PyExc_ValueError, "Expecting an integer");
-return NULL;
+SWIG_fail;
   }
   $2 = PyInt_AsLong($input);
   if ($2 <= 0) {
 PyErr_SetString(PyExc_ValueError, "Positive integer expected");
-return NULL;
+SWIG_fail;
   }
   $1 = (char *)malloc($2);
 }
@@ -129,12 +129,12 @@
 %typemap(in) (char *dst_or_null, size_t dst_len) {
   if (!PyInt_Check($input)) {
 PyErr_SetString(PyExc_ValueError, "Expecting an integer");
-return NULL;
+SWIG_fail;
   }
   $2 = PyInt_AsLong($input);
   if ($2 <= 0) {
 PyErr_SetString(PyExc_ValueError, "Positive integer expected");
-return NULL;
+SWIG_fail;
   }
   $1 = (char *)malloc($2);
 }
@@ -166,7 +166,7 @@
 $2 = bytes.GetSize();
   } else {
 PyErr_SetString(PyExc_ValueError, "Expecting a string");
-return NULL;
+SWIG_fail;
   }
 }
 // For SBProcess::WriteMemory, SBTarget::GetInstructions and SBDebugger::DispatchInput.
@@ -186,7 +186,7 @@
 $2 = bytes.GetSize();
   } else {
 PyErr_SetString(PyExc_ValueError, "Expecting a buffer");
-return NULL;
+SWIG_fail;
   }
 }
 
@@ -199,11 +199,11 @@
 $2 = PyLong_AsLong($input);
   } else {
 PyErr_SetString(PyExc_ValueError, "Expecting an integer or long object");
-return NULL;
+SWIG_fail;
   }
   if ($2 <= 0) {
 PyErr_SetString(PyExc_ValueError, "Positive integer expected");
-return NULL;
+SWIG_fail;
   }
   $1 = (void *)malloc($2);
 }
@@ -287,12 +287,12 @@
   if (!SetNumberFromPyObject($1[i], o)) {
 PyErr_SetString(PyExc_TypeError, "list must contain numbers");
 free($1);
-return NULL;
+SWIG_fail;
   }
 
   if (PyErr_Occurred()) {
 free($1);
-return NULL;
+SWIG_fail;
   }
 }
   } else if ($input == Py_None) {
@@ -300,7 +300,7 @@
 $2 = 0;
   } else {
 PyErr_SetString(PyExc_TypeError, "not a list");
-return NULL;
+SWIG_fail;
   }
 }
 
@@ -353,7 +353,7 @@
   if (!($input == Py_None ||
 PyCallable_Check(reinterpret_cast($input {
 PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
-return NULL;
+SWIG_fail;
   }
 
   // FIXME (filcab): We can't currently check if our callback is already
@@ -377,11 +377,11 @@
   PythonFile py_file(PyRefType::Borrowed, $input);
   if (!py_file) {
 PyErr_SetString(PyExc_TypeError, "not a file");
-return nullptr;
+SWIG_fail;
   }
   auto sp = unwrapOrSetPythonException(py_file.ConvertToFile());
   if (!sp)
-return nullptr;
+SWIG_fail;
   $1 = sp;
 }
 
@@ -389,12 +389,12 @@
   PythonFile py_file(PyRefType::Borrowed, $input);
   if (!py_file) {
 PyErr_SetString(PyExc_TypeError, "

[Lldb-commits] [PATCH] D133961: [lldb] Use SWIG_fail in python-typemaps.swig (NFC)

2022-09-15 Thread 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133961

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


[Lldb-commits] [PATCH] D133973: [test] Fix LLDB tests with just-built libcxx when using a target directory.

2022-09-15 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht created this revision.
rupprecht added reviewers: JDevlieghere, fdeazeve.
Herald added a subscriber: mgorny.
Herald added a project: All.
rupprecht requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

In certain configurations, libc++ headers all exist in the same directory, and 
libc++ binaries exist in the same directory as lldb libs. When 
`LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is enabled (*and* the host is not Apple, 
which is why I assume this wasn't caught by others?), this is not the case: 
most headers will exist in the usual `include/c++/v1` directory, but 
`__config_site` exists in `include/$TRIPLE/c++/v1`. Likewise, the libc++.so 
binary exists in `lib/$TRIPLE/`, not `lib/` (where LLDB libraries reside).

The `LIBCXX_` cmake config is borrowed from `libcxx/CMakeLists.txt`. I could 
not figure out a way to share the cmake config; ideally we would reuse the same 
config instead of copy/paste.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133973

Files:
  lldb/packages/Python/lldbsuite/test/builders/builder.py
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/test/API/lit.cfg.py
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt
  lldb/utils/lldb-dotest/lldb-dotest.in

Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -13,6 +13,10 @@
 lldb_framework_dir = "@LLDB_FRAMEWORK_DIR_CONFIGURED@"
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
 llvm_tools_dir = "@LLVM_TOOLS_DIR_CONFIGURED@"
+has_libcxx = @LLDB_HAS_LIBCXX@
+libcxx_libs_dir = "@LIBCXX_LIBRARY_DIR@"
+libcxx_include_dir = "@LIBCXX_GENERATED_INCLUDE_DIR@"
+libcxx_include_target_dir = "@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@"
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -31,6 +35,11 @@
 cmd.extend(['--dsymutil', dsymutil])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
 cmd.extend(['--llvm-tools-dir', llvm_tools_dir])
+if has_libcxx:
+cmd.extend(['--libcxx-include-dir', libcxx_include_dir])
+if libcxx_include_target_dir:
+cmd.extend(['--libcxx-include-target-dir', libcxx_include_target_dir])
+cmd.extend(['--libcxx-library-dir', libcxx_libs_dir])
 if lldb_framework_dir:
 cmd.extend(['--framework', lldb_framework_dir])
 if lldb_build_intel_pt == "1":
Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -9,8 +9,21 @@
 
 llvm_canonicalize_cmake_booleans(
   LLDB_BUILD_INTEL_PT
+  LLDB_HAS_LIBCXX
 )
 
+if ("libcxx" IN_LIST LLVM_ENABLE_RUNTIMES)
+  set(LLDB_HAS_LIBCXX ON)
+  if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE})
+set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
+set(LIBCXX_GENERATED_INCLUDE_TARGET_DIR "${LLVM_BINARY_DIR}/include/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+  else()
+set(LIBCXX_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXX_LIBDIR_SUFFIX})
+set(LIBCXX_GENERATED_INCLUDE_DIR "${CMAKE_BINARY_DIR}/include/c++/v1")
+  endif()
+endif()
+
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}")
 set(vars
   LLDB_TEST_COMMON_ARGS
@@ -23,8 +36,13 @@
   LLDB_TEST_DSYMUTIL
   LLDB_LIBS_DIR
   LLVM_TOOLS_DIR
+  LIBCXX_LIBRARY_DIR
+  LIBCXX_GENERATED_INCLUDE_DIR
+  LIBCXX_GENERATED_INCLUDE_TARGET_DIR
   )
 
+llvm_canonicalize_cmake_booleans(LLDB_HAS_LIBCXX)
+
 # Generate lldb-dotest Python driver script for each build mode.
 if(LLDB_BUILT_STANDALONE)
   set(config_types ".")
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -97,6 +97,14 @@
 
   if (TARGET libcxx OR ("libcxx" IN_LIST LLVM_ENABLE_RUNTIMES))
 set(LLDB_HAS_LIBCXX ON)
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE})
+  set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
+  set(LIBCXX_GENERATED_INCLUDE_TARGET_DIR "${LLVM_BINARY_DIR}/include/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+else()
+  set(LIBCXX_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXX_LIBDIR_SUFFIX})
+  set(LIBCXX_GENERATED_INCLUDE_DIR "${CMAKE_BINARY_DIR}/include/c++/v1")
+endif()
 add_lldb_test_dependency(cxx)
   endif()
 
Index: lldb/test/API/lit.site.cfg.py.in
===
--- lldb/test/API/lit.site.cfg.py.in
+++ lldb/

[Lldb-commits] [PATCH] D133973: [test] Fix LLDB tests with just-built libcxx when using a target directory.

2022-09-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

Do we need both `LIBCPP_INCLUDE_DIR` and `LIBCPP_INCLUDE_TARGET_DIR`? I see we 
pass another `-cxx-isystem` flag, does this result in another search path or 
does that replace the former? If we only need one, we could set 
`LIBCPP_INCLUDE_DIR` to `libcxx_include_target_dir` or `libcxx_include_dir` 
respectively.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133973

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


[Lldb-commits] [PATCH] D133906: [lldb] Generate lldb-forward with .def file

2022-09-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

I really wish the .def files would actually generate a header file in the 
output directory. I am not a fan of using code navigation when saying "take me 
to the definition of "class Block;" and it shows:

  #define LLDB_FORWARD_CLASS(Name) class Name;
  #include "lldb-forward.def"
  #undef LLDB_FORWARD_CLASS

Also as Jim pointed out, we now will generate shared, unique, and weak pointers 
for everything, even if those classes  are never used that way.

I am not a fan of this. Wouldn't this also slow down compilation a bit? Each 
file that #include "lldb-forward.h" will not do a large amount of preprocessor 
stuff for each and every file that is compiled that includes this?


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

https://reviews.llvm.org/D133906

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


[Lldb-commits] [PATCH] D133042: Add auto deduce source map setting

2022-09-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In D133042#3791146 , @yinghuitan 
wrote:

> @clayborg , it is my intention to make `target.auto-deduce-source-map` 
> boolean flag ultimately working for both relative paths and two full paths 
> (two full paths are really important for off build host debugging, e.g. dump 
> or production debugging). In this patch, I focuses on getting relative path 
> working because that's the default behavior; a follow-up patch can get two 
> full paths working. 
> In my opinion boolean flag setting is dead simple to use (to enable both) and 
> an enumeration setting would add extra barrier for adoption.

So I think we should have two settings if this is the case. Why? Because I 
think the default values for "target.auto-deduce-source-map" should be true for 
relative paths in debug info, but not true by default for two different full 
paths in the debug info. We have no way to doing a great job at trying to match 
up two different full paths. What if we have "/a/b/c/foo.cpp" and 
"/d/e/f/foo.cpp". If this setting is enabled, I don't expect us to make an auto 
source map between "/a/b/c" -> "/d/e/f". We would need to specify a minimum 
directory depth and that gets really messy and involved more settings, and I 
don't think the full path stuff should be enabled by default.

> I can add description to `target.auto-deduce-source-map` to explain it only 
> works for relative paths.

So not sure if we need to rename this setting to something like 
"target.auto-source-map-relative" to make this clear, and then this can and 
should default to true. Then if we later add full path remapping we can use 
"target.auto-source-map-absolute" and default it to false, and there will need 
to be many other settings for directory depth and other things. I just think 
there is too much that can go wrong with the auto remapping of full paths to 
group them both into the same bucket. I would also rather teach the production 
build toolchains to emit relative debug info and then never have to add the 
really hard auto source maps for different paths. That is unless we expose the 
DW_AT_comp_dir value in lldb_private::CompileUnit and can make a rock solid 
auto path mapping tool from those settings.




Comment at: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp:225
+  const bool case_sensitive = request_file.IsCaseSensitive();
+  const bool full = !request_file.GetDirectory().IsEmpty();
+  for (uint32_t i = 0; i < sc_list.GetSize(); ++i) {

If the directory is empty on the requested file, should we be doing anything 
here? Can we early return?



Comment at: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp:307
 
+  if (GetBreakpoint()->GetTarget().GetAutoDeduceSourceMap())
+DeduceSourceMapping(sc_list);

See my inline comment below, but the BreakpointResolverFileLine is created with 
a NULL for the breakpoint. Does the breakpoint get set properly prior any of 
these calls? I am sure it must?



Comment at: lldb/source/Target/Target.cpp:405
   BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine(
-  nullptr, offset, skip_prologue, location_spec));
+  nullptr, offset, skip_prologue, location_spec, removed_prefix_opt));
   return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);

The breakpoint is initialized with NULL here. Does it get set to something 
valid before we try to use it somehow? I am worried we won't be able to get a 
target from the BreakpointResolver's stored breakpoint



Comment at: lldb/source/Target/TargetProperties.td:40
 Desc<"Source path remappings apply substitutions to the paths of source 
files, typically needed to debug from a different host than the one that built 
the target.  The source-map property consists of an array of pairs, the first 
element is a path prefix, and the second is its replacement.  The syntax is 
`prefix1 replacement1 prefix2 replacement2...`.  The pairs are checked in 
order, the first prefix that matches is used, and that prefix is substituted 
with the replacement.  A common pattern is to use source-map in conjunction 
with the clang -fdebug-prefix-map flag.  In the build, use 
`-fdebug-prefix-map=/path/to/build_dir=.` to rewrite the host specific build 
directory to `.`.  Then for debugging, use `settings set target.source-map . 
/path/to/local_dir` to convert `.` to a valid local path.">;
+  def AutoDeduceSourceMap: Property<"auto-deduce-source-map", "Boolean">,
+DefaultFalse,





Comment at: lldb/source/Target/TargetProperties.td:41
+  def AutoDeduceSourceMap: Property<"auto-deduce-source-map", "Boolean">,
+DefaultFalse,
+Desc<"Automatically deduce source path mappings based on source file 
breakpoint resolution. It only deduces source mapping if source file breakpoint 
request is using full path.">;

[Lldb-commits] [PATCH] D133790: Fix heap-use-after-free when clearing DIEs in fission compile units.

2022-09-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Thanks for following up on this and fixing the issue!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133790

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


[Lldb-commits] [PATCH] D133042: Add auto deduce source map setting

2022-09-15 Thread jeffrey tan via Phabricator via lldb-commits
yinghuitan added a comment.

@clayborg, now I see you want to make this option true by default while full 
paths one to be optional. Then sure, it makes sense.




Comment at: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp:225
+  const bool case_sensitive = request_file.IsCaseSensitive();
+  const bool full = !request_file.GetDirectory().IsEmpty();
+  for (uint32_t i = 0; i < sc_list.GetSize(); ++i) {

clayborg wrote:
> If the directory is empty on the requested file, should we be doing anything 
> here? Can we early return?
This should be handled by check at line 221, right? If 
`request_file.GetDirectory().IsEmpty()` then `request_file.IsRelative()` should 
be true and early return already?



Comment at: lldb/source/Target/Target.cpp:405
   BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine(
-  nullptr, offset, skip_prologue, location_spec));
+  nullptr, offset, skip_prologue, location_spec, removed_prefix_opt));
   return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);

clayborg wrote:
> The breakpoint is initialized with NULL here. Does it get set to something 
> valid before we try to use it somehow? I am worried we won't be able to get a 
> target from the BreakpointResolver's stored breakpoint
It is initialized as nullptr here but `CreateBreakpoint()` call in next line 
will call `BreakpointResolver::SetBreakpoint()` to initialize it. Actually in 
`BreakpointResolver::GetBreakpoint()` explicitly has an assertion to ensure it 
can't be nullptr so I assume we should be safe here. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133042

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


[Lldb-commits] [PATCH] D133042: Add auto deduce source map setting

2022-09-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp:225
+  const bool case_sensitive = request_file.IsCaseSensitive();
+  const bool full = !request_file.GetDirectory().IsEmpty();
+  for (uint32_t i = 0; i < sc_list.GetSize(); ++i) {

yinghuitan wrote:
> clayborg wrote:
> > If the directory is empty on the requested file, should we be doing 
> > anything here? Can we early return?
> This should be handled by check at line 221, right? If 
> `request_file.GetDirectory().IsEmpty()` then `request_file.IsRelative()` 
> should be true and early return already?
Then you can just set full to true all the time then right?



Comment at: lldb/source/Target/Target.cpp:405
   BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine(
-  nullptr, offset, skip_prologue, location_spec));
+  nullptr, offset, skip_prologue, location_spec, removed_prefix_opt));
   return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);

yinghuitan wrote:
> clayborg wrote:
> > The breakpoint is initialized with NULL here. Does it get set to something 
> > valid before we try to use it somehow? I am worried we won't be able to get 
> > a target from the BreakpointResolver's stored breakpoint
> It is initialized as nullptr here but `CreateBreakpoint()` call in next line 
> will call `BreakpointResolver::SetBreakpoint()` to initialize it. Actually in 
> `BreakpointResolver::GetBreakpoint()` explicitly has an assertion to ensure 
> it can't be nullptr so I assume we should be safe here. 
> 
Sounds good. I looked around the code. Looks like the breakpoint in the 
constructor is used for the "create from StructuredData".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133042

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


[Lldb-commits] [PATCH] D133973: [test] Fix LLDB tests with just-built libcxx when using a target directory.

2022-09-15 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht added a comment.

In D133973#3793398 , @JDevlieghere 
wrote:

> Do we need both `LIBCPP_INCLUDE_DIR` and `LIBCPP_INCLUDE_TARGET_DIR`? I see 
> we pass another `-cxx-isystem` flag, does this result in another search path 
> or does that replace the former? If we only need one, we could set 
> `LIBCPP_INCLUDE_DIR` to `libcxx_include_target_dir` or `libcxx_include_dir` 
> respectively.

We need both. `--libcxx_include_dir` sets the regular `include/c++/v1` dir, 
which contains almost all of libc++ headers. But `__config_site` is separately 
installed to `include/$TRIPLE/c++/v1`, so without it, all the build steps fail 
because it can't find `#include <__config_site>`. And `include/$TRIPLE/c++/v1` 
only contains the `__config_site` file, so we can't use the target dir alone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133973

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


[Lldb-commits] [PATCH] D133042: Add auto deduce source map setting

2022-09-15 Thread jeffrey tan via Phabricator via lldb-commits
yinghuitan updated this revision to Diff 460578.
yinghuitan added a comment.

Address review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133042

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Target/PathMappingList.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/unittests/Target/PathMappingListTest.cpp

Index: lldb/unittests/Target/PathMappingListTest.cpp
===
--- lldb/unittests/Target/PathMappingListTest.cpp
+++ lldb/unittests/Target/PathMappingListTest.cpp
@@ -40,7 +40,7 @@
 map.RemapPath(ConstString(match.original.GetPath()), actual_remapped));
 EXPECT_EQ(FileSpec(actual_remapped.GetStringRef()), match.remapped);
 FileSpec unmapped_spec;
-EXPECT_TRUE(map.ReverseRemapPath(match.remapped, unmapped_spec));
+EXPECT_TRUE(map.ReverseRemapPath(match.remapped, unmapped_spec).hasValue());
 std::string unmapped_path = unmapped_spec.GetPath();
 EXPECT_EQ(unmapped_path, orig_normalized);
   }
Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
--- lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -8,6 +8,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+import json
 import os
 import side_effect
 
@@ -97,6 +98,8 @@
 "/x/main.cpp",
 "./x/y/a/d/c/main.cpp",
 ]
+# Reset source map.
+self.runCmd("settings clear target.source-map")
 for path in invalid_paths:
 bkpt = target.BreakpointCreateByLocation(path, 2)
 self.assertTrue(bkpt.GetNumLocations() == 0,
@@ -428,3 +431,68 @@
 
 bp_3 = target.FindBreakpointByID(bp_id_3)
 self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
+
+
+def get_source_map_json(self):
+stream = lldb.SBStream()
+self.dbg.GetSetting("target.source-map").GetAsJSON(stream)
+return json.loads(stream.GetData())
+
+def verify_source_map_entry_pair(self, entry, original, replacement):
+self.assertEquals(entry[0], original,
+"source map entry 'original' does not match")
+self.assertEquals(entry[1], replacement,
+"source map entry 'replacement' does not match")
+
+@skipIf(oslist=["windows"])
+@no_debug_info_test
+def test_breakpoints_auto_source_map_relative(self):
+"""
+Test that with target.auto-source-map-relative settings.
+
+The "relative.yaml" contains a line table that is:
+
+Line table for a/b/c/main.cpp in `a.out
+0x00013f94: a/b/c/main.cpp:1
+0x00013fb0: a/b/c/main.cpp:2:3
+0x00013fb8: a/b/c/main.cpp:2:3
+"""
+src_dir = self.getSourceDir()
+yaml_path = os.path.join(src_dir, "relative.yaml")
+yaml_base, ext = os.path.splitext(yaml_path)
+obj_path = self.getBuildArtifact("a.out")
+self.yaml2obj(yaml_path, obj_path)
+
+# Create a target with the object file we just created from YAML
+target = self.dbg.CreateTarget(obj_path)
+# We now have debug information with line table paths that start are
+# "./a/b/c/main.cpp".
+
+source_map_json = self.get_source_map_json()
+self.assertEquals(len(source_map_json), 0, "source map should be empty initially")
+
+# Verify auto deduced source map when file path in debug info
+# is a suffix of request breakpoint file path
+path = "/x/y/a/b/c/main.cpp"
+bp = target.BreakpointCreateByLocation(path, 2)
+self.assertTrue(bp.GetNumLocations() > 0,
+'Couldn\'t resolve breakpoint using full path "%s" in executate "%s" with '
+'debug info that has relative path with matching suffix' % (path, self.getBuildArtifact("a.out")))
+
+source_map_json = self.get_source_map_json()
+self.assertEquals(len(source_map_json), 1, "source map should not be empty")
+self.verify_source_map_entry_pair(source_map_json[0], ".", "/x/y")
+
+# Reset source map.
+self.runCmd("settings clear target.source-map")
+
+# Verify source map will not auto deduced when file path of request breakpoint
+# equals the file path in debug info.
+path = "a/b/c/main.cpp"
+bp = target.BreakpointCreateByLoca

[Lldb-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

2022-09-15 Thread Matheus Izvekov via Phabricator via lldb-commits
mizvekov updated this revision to Diff 460587.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

Files:
  clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/compound-assign-overflow.c
  clang/test/Sema/complex-int.c
  clang/test/Sema/matrix-type-operators.c
  clang/test/Sema/nullability.c
  clang/test/Sema/sugar-common-types.c
  clang/test/SemaCXX/complex-conversion.cpp
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaCXX/sugar-common-types.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  clang/test/SemaObjC/format-strings-objc.m
  compiler-rt/test/ubsan/TestCases/Integer/add-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/no-recover.cpp
  compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
  libcxx/DELETE.ME
  lldb/test/API/commands/expression/rdar42038760/main.c
  lldb/test/API/commands/expression/rdar44436068/main.c

Index: lldb/test/API/commands/expression/rdar44436068/main.c
===
--- lldb/test/API/commands/expression/rdar44436068/main.c
+++ lldb/test/API/commands/expression/rdar44436068/main.c
@@ -3,6 +3,6 @@
 __int128_t n = 1;
 n = n + n;
 return n; //%self.expect("p n", substrs=['(__int128_t) $0 = 2'])
-  //%self.expect("p n + 6", substrs=['(__int128) $1 = 8'])
-  //%self.expect("p n + n", substrs=['(__int128) $2 = 4'])
+  //%self.expect("p n + 6", substrs=['(__int128_t) $1 = 8'])
+  //%self.expect("p n + n", substrs=['(__int128_t) $2 = 4'])
 }
Index: lldb/test/API/commands/expression/rdar42038760/main.c
===
--- lldb/test/API/commands/expression/rdar42038760/main.c
+++ lldb/test/API/commands/expression/rdar42038760/main.c
@@ -10,7 +10,7 @@
   struct S0 l_19;
   l_19.f2 = 419;
   uint32_t l_4037 = 4294967295UL;
-  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", substrs=['(unsigned int) $0 = 358717883'])
+  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", substrs=['(uint32_t) $0 = 358717883'])
 }
 int main()
 {
Index: libcxx/DELETE.ME
===
--- /dev/null
+++ libcxx/DELETE.ME
@@ -0,0 +1 @@
+D111509
Index: compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
@@ -12,12 +12,12 @@
 
 #ifdef SUB_I32
   (void)(uint32_t(1) - uint32_t(2));
-  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type 'unsigned int'
+  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 #endif
 
 #ifdef SUB_I64
   (void)(uint64_t(800ll) - uint64_t(900ll));
-  // CHECK-SUB_I64: 800 - 900 cannot be represented in type 'unsigned {{long( long)?}}'
+  // CHECK-SUB_I64: 800 - 900 cannot be represented in type '{{uint64_t|unsigned long( long)?}}'
 #endif
 
 #ifdef SUB_I128
@@ -26,6 +26,6 @@
 # else
   puts("__int128 not supported\n");
 # endif
-  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type 'unsigned __int128'|__int128 not supported}}
+  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type '__uint128_t'|__int128 not supported}}
 #endif
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
@@ -13,7 +13,7 @@
   (void)(uint16_t(0x) * uint16_t(0x8001));
 
   (void)(uint32_t(0x) * uint32_t(0x2));
-  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type 'unsigned int'
+  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 
   return 0;
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
=

[Lldb-commits] [PATCH] D133944: [lldb][tests][gmodules] Test for expression evaluator crash for types with template base class

2022-09-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 460615.
Michael137 added a comment.

- Add check on structure of AST in API test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133944

Files:
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/Makefile
  
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.cpp
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.h
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/main.cpp
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module.modulemap
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.cpp
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.h
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.cpp
  lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.h

Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.h
@@ -0,0 +1,10 @@
+#ifndef MOD2_H_IN
+#define MOD2_H_IN
+
+#include "base_module.h"
+
+struct ClassInMod2 {
+  ClassInMod3 VecInMod2;
+};
+
+#endif
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module2.cpp
@@ -0,0 +1,3 @@
+#include "module2.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.h
@@ -0,0 +1,10 @@
+#ifndef MOD1_H_IN
+#define MOD1_H_IN
+
+#include "base_module.h"
+
+struct ClassInMod1 {
+  ClassInMod3 VecInMod1;
+};
+
+#endif // _H_IN
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module1.cpp
@@ -0,0 +1,3 @@
+#include "module1.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module.modulemap
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/module.modulemap
@@ -0,0 +1,14 @@
+module Module1 {
+  header "module1.h"
+  export *
+}
+
+module Module2 {
+  header "module2.h"
+  export *
+}
+
+module BaseModule {
+  header "base_module.h"
+  export *
+}
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/main.cpp
@@ -0,0 +1,15 @@
+#include "module1.h"
+#include "module2.h"
+
+#include 
+
+int main() {
+  ClassInMod1 FromMod1;
+  ClassInMod2 FromMod2;
+
+  FromMod1.VecInMod1.BaseMember = 137;
+  FromMod2.VecInMod2.BaseMember = 42;
+
+  std::puts("Break here");
+  return 0;
+}
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.h
@@ -0,0 +1,8 @@
+#ifndef MOD3_H_IN
+#define MOD3_H_IN
+
+template  struct ClassInMod3Base { int BaseMember = 0; };
+
+template  struct ClassInMod3 : public ClassInMod3Base {};
+
+#endif // _H_IN
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/base_module.cpp
@@ -0,0 +1,3 @@
+#include "base_module.h"
+
+namespace crash {} // namespace crash
Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -0,0 +1,71 @@
+"""
+Tests the scenario where we evaluate expressions
+of two types in different modules that reference
+a base class template instantiated with the same
+template argument.
+
+Note that,
+1. Since the decls originate from modules, LLDB
+   marks them as such and Clang doesn't create
+   a LookupPtr map on the corresponding DeclContext.
+   This prevents regular DeclContext::lookup from
+   succeeding.
+2. Because we reference the same base template
+   from two different modules we get a redeclaration
+   chain for the base class's ClassTemplateSpecializationDecl

[Lldb-commits] [PATCH] D132300: [clang][lldb][cmake] Use new `*_INSTALL_LIBDIR_BASENAME` CPP macro

2022-09-15 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.

In the long term we should just remove the `CLANG_INSTALL_LIBDIR_BASENAME` 
customization. This is supposed for GCC multilib lib32 lib64 names but we don't 
necessarily use it for Clang + compiler-rt files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132300

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


[Lldb-commits] [PATCH] D134011: [lldb] Fix parentheses placement in GetExpressionPath

2022-09-15 Thread Dave Lee via Phabricator via lldb-commits
kastiglione created this revision.
kastiglione added a reviewer: jingham.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Adjust the placement of parentheses around a pointer dereference in
`GetExpressionPath`.

The comment states:

> `*(a_ptr).memberName`, which is entirely fine

but the expression should be: `(*a_ptr).memberName`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134011

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/python_api/exprpath_synthetic/main.mm


Index: lldb/test/API/python_api/exprpath_synthetic/main.mm
===
--- lldb/test/API/python_api/exprpath_synthetic/main.mm
+++ lldb/test/API/python_api/exprpath_synthetic/main.mm
@@ -5,8 +5,11 @@
 {
 std::vector v{1,2,3,4,5};
 NSArray *a = @[@"Hello",@"World",@"From Me"];
+int *p;
 return 0; //% v = self.frame().FindVariable("v"); v0 = 
v.GetChildAtIndex(0); s = lldb.SBStream(); v0.GetExpressionPath(s);
 //% self.runCmd("expr %s = 12" % s.GetData()); 
self.assertTrue(v0.GetValueAsUnsigned() == 12, "value change via expr failed")
 //% a = self.frame().FindVariable("a"); a1 = a.GetChildAtIndex(1); s = 
lldb.SBStream(); a1.GetExpressionPath(s);
 //% self.expect("po %s" % s.GetData(), substrs = ["World"])
+//% p = self.frame().GetValueForVariablePath("*p")
+//% self.assertEqual(p.path, "(*p)")
 }
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -1929,11 +1929,11 @@
   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
+// (*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.PutCString("*(");
+s.PutCString("(*");
   }
 
   ValueObject *parent = GetParent();


Index: lldb/test/API/python_api/exprpath_synthetic/main.mm
===
--- lldb/test/API/python_api/exprpath_synthetic/main.mm
+++ lldb/test/API/python_api/exprpath_synthetic/main.mm
@@ -5,8 +5,11 @@
 {
 std::vector v{1,2,3,4,5};
 NSArray *a = @[@"Hello",@"World",@"From Me"];
+int *p;
 return 0; //% v = self.frame().FindVariable("v"); v0 = v.GetChildAtIndex(0); s = lldb.SBStream(); v0.GetExpressionPath(s);
 //% self.runCmd("expr %s = 12" % s.GetData()); self.assertTrue(v0.GetValueAsUnsigned() == 12, "value change via expr failed")
 //% a = self.frame().FindVariable("a"); a1 = a.GetChildAtIndex(1); s = lldb.SBStream(); a1.GetExpressionPath(s);
 //% self.expect("po %s" % s.GetData(), substrs = ["World"])
+//% p = self.frame().GetValueForVariablePath("*p")
+//% self.assertEqual(p.path, "(*p)")
 }
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -1929,11 +1929,11 @@
   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
+// (*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.PutCString("*(");
+s.PutCString("(*");
   }
 
   ValueObject *parent = GetParent();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D134011: [lldb] Fix parentheses placement in GetExpressionPath

2022-09-15 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment.

I noticed this when running `v *SomeVar` and the variable contained a 
`llvm::PointerUnion`. The synthetic provider for `PointerUnion` uses 
`GetExpressionPath`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134011

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


[Lldb-commits] [lldb] ab755e6 - [lldb] Improve formatting of skipped categories message (NFC)

2022-09-15 Thread Dave Lee via lldb-commits

Author: Dave Lee
Date: 2022-09-15T20:39:25-07:00
New Revision: ab755e65629ea098cb6faa77b13ac087849ffc67

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

LOG: [lldb] Improve formatting of skipped categories message (NFC)

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/dotest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 37842c96ba388..b4b1a365bbdfe 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -959,7 +959,8 @@ def run_suite():
 # Note that it's not dotest's job to clean this directory.
 lldbutil.mkdir_p(configuration.test_build_dir)
 
-print("Skipping the following test categories: 
{}".format(configuration.skip_categories))
+skipped_categories_list = ", ".join(configuration.skip_categories)
+print("Skipping the following test categories: 
{}".format(skipped_categories_list))
 
 for testdir in configuration.testdirs:
 for (dirpath, dirnames, filenames) in os.walk(testdir):



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


[Lldb-commits] [lldb] 8704281 - [LLDB][NativePDB] Global ctor and dtor should be global decls.

2022-09-15 Thread Zequan Wu via lldb-commits

Author: Zequan Wu
Date: 2022-09-15T22:35:58-07:00
New Revision: 8704281c567705822a1c23b9ec40f5bdc5d58352

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

LOG: [LLDB][NativePDB] Global ctor and dtor should be global decls.

This fixes a crash that mistaken global ctor/dtor as funciton methods.

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

Added: 
lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp

Modified: 
lldb/source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.cpp
index 8ecf6712eace2..72afc6fe75acc 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.cpp
@@ -11,6 +11,13 @@
 #include 
 
 MSVCUndecoratedNameParser::MSVCUndecoratedNameParser(llvm::StringRef name) {
+  // Global ctor and dtor are global functions.
+  if (name.contains("dynamic initializer for") ||
+  name.contains("dynamic atexit destructor for")) {
+m_specifiers.emplace_back(name, name);
+return;
+  }
+
   std::size_t last_base_start = 0;
 
   std::stack stack;

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 1be80e31c7fcc..ab467f20990c4 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -537,7 +537,7 @@ 
PdbAstBuilder::CreateDeclInfoForUndecoratedName(llvm::StringRef name) {
   MSVCUndecoratedNameParser parser(name);
   llvm::ArrayRef specs = parser.GetSpecifiers();
 
-  auto context = FromCompilerDeclContext(GetTranslationUnitDecl());
+  auto *context = FromCompilerDeclContext(GetTranslationUnitDecl());
 
   llvm::StringRef uname = specs.back().GetBaseName();
   specs = specs.drop_back();
@@ -1226,7 +1226,6 @@ PdbAstBuilder::GetOrCreateFunctionDecl(PdbCompilandSymId 
func_id) {
   llvm::StringRef proc_name = proc.Name;
   proc_name.consume_front(context_name);
   proc_name.consume_front("::");
-
   clang::FunctionDecl *function_decl =
   CreateFunctionDecl(func_id, proc_name, proc.FunctionType, func_ct,
  func_type->getNumParams(), storage, false, parent);

diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp 
b/lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
new file mode 100644
index 0..15b4d330fabb0
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
@@ -0,0 +1,30 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Global ctor and dtor should be globals decls.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -GS- -fno-addrsig -c 
/Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb -force
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb-test symbols --dump-ast %t.exe | 
FileCheck %s
+
+struct A {
+  ~A() {};
+};
+struct B {
+  static A glob;
+};
+
+A B::glob = A();
+int main() {
+  return 0;
+}
+
+// CHECK:  static void B::`dynamic initializer for 'glob'();
+// CHECK-NEXT: static void B::`dynamic atexit destructor for 'glob'();
+// CHECK-NEXT: int main();
+// CHECK-NEXT: static void _GLOBAL__sub_I_global_ctor_dtor.cpp();
+// CHECK-NEXT: struct A {
+// CHECK-NEXT: ~A();
+// CHECK-NEXT: };
+// CHECK-NEXT: struct B {
+// CHECK-NEXT: static A glob;
+// CHECK-NEXT: };



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


[Lldb-commits] [PATCH] D133446: [LLDB][NativePDB] Global ctor and dtor should be global decls.

2022-09-15 Thread Zequan Wu via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8704281c5677: [LLDB][NativePDB] Global ctor and dtor should 
be global decls. (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133446

Files:
  lldb/source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp


Index: lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
@@ -0,0 +1,30 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Global ctor and dtor should be globals decls.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -GS- -fno-addrsig -c 
/Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb -force
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb-test symbols --dump-ast %t.exe | 
FileCheck %s
+
+struct A {
+  ~A() {};
+};
+struct B {
+  static A glob;
+};
+
+A B::glob = A();
+int main() {
+  return 0;
+}
+
+// CHECK:  static void B::`dynamic initializer for 'glob'();
+// CHECK-NEXT: static void B::`dynamic atexit destructor for 'glob'();
+// CHECK-NEXT: int main();
+// CHECK-NEXT: static void _GLOBAL__sub_I_global_ctor_dtor.cpp();
+// CHECK-NEXT: struct A {
+// CHECK-NEXT: ~A();
+// CHECK-NEXT: };
+// CHECK-NEXT: struct B {
+// CHECK-NEXT: static A glob;
+// CHECK-NEXT: };
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -537,7 +537,7 @@
   MSVCUndecoratedNameParser parser(name);
   llvm::ArrayRef specs = parser.GetSpecifiers();
 
-  auto context = FromCompilerDeclContext(GetTranslationUnitDecl());
+  auto *context = FromCompilerDeclContext(GetTranslationUnitDecl());
 
   llvm::StringRef uname = specs.back().GetBaseName();
   specs = specs.drop_back();
@@ -1226,7 +1226,6 @@
   llvm::StringRef proc_name = proc.Name;
   proc_name.consume_front(context_name);
   proc_name.consume_front("::");
-
   clang::FunctionDecl *function_decl =
   CreateFunctionDecl(func_id, proc_name, proc.FunctionType, func_ct,
  func_type->getNumParams(), storage, false, parent);
Index: lldb/source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.cpp
@@ -11,6 +11,13 @@
 #include 
 
 MSVCUndecoratedNameParser::MSVCUndecoratedNameParser(llvm::StringRef name) {
+  // Global ctor and dtor are global functions.
+  if (name.contains("dynamic initializer for") ||
+  name.contains("dynamic atexit destructor for")) {
+m_specifiers.emplace_back(name, name);
+return;
+  }
+
   std::size_t last_base_start = 0;
 
   std::stack stack;


Index: lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/global-ctor-dtor.cpp
@@ -0,0 +1,30 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Global ctor and dtor should be globals decls.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -GS- -fno-addrsig -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb -force
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb-test symbols --dump-ast %t.exe | FileCheck %s
+
+struct A {
+  ~A() {};
+};
+struct B {
+  static A glob;
+};
+
+A B::glob = A();
+int main() {
+  return 0;
+}
+
+// CHECK:  static void B::`dynamic initializer for 'glob'();
+// CHECK-NEXT: static void B::`dynamic atexit destructor for 'glob'();
+// CHECK-NEXT: int main();
+// CHECK-NEXT: static void _GLOBAL__sub_I_global_ctor_dtor.cpp();
+// CHECK-NEXT: struct A {
+// CHECK-NEXT: ~A();
+// CHECK-NEXT: };
+// CHECK-NEXT: struct B {
+// CHECK-NEXT: static A glob;
+// CHECK-NEXT: };
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -537,7 +537,7 @@
   MSVCUndecoratedNameParser parser(name);
   llvm::ArrayRef specs = parser.GetSpecifiers();
 
-  auto context = FromCompilerDeclContext(GetTranslationUnitDecl());
+  auto *context = FromCompilerDeclContext(GetTranslationUnitDecl());
 
   llvm::StringRef uname = specs.back().GetBaseName();
   specs = specs.drop_back();
@@ -1226,7 +1226,6 @@
   llvm::St

[Lldb-commits] [PATCH] D133601: [LLDB][NativePDB] ResolveSymbolContext should return the innermost block

2022-09-15 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 460644.
zequanwu added a comment.

Rebase and update test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133601

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s

Index: lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/nested-blocks-same-address.s
@@ -0,0 +1,397 @@
+# clang-format off
+# REQUIRES: lld, x86
+
+# Test when nested S_BLOCK32 have same address range, ResolveSymbolContext should return the innnermost block.
+# RUN: llvm-mc -triple=x86_64-windows-msvc --filetype=obj %s > %t.obj
+# RUN: lld-link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe /base:0x14000
+# RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -o "image lookup -a 0x14000103c -v" -o "exit" | FileCheck %s
+
+# This file is compiled from following source file:
+# $ clang-cl /Z7 /GS- /c /O2 test.cpp /Fatest.s
+# __attribute__((optnone)) bool func(const char* cp, volatile char p[]) {
+#   return false;
+# }
+#
+# int main() {
+#   const char* const kMfDLLs[] = {"a"};
+#   asm("nop");
+#   for (const char* kMfDLL : kMfDLLs) {
+# volatile char path[10] = {0};
+# if (func(kMfDLL, path))
+#   break;
+#   }
+#   return 0;
+# }
+
+# CHECK:   Function: id = {{.*}}, name = "main", range = [0x000140001020-0x00014000104d)
+# CHECK-NEXT:  FuncType: id = {{.*}}, byte-size = 0, compiler_type = "int (void)"
+# CHECK-NEXT:Blocks: id = {{.*}}, range = [0x140001020-0x14000104d)
+# CHECK-NEXT:id = {{.*}}, range = [0x140001025-0x140001046)
+# CHECK-NEXT:id = {{.*}}, range = [0x140001025-0x140001046)
+# CHECK-NEXT:id = {{.*}}, range = [0x140001025-0x140001046)
+# CHECK-NEXT: LineEntry: [0x000140001035-0x000140001046): /tmp/test.cpp:10
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "path", type = "volatile char[10]", valid ranges = , location = [0x000140001025, 0x000140001046) -> DW_OP_breg7 RSP+40, decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "kMfDLL", type = "const char *", valid ranges = , location = [0x00014000103c, 0x000140001046) -> DW_OP_reg2 RCX, decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "__range1", type = "const char *const (&)[1]", valid ranges = , location = , decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "__begin1", type = "const char *const *", valid ranges = , location = , decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "__end1", type = "const char *const *", valid ranges = , location = , decl =
+# CHECK-NEXT:  Variable: id = {{.*}}, name = "kMfDLLs", type = "const char *const[1]", valid ranges = , location = [0x00014000103c, 0x000140001046) -> DW_OP_reg2 RCX, decl =
+
+
+	.text
+	.def	@feat.00;
+	.scl	3;
+	.type	0;
+	.endef
+	.globl	@feat.00
+.set @feat.00, 0
+	.intel_syntax noprefix
+	.file	"test.cpp"
+	.def	"?func@@YA_NPEBDQECD@Z";
+	.scl	2;
+	.type	32;
+	.endef
+	.section	.text,"xr",one_only,"?func@@YA_NPEBDQECD@Z"
+	.globl	"?func@@YA_NPEBDQECD@Z" # -- Begin function ?func@@YA_NPEBDQECD@Z
+	.p2align	4, 0x90
+"?func@@YA_NPEBDQECD@Z":# @"?func@@YA_NPEBDQECD@Z"
+.Lfunc_begin0:
+	.cv_func_id 0
+	.cv_file	1 "/tmp/test.cpp" "8CDAA03EE93954606427F9B409CE7638" 1
+	.cv_loc	0 1 1 0 # test.cpp:1:0
+.seh_proc "?func@@YA_NPEBDQECD@Z"
+# %bb.0:# %entry
+	sub	rsp, 16
+	.seh_stackalloc 16
+	.seh_endprologue
+	mov	qword ptr [rsp + 8], rdx
+	mov	qword ptr [rsp], rcx
+.Ltmp0:
+	.cv_loc	0 1 2 0 # test.cpp:2:0
+	xor	eax, eax
+	and	al, 1
+	movzx	eax, al
+	add	rsp, 16
+	ret
+.Ltmp1:
+.Lfunc_end0:
+	.seh_endproc
+# -- End function
+	.def	main;
+	.scl	2;
+	.type	32;
+	.endef
+	.section	.text,"xr",one_only,main
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+main:   # @main
+.Lfunc_begin1:
+	.cv_func_id 1
+	.cv_loc	1 1 5 0 # test.cpp:5:0
+.seh_proc main
+# %bb.0:# %entry
+	sub	rsp, 56
+	.seh_stackalloc 56
+	.seh_endprologue
+.Ltmp2:
+	.cv_loc	1 1 7 0 # test.cpp:7:0
+	#APP
+	nop
+	#NO_APP
+.Ltmp3:
+	#DEBUG_VALUE: __range1 <- undef
+	#DEBUG_VALUE: __begin1 <- undef
+	#DEBUG_VALUE: __end1 <- [DW_OP_plus_uconst 8, DW_OP_stack_value] undef
+	.cv_loc	1 1 9 0 # test.cpp:9:0
+	mov	word ptr [rsp + 48], 0
+	mov	qword ptr [rsp + 40], 0
+	.cv_loc	1 1 10 0# test.cpp:10:0
+	lea	rcx, [rip + "??_C@_01MCMALHOG@a?$AA@"]
+.Ltmp4:
+	#DEBUG_VALUE: main:kMfDLLs <- $rcx
+	#DEBUG_VALUE: kMfDLL <- $rcx
+	lea	rdx, [rsp + 40]
+	call	"?func@@YA_NPEBDQECD@Z"
+.Ltmp5