[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D159127#4655450 , @aprantl wrote:

> @Mordante @Michael137  This seems to fail on older versions of 
> compiler/libcxx 
> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-matrix/7247/ You 
> can probably fix this by just requiring a minimum clang version in the tests

https://github.com/llvm/llvm-project/pull/70544


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159127

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

[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

[Michael Buch via Phabricator via lldb-commits]

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

Was looking at leftover reviews, would be nice to land this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159127

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Also, I assume the extra changes to make the PointerIntPair formatter work will 
be in a follow-up patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D156774#4644503 , @Endill wrote:

> Ping @Michael137

Sorry for the delay, just came back from vacation

The change itself LGTM. Can we add a test though? We do have 
DWARFASTParserClang unittests: 
https://github.com/llvm/llvm-project/blob/main/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp

If the yaml input file becomes tedious to write you could also just an API 
test. E.g., under 
https://github.com/llvm/llvm-project/tree/main/lldb/test/API/lang/cpp

Let me know if you need some pointers in writing the tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D156774#4601767 , @Endill wrote:

> In D156774#4601736 , @Michael137 
> wrote:
>
>> What were your lldb commands when you tested this?
>
> `script import lldb; frame = lldb.thread.GetFrameAtIndex(0); 
> print(frame.variables[0].type.GetNumberOfMemberEnums())`
>
>> LLDB currently completes types lazily when it thinks it can. Does your new 
>> API still fail if you run `expr p` prior? (the idea is that that would 
>> trigger completion of the type and parse dwarf). If we dont ever call 
>> `GetFullCompilerType` on your type LLDB will never try to pull in the 
>> definition
>
> No amount of tinkering with `expr` makes `script 
> print(frame.variables[0].type.GetNumberOfMemberEnums())` output a non-zero 
> value.
> I tested this with the changes I uploaded here half an hour ago.



In D156774#4601767 , @Endill wrote:

> In D156774#4601736 , @Michael137 
> wrote:
>
>> What were your lldb commands when you tested this?
>
> `script import lldb; frame = lldb.thread.GetFrameAtIndex(0); 
> print(frame.variables[0].type.GetNumberOfMemberEnums())`
>
>> LLDB currently completes types lazily when it thinks it can. Does your new 
>> API still fail if you run `expr p` prior? (the idea is that that would 
>> trigger completion of the type and parse dwarf). If we dont ever call 
>> `GetFullCompilerType` on your type LLDB will never try to pull in the 
>> definition
>
> No amount of tinkering with `expr` makes `script 
> print(frame.variables[0].type.GetNumberOfMemberEnums())` output a non-zero 
> value.
> I tested this with the changes I uploaded here half an hour ago.

I think you may want to use `GetCompleteQualType` before iterating the 
DeclContext. That will make sure we complete the type by the time we look for 
the enums.

E.g.,:

  clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));  
 
  if (GetCompleteQualType(&getASTContext(), qual_type)) {   
  
const clang::RecordType *record_type =  
   
llvm::cast(qual_type.getTypePtr());  
  
const clang::RecordDecl *record_decl = record_type->getDecl();  
   
assert(record_decl);
   
return std::distance(EnumIt(record_decl->decls_begin()), 
EnumIt(record_decl->decls_end())); 
  } 
   

Will review it more thoroughly on tomorrow though


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D156774#4601705 , @Endill wrote:

> I tested this patch together with the following new code:
>
>   uint32_t TypeSystemClang::GetNumMemberEnums(lldb::opaque_compiler_type_t 
> type) {
> using EnumIt = 
> clang::DeclContext::specific_decl_iterator;
> if (!type)
>   return 0;
> clang::QualType qual_type = 
> RemoveWrappingTypes(GetCanonicalQualType(type));
> clang::DeclContext *ctx = qual_type->getAsRecordDecl()->getDeclContext();
> return std::distance(EnumIt(ctx->decls_begin()), 
> EnumIt(ctx->decls_end()));
>   }
>   
>   CompilerType
>   TypeSystemClang::GetMemberEnumAtIndex(lldb::opaque_compiler_type_t type,
> size_t index) {
> using EnumIt = 
> clang::DeclContext::specific_decl_iterator;
> if (!type)
>   return CompilerType();
>   
> clang::QualType qual_type = 
> RemoveWrappingTypes(GetCanonicalQualType(type));
> clang::DeclContext *ctx = qual_type->getAsRecordDecl()->getDeclContext();
> size_t enum_index = 0;
> for (EnumIt enums_it(ctx->decls_begin()), enums_end(ctx->decls_end());
>  enums_it != enums_end;
>  ++enums_it, ++enum_index) {
> if (enum_index == index) {
>   return CompilerType(weak_from_this(), *enums_it);
> }
> }
>   }
>
> I created all the wrappers to make it available in Python. The result was 
> unsatisfactory: this code doesn't even trigger 
> `DWARFASTParserClang::ParseChildMembers` that this patch touches, and return 
> 0 instead of 1. This doesn't change even if I trigger `ParseChildMembers` via 
> other means before asking for a number of member enums in a type.
>
> Code I tested this on:
>
>   using intptr_t = long;
>   using uintptr_t = unsigned long;
>   
>   struct PointerIntPairInfo {
> enum MaskAndShiftConstants : uintptr_t {
>   PointerBitMask =
>   ~(uintptr_t)(((intptr_t)1 << 3) - 1),
> };
>   
> int a{};
>   };
>   
>   static uintptr_t dummy() {
> return PointerIntPairInfo::PointerBitMask;
>   }
>   
>   int main()
>   {
>   PointerIntPairInfo p;
>   __builtin_debugtrap();
>   return p.a + foo();
>   }
>
> If you have any suggestions what I missed or did wrong, please let me know.
>
> I'll continue with this patch nevertheless, but it's clear now that there's 
> still a way to go until I can access that enum without going through slow 
> expression evaluator.

What were your lldb commands when you tested this?

LLDB currently completes types lazily when it thinks it can. Does your new API 
still fail if you run `expr p` prior? (the idea is that that would trigger 
completion of the type and parse dwarf). If we dont ever call 
`GetFullCompilerType` on your type LLDB will never try to pull in the definition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D158172: [lldb][ClangASTImporter][NFC] Remove redundant calls to ASTImporter::Imported

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c3f1f42cbed: [lldb][ClangASTImporter][NFC] Remove redundant 
calls to ASTImporter::Imported (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158172

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158172: [lldb][ClangASTImporter][NFC] Remove redundant calls to ASTImporter::Imported

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 551086.
Michael137 added a comment.

- Reword commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158172

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158172: [lldb][ClangASTImporter][NFC] Remove redundant calls to ASTImporter::Imported

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a subscriber: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The `ASTImporter::Imported` base method has been made a no-op in
`26f72a96559f2acd6799c363f1ca88ef3238c601`. So all calls to it from
a base-class are now redundant. The API is now only used to notify
subclasses that an import occurred and not for any other bookkeeping.
That is done in `MapImported` which we call properly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158172

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157992: [lldb][CPlusPlus][NFCI] Remove redundant construction of ClangASTImporter

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG92d7254a989d: [lldb][CPlusPlus][NFCI] Remove redundant 
construction of ClangASTImporter (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157992

Files:
  lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
@@ -54,18 +54,6 @@
 if (!clang_ast_context)
   return;
 
-std::shared_ptr clang_ast_importer;
-auto *state = target_sp->GetPersistentExpressionStateForLanguage(
-lldb::eLanguageTypeC_plus_plus);
-if (state) {
-  auto *persistent_vars = llvm::cast(state);
-  clang_ast_importer = persistent_vars->GetClangASTImporter();
-}
-
-if (!clang_ast_importer) {
-  return;
-}
-
 const char *const isa_name("__isa");
 const CompilerType isa_type =
 clang_ast_context->GetBasicType(lldb::eBasicTypeObjCClass);


Index: lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
@@ -54,18 +54,6 @@
 if (!clang_ast_context)
   return;
 
-std::shared_ptr clang_ast_importer;
-auto *state = target_sp->GetPersistentExpressionStateForLanguage(
-lldb::eLanguageTypeC_plus_plus);
-if (state) {
-  auto *persistent_vars = llvm::cast(state);
-  clang_ast_importer = persistent_vars->GetClangASTImporter();
-}
-
-if (!clang_ast_importer) {
-  return;
-}
-
 const char *const isa_name("__isa");
 const CompilerType isa_type =
 clang_ast_context->GetBasicType(lldb::eBasicTypeObjCClass);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157992: [lldb][CPlusPlus][NFCI] Remove redundant construction of ClangASTImporter

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a subscriber: martong.
Herald added a reviewer: a.sidorin.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The usage of this variable was removed in 
`4f14c17df70916913d71914343dd4f6c709e218d`.

This is no longer used inside this file. Since the call to
`GetPersistentExpressionStateForLanguage` has side-effects I marked this
NFCI. But there is no good reason we need this here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157992

Files:
  lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
@@ -54,18 +54,6 @@
 if (!clang_ast_context)
   return;
 
-std::shared_ptr clang_ast_importer;
-auto *state = target_sp->GetPersistentExpressionStateForLanguage(
-lldb::eLanguageTypeC_plus_plus);
-if (state) {
-  auto *persistent_vars = llvm::cast(state);
-  clang_ast_importer = persistent_vars->GetClangASTImporter();
-}
-
-if (!clang_ast_importer) {
-  return;
-}
-
 const char *const isa_name("__isa");
 const CompilerType isa_type =
 clang_ast_context->GetBasicType(lldb::eBasicTypeObjCClass);


Index: lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
@@ -54,18 +54,6 @@
 if (!clang_ast_context)
   return;
 
-std::shared_ptr clang_ast_importer;
-auto *state = target_sp->GetPersistentExpressionStateForLanguage(
-lldb::eLanguageTypeC_plus_plus);
-if (state) {
-  auto *persistent_vars = llvm::cast(state);
-  clang_ast_importer = persistent_vars->GetClangASTImporter();
-}
-
-if (!clang_ast_importer) {
-  return;
-}
-
 const char *const isa_name("__isa");
 const CompilerType isa_type =
 clang_ast_context->GetBasicType(lldb::eBasicTypeObjCClass);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157636: [lldb][test] Remove tests relying on deprecated std::char_traits specializations

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG487ab39a5082: [lldb][test] Remove tests relying on 
deprecated std::char_traits specializations (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157636

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -9,7 +9,6 @@
 std::string empty("");
 std::string q("hello world");
 std::string Q("quite a long std::strin with lots of info inside it");
-std::basic_string uchar(5, 'a');
 auto &rq = q, &rQ = Q;
 std::string *pq = &q, *pQ = &Q;
 S.assign(L"!"); // Set break point at this line.
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -61,8 +61,6 @@
 var_pq = self.frame().FindVariable("pq")
 var_pQ = self.frame().FindVariable("pQ")
 
-var_uchar = self.frame().FindVariable("uchar")
-
 self.assertEqual(var_wempty.GetSummary(), 'L""', "wempty summary wrong")
 self.assertEqual(
 var_s.GetSummary(), 'L"hello world! מזל טוב!"', "s summary wrong"
@@ -78,7 +76,6 @@
 '"quite a long std::strin with lots of info inside it"',
 "Q summary wrong",
 )
-self.assertEqual(var_uchar.GetSummary(), '"a"', "u summary wrong")
 self.assertEqual(var_rq.GetSummary(), '"hello world"', "rq summary wrong")
 self.assertEqual(
 var_rQ.GetSummary(),
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
@@ -92,8 +92,6 @@
   std::u16string_view u16_empty(u"");
   std::u32string_view u32_string(U"🍄🍅🍆🍌");
   std::u32string_view u32_empty(U"");
-  std::basic_string uchar_source(10, 'a');
-  std::basic_string_view uchar(uchar_source.data(), 5);
   std::string_view *null_str = nullptr;
 
   std::string hello = "Hellooo ";
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
@@ -60,15 +60,6 @@
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
 
-if self.expectedCompiler(["clang"]) and self.expectedCompilerVersion(
-[">", "16.0"]
-):
-expected_basic_string = "std::basic_string"
-expected_basic_string_view = "std::basic_string_view"
-else:
-expected_basic_string = "std::basic_string, std::allocator >"
-expected_basic_string_view = "std::basic_string_view >"
-
 self.expect_var_path("wempty", type="std::wstring_view", summary='L""')
 self.expect_var_path(
 "s", type="std::wstring_view", summary='L"hello world! מזל טוב!"'
@@ -96,12 +87,6 @@
 "u32_string", type="std::u32string_view", summary='U"🍄🍅🍆🍌"'
 )
 self.expect_var_path("u32_empty", type="std::u32string_view", summary='""')
-self.expect_var_path(
-"uchar_source", type=expected_basic_string, summary='"aa"'
-)
-se

[Lldb-commits] [PATCH] D157636: [lldb][test] Remove tests relying on deprecated std::char_traits specializations

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added reviewers: gribozavr2, JDevlieghere.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

(motivated by test failures after D157058 )

With D157058  the base template for 
`std::char_traits` was removed from
libc++. Quoting the release notes:

  The base template for ``std::char_traits`` has been removed. If you are using
  ``std::char_traits`` with types other than ``char``, ``wchar_t``, ``char8_t``,
  ``char16_t``, ``char32_t`` or a custom character type for which you
  specialized ``std::char_traits``, your code will no longer work.

This patch simply removes all such instantiations to make sure the
tests that run against the latest libc++ version pass.

One could try testing the existence of this base template from within
the test source files but this doesn't seem like something we want
support.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157636

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -9,7 +9,6 @@
 std::string empty("");
 std::string q("hello world");
 std::string Q("quite a long std::strin with lots of info inside it");
-std::basic_string uchar(5, 'a');
 auto &rq = q, &rQ = Q;
 std::string *pq = &q, *pQ = &Q;
 S.assign(L"!"); // Set break point at this line.
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -61,8 +61,6 @@
 var_pq = self.frame().FindVariable("pq")
 var_pQ = self.frame().FindVariable("pQ")
 
-var_uchar = self.frame().FindVariable("uchar")
-
 self.assertEqual(var_wempty.GetSummary(), 'L""', "wempty summary wrong")
 self.assertEqual(
 var_s.GetSummary(), 'L"hello world! מזל טוב!"', "s summary wrong"
@@ -78,7 +76,6 @@
 '"quite a long std::strin with lots of info inside it"',
 "Q summary wrong",
 )
-self.assertEqual(var_uchar.GetSummary(), '"a"', "u summary wrong")
 self.assertEqual(var_rq.GetSummary(), '"hello world"', "rq summary wrong")
 self.assertEqual(
 var_rQ.GetSummary(),
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
@@ -92,8 +92,6 @@
   std::u16string_view u16_empty(u"");
   std::u32string_view u32_string(U"🍄🍅🍆🍌");
   std::u32string_view u32_empty(U"");
-  std::basic_string uchar_source(10, 'a');
-  std::basic_string_view uchar(uchar_source.data(), 5);
   std::string_view *null_str = nullptr;
 
   std::string hello = "Hellooo ";
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
@@ -60,15 +60,6 @@
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
 
-if self.expectedCompiler(["clang"]) and self.expectedCompilerVersion(
-[">", "16.0"]
-):
-expected_basic_string = "std::basic_string"
-e

[Lldb-commits] [PATCH] D157512: [lldb][PlatformDarwin] Only parse SDK from debug-info if target language is Objective-C

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a subscriber: kadircet.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added subscribers: lldb-commits, ilya-biryukov.
Herald added a project: LLDB.

The call to `HostInfoMacOSX::GetSDKRoot` can noticeably slow down the first
expression evaluation in an LLDB session. For C++ expressions we don't
actually ever use the results of the `ClangDeclVendor`, so getting the
flags right isn't necessary. Thus skip SDK parsing logic for C++
targets.

An alternative would be to entirely omit the setup of the
ClangDeclVendor (and other Objective-C machinery) for C++ targets.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157512

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Breakpoint/BreakpointSite.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
@@ -1097,7 +1098,7 @@
 
   FileSpec sysroot_spec;
 
-  if (target) {
+  if (target && Language::LanguageIsObjC(target->GetLanguage())) {
 if (ModuleSP exe_module_sp = target->GetExecutableModule()) {
   auto path_or_err = ResolveSDKPathFromDebugInfo(*exe_module_sp);
   if (path_or_err) {


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Breakpoint/BreakpointSite.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
@@ -1097,7 +1098,7 @@
 
   FileSpec sysroot_spec;
 
-  if (target) {
+  if (target && Language::LanguageIsObjC(target->GetLanguage())) {
 if (ModuleSP exe_module_sp = target->GetExecutableModule()) {
   auto path_or_err = ResolveSDKPathFromDebugInfo(*exe_module_sp);
   if (path_or_err) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D156774#4572967 , @Endill wrote:

> In D156774#4572947 , @Michael137 
> wrote:
>
>> Are you still planning on moving this forward? Otherwise I could commandeer 
>> the revision to get this in. I do think it's a useful bug to address
>
> I do. Locally I've been preparing additional changes on top of this patch 
> that expose enums in SBType, so that I can do end-to-end test to ensure this 
> sufficiently addresses my use case.
> On top of that, I'm planning to redo this patch after the example of 
> `DW_TAG_subprogram` per your suggestion.
>
> Unfortunately, I didn't have time last week for this, and now I'm knee deep 
> into triaging old Clang bugs. I'm not finished with that until number of open 
> issues is brought under 20k :)

Sounds good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Are you still planning on moving this forward? Otherwise I could commandeer the 
revision to get this in. I do think it's a useful bug to address


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D157059: [lldb][PECOFF] Exclude alignment padding when reading section data

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Hi, this is failing on swift-ci (runs on x86 bots) with the following error:

  
/Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/build/Ninja-ReleaseAssert+stdlib-Release/lldb-macosx-x86_64/unittests/ObjectFile/PECOFF/./ObjectFilePECOFFTests
 --gtest_filter=SectionSizeTest.NoAlignmentPadding
  --
  YAML:12:5: error: unknown key 'SizeOfRawData'
  SizeOfRawData:   512
  ^
  
/Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/llvm-project/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp:49:
 Failure
  Value of: llvm::detail::TakeExpected(ExpectedFile)
  Expected: succeeded
Actual: failed  (convertYAML() failed)
  
  
/Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/llvm-project/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp:49
  Value of: llvm::detail::TakeExpected(ExpectedFile)
  Expected: succeeded
Actual: failed  (convertYAML() failed)

https://ci.swift.org/view/LLDB/job/oss-lldb-incremental-macos-cmake/

Could you take a look please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157059

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3040
 
+case DW_TAG_enumeration_type:
+{

Endill wrote:
> Michael137 wrote:
> > Michael137 wrote:
> > > Michael137 wrote:
> > > > Michael137 wrote:
> > > > > At first glance this seems OK but I'll want to check why the type 
> > > > > doesn't get resolved when `clang::Sema` asks LLDB for it by name
> > > > Checking locally and will report back
> > > Ok, so what happens for `expr Outer::Enum::var` is that LLDB will first 
> > > resolve the `Outer` structure (and all its member fields, but not nested 
> > > types). When clang looks for `Enum`, it wants to find it in a direct 
> > > lookup into the `Outer` DeclContext. But that lookup will fail because we 
> > > never created the decls for the nested type. There is no fallback to the 
> > > external source in such a case unfortunately so LLDB never has the chance 
> > > to correct this. See the `LookupQualifiedName` code path in 
> > > `Sema::BuildCXXNestedNameSpecifier`.
> > > 
> > > So I think this proposed approach should be fine. But what I think could 
> > > be better is if we just do the same for `DW_TAG_enumeration_type` and 
> > > `DW_TAG_structure_type` as we do for `DW_TAG_subprogram`.
> > > 
> > > I.e., simply push back the type into `member_function_dies` (we should 
> > > rename this) and then it will get resolved properly in 
> > > `CompleteRecordType`.
> > Also `DW_TAG_union_type` while you're here :)
> I can look into doing things more lazily like `DW_TAG_subprogram`, if you can 
> point me to code paths for this on LLDB side:
> > When clang looks for Enum, it wants to find it in a direct lookup into the 
> > Outer DeclContext. But that lookup will fail because we never created the 
> > decls for the nested type. There is no fallback to the external source in 
> > such a case unfortunately so LLDB never has the chance to correct this.
The `DW_TAG_subprogram` path is just above this one in the switch statement 
(line 3030). Search for `member_function_dies` in this file.

> if you can point me to code paths for this on LLDB side

Clang requests type completion by calling `FindExternalVisibleDeclsByName` 
(which LLDB implements). So if you break in that function you should be able to 
see what LLDB finds when it looks for the outer structure and the enum. But you 
don't necessarily need to do that. If we just fill up the 
`member_function_dies` vector with the nested enum/union/structure types like 
we do for `DW_TAG_subprogram` that is sufficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb] Parse enums while parsing a type

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3040
 
+case DW_TAG_enumeration_type:
+{

Michael137 wrote:
> Michael137 wrote:
> > Michael137 wrote:
> > > At first glance this seems OK but I'll want to check why the type doesn't 
> > > get resolved when `clang::Sema` asks LLDB for it by name
> > Checking locally and will report back
> Ok, so what happens for `expr Outer::Enum::var` is that LLDB will first 
> resolve the `Outer` structure (and all its member fields, but not nested 
> types). When clang looks for `Enum`, it wants to find it in a direct lookup 
> into the `Outer` DeclContext. But that lookup will fail because we never 
> created the decls for the nested type. There is no fallback to the external 
> source in such a case unfortunately so LLDB never has the chance to correct 
> this. See the `LookupQualifiedName` code path in 
> `Sema::BuildCXXNestedNameSpecifier`.
> 
> So I think this proposed approach should be fine. But what I think could be 
> better is if we just do the same for `DW_TAG_enumeration_type` and 
> `DW_TAG_structure_type` as we do for `DW_TAG_subprogram`.
> 
> I.e., simply push back the type into `member_function_dies` (we should rename 
> this) and then it will get resolved properly in `CompleteRecordType`.
Also `DW_TAG_union_type` while you're here :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb] Parse enums while parsing a type

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Could you update the commit message with a description of the failure and 
summary of the fix? And change the title to something like 
`[lldb][DWARFASTParserClang] Resolve nested types when parsing structures`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb] Parse enums while parsing a type

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3040
 
+case DW_TAG_enumeration_type:
+{

Michael137 wrote:
> Michael137 wrote:
> > At first glance this seems OK but I'll want to check why the type doesn't 
> > get resolved when `clang::Sema` asks LLDB for it by name
> Checking locally and will report back
Ok, so what happens for `expr Outer::Enum::var` is that LLDB will first resolve 
the `Outer` structure (and all its member fields, but not nested types). When 
clang looks for `Enum`, it wants to find it in a direct lookup into the `Outer` 
DeclContext. But that lookup will fail because we never created the decls for 
the nested type. There is no fallback to the external source in such a case 
unfortunately so LLDB never has the chance to correct this. See the 
`LookupQualifiedName` code path in `Sema::BuildCXXNestedNameSpecifier`.

So I think this proposed approach should be fine. But what I think could be 
better is if we just do the same for `DW_TAG_enumeration_type` and 
`DW_TAG_structure_type` as we do for `DW_TAG_subprogram`.

I.e., simply push back the type into `member_function_dies` (we should rename 
this) and then it will get resolved properly in `CompleteRecordType`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156827: [lldb][test] Skip *-dbg-info-content API tests

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ce7831b4023: [lldb][test] Skip *-dbg-info-content API tests 
(authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156827

Files:
  
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py


Index: 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 
Index: 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 


Index: lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 
Index: lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156822: [lldb] Make IR interpretation interruptible

[Michael Buch via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D156822

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

[Lldb-commits] [PATCH] D156827: [lldb][test] Skip *-dbg-info-content API tests

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 546226.
Michael137 added a comment.

- Amend commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156827

Files:
  
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py


Index: 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 
Index: 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 


Index: lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 
Index: lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156827: [lldb][test] Skip *-dbg-info-content API tests

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, iana, JDevlieghere.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

These tests started failing on the public build-bots recently
with following error:

  AssertionError: 'error: Couldn't lookup symbols:
__ZNSt3__122__libcpp_verbose_abortEPKcz
  ' is not success

We've seen this previously when the SDKs we used to compile the
`std` module differ from the test program. (see D146714 
, rdar://107052293)

Skip these tests on older MacOS versions for now.

This is possibly related to the recent `std` module changes in D144322 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156827

Files:
  
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py


Index: 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 
Index: 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 


Index: lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 
Index: lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
@@ -12,6 +12,7 @@
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler="clang", compiler_version=["<", "12.0"])
+@skipIf(macos_version=["<", "14.0"])
 def test(self):
 self.build()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156774: [lldb] Parse enums while parsing a type

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3040
 
+case DW_TAG_enumeration_type:
+{

Michael137 wrote:
> At first glance this seems OK but I'll want to check why the type doesn't get 
> resolved when `clang::Sema` asks LLDB for it by name
Checking locally and will report back


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb] Parse enums while parsing a type

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Thanks for addressing this.

Can you add a test? Possibly in `lldb/test/API/lang/cpp/enum_types/`. Or 
`lldb/test/API/lang/cpp/const_static_integral_member/`




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3040
 
+case DW_TAG_enumeration_type:
+{

At first glance this seems OK but I'll want to check why the type doesn't get 
resolved when `clang::Sema` asks LLDB for it by name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156606: [lldb] Improve memory usage by freeing CTF types (NFC)

[Michael Buch via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D156606

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

[Lldb-commits] [PATCH] D156498: [lldb] Support recursive record types in CTF

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Generally LGTM, just some clarifications questions




Comment at: lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp:508
   ctf_record.name.data(), tag_kind, 
eLanguageTypeC);
+  m_compiler_types[record_type.GetOpaqueQualType()] = &ctf_record;
+  Declaration decl;

Just to clarify the lifetimes. This `ctf_record` lives on `m_ast` while the 
`m_compiler_types` on the SymbolFile, so we're guaranteed that the `ctf_record` 
lives long enough?



Comment at: lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp:524-529
+  // We only support resolving record types.
+  assert(ctf_type->kind == CTFType::Kind::eStruct ||
+ ctf_type->kind == CTFType::Kind::eUnion);
 
-  m_ast->StartTagDeclarationDefinition(record_type);
-  for (const CTFRecord::Field &field : ctf_record.fields) {
-if (Type *field_type = ResolveTypeUID(field.type)) {
-  const uint32_t field_size = field_type->GetByteSize(nullptr).value_or(0);
-  TypeSystemClang::AddFieldToRecordType(record_type, field.name,
-field_type->GetFullCompilerType(),
-eAccessPublic, field_size);
+  // Cast to the appropriate CTF type.
+  const CTFRecord *ctf_record = static_cast(ctf_type);

Nit: would it make sense to just add `classof` methods to `CTFType` and use the 
llvm cast facilities?

Feel free to ignore since there's just one instance of such cast afaict


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

https://reviews.llvm.org/D156498

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

[Lldb-commits] [PATCH] D156447: [lldb] Split CTF parsing and type creation (NFC)

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/CTF/CTFTypes.h:108
+  uint32_t size, std::vector values)
+  : CTFType(eEnum, uid, name), nelems(nelems), size(size) {
+assert(values.size() == nelems);

JDevlieghere wrote:
> Michael137 wrote:
> > did you omit this by accident?
> Yes, good eye. I need to see why my test didn't catch this. 
(also to add to the suggestion, probably best to use `this->values.size()` in 
the assert below to avoid a use-after-move; not sure which `values` takes 
precedence)


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

https://reviews.llvm.org/D156447

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

[Lldb-commits] [PATCH] D156447: [lldb] Split CTF parsing and type creation (NFC)

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/CTF/CTFTypes.h:108
+  uint32_t size, std::vector values)
+  : CTFType(eEnum, uid, name), nelems(nelems), size(size) {
+assert(values.size() == nelems);

did you omit this by accident?


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

https://reviews.llvm.org/D156447

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

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb0b2b6bab4d2: [lldb][PlatformDarwin] Parse SDK path for 
module compilation from debug-info (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,8 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -23,8 +25,55 @@
 class XcodeSDKModuleTests : public testing::Test {
   SubsystemRAII subsystems;
 };
-} // namespace
 
+struct SDKPathParsingTestData {
+  /// Each path will be put into a new CU's
+  /// DW_AT_LLVM_sysroot.
+  std::vector input_sdk_paths;
+
+  /// 'true' if we expect \ref GetSDKPathFromDebugInfo
+  /// to notify us about an SDK mismatch.
+  bool expect_mismatch;
+
+  /// 'true if the test expects the parsed SDK to
+  /// be an internal one.
+  bool expect_internal_sdk;
+
+  /// A substring that the final parsed sdk
+  /// is expected to contain.
+  llvm::StringRef expect_sdk_path_pattern;
+};
+
+struct SDKPathParsingMultiparamTests
+: public XcodeSDKModuleTests,
+  public testing::WithParamInterface {
+  std::vector
+  createCompileUnits(std::vector const &sdk_paths) {
+std::vector compile_units;
+
+for (auto sdk_path : sdk_paths) {
+  compile_units.emplace_back(llvm::formatv(
+  R"(
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:{0}
+- CStr:{1}
+- AbbrCode:0x
+)",
+  llvm::sys::path::filename(sdk_path, llvm::sys::path::Style::posix),
+  sdk_path));
+}
+
+return compile_units;
+  }
+};
+} // namespace
 
 TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   const char *yamldata = R"(
@@ -72,4 +121,190 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  EXPECT_FALSE(static_cast(path_or_err));
+  llvm::consumeError(path_or_err.takeError());
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
+  // Tests that parsing a CU without a DW_AT_APPLE_sdk fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"/Li

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 544378.
Michael137 added a comment.

- EXPECT -> ASSERT


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,8 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -23,8 +25,55 @@
 class XcodeSDKModuleTests : public testing::Test {
   SubsystemRAII subsystems;
 };
-} // namespace
 
+struct SDKPathParsingTestData {
+  /// Each path will be put into a new CU's
+  /// DW_AT_LLVM_sysroot.
+  std::vector input_sdk_paths;
+
+  /// 'true' if we expect \ref GetSDKPathFromDebugInfo
+  /// to notify us about an SDK mismatch.
+  bool expect_mismatch;
+
+  /// 'true if the test expects the parsed SDK to
+  /// be an internal one.
+  bool expect_internal_sdk;
+
+  /// A substring that the final parsed sdk
+  /// is expected to contain.
+  llvm::StringRef expect_sdk_path_pattern;
+};
+
+struct SDKPathParsingMultiparamTests
+: public XcodeSDKModuleTests,
+  public testing::WithParamInterface {
+  std::vector
+  createCompileUnits(std::vector const &sdk_paths) {
+std::vector compile_units;
+
+for (auto sdk_path : sdk_paths) {
+  compile_units.emplace_back(llvm::formatv(
+  R"(
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:{0}
+- CStr:{1}
+- AbbrCode:0x
+)",
+  llvm::sys::path::filename(sdk_path, llvm::sys::path::Style::posix),
+  sdk_path));
+}
+
+return compile_units;
+  }
+};
+} // namespace
 
 TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   const char *yamldata = R"(
@@ -72,4 +121,190 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  EXPECT_FALSE(static_cast(path_or_err));
+  llvm::consumeError(path_or_err.takeError());
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
+  // Tests that parsing a CU without a DW_AT_APPLE_sdk fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 544376.
Michael137 added a comment.

- Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,8 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -23,8 +25,55 @@
 class XcodeSDKModuleTests : public testing::Test {
   SubsystemRAII subsystems;
 };
-} // namespace
 
+struct SDKPathParsingTestData {
+  /// Each path will be put into a new CU's
+  /// DW_AT_LLVM_sysroot.
+  std::vector input_sdk_paths;
+
+  /// 'true' if we expect \ref GetSDKPathFromDebugInfo
+  /// to notify us about an SDK mismatch.
+  bool expect_mismatch;
+
+  /// 'true if the test expects the parsed SDK to
+  /// be an internal one.
+  bool expect_internal_sdk;
+
+  /// A substring that the final parsed sdk
+  /// is expected to contain.
+  llvm::StringRef expect_sdk_path_pattern;
+};
+
+struct SDKPathParsingMultiparamTests
+: public XcodeSDKModuleTests,
+  public testing::WithParamInterface {
+  std::vector
+  createCompileUnits(std::vector const &sdk_paths) {
+std::vector compile_units;
+
+for (auto sdk_path : sdk_paths) {
+  compile_units.emplace_back(llvm::formatv(
+  R"(
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:{0}
+- CStr:{1}
+- AbbrCode:0x
+)",
+  llvm::sys::path::filename(sdk_path, llvm::sys::path::Style::posix),
+  sdk_path));
+}
+
+return compile_units;
+  }
+};
+} // namespace
 
 TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   const char *yamldata = R"(
@@ -72,4 +121,190 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  EXPECT_FALSE(static_cast(path_or_err));
+  llvm::consumeError(path_or_err.takeError());
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
+  // Tests that parsing a CU without a DW_AT_APPLE_sdk fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YA

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 544350.
Michael137 added a comment.

- Parameterize tests
- Return bool to indicate SDK mismatch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,8 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -23,8 +25,55 @@
 class XcodeSDKModuleTests : public testing::Test {
   SubsystemRAII subsystems;
 };
-} // namespace
 
+struct SDKPathParsingTestData {
+  /// Each path will be put into a new CU's
+  /// DW_AT_LLVM_sysroot.
+  std::vector input_sdk_paths;
+
+  /// 'true' if we expect \ref GetSDKPathFromDebugInfo
+  /// to notify us about an SDK mismatch.
+  bool expect_mismatch;
+
+  /// 'true if the test expects the parsed SDK to
+  /// be an internal one.
+  bool expect_internal_sdk;
+
+  /// A substring that the final parsed sdk
+  /// is expected to contain.
+  llvm::StringRef expect_sdk_path_pattern;
+};
+
+struct SDKPathParsingMultiparamTests
+: public XcodeSDKModuleTests,
+  public testing::WithParamInterface {
+  std::vector
+  createCompileUnits(std::vector const &sdk_paths) {
+std::vector compile_units;
+
+for (auto sdk_path : sdk_paths) {
+  compile_units.emplace_back(llvm::formatv(
+  R"(
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:{0}
+- CStr:{1}
+- AbbrCode:0x
+)",
+  llvm::sys::path::filename(sdk_path, llvm::sys::path::Style::posix),
+  sdk_path));
+}
+
+return compile_units;
+  }
+};
+} // namespace
 
 TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   const char *yamldata = R"(
@@ -72,4 +121,190 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  EXPECT_FALSE(static_cast(path_or_err));
+  llvm::consumeError(path_or_err.takeError());
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
+  // Tests that parsing a CU without a DW_AT_APPLE_sdk fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+  

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 543967.
Michael137 marked an inline comment as not done.
Michael137 added a comment.

- Update unit-tests
- Expand function docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,181 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InternalAndPublicSDK) {
+  // Tests that we can parse the SDK path from debug-info.
+  // In the presence of multiple compile units, one of which
+  // points to an internal SDK, we should pick the internal SDK.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x0010
+- CStr:"something.invalid.sdk"
+- CStr:"/invalid/path/to/something.invalid.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x0010
+- CStr:"iPhoneOS14.0.Internal.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast(dwarf_unit);
+  ASSERT_TRUE(static_cast(dwarf_cu));
+  SymbolFileDWARF &sym_file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), 4U);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module);
+  ASSERT_TRUE(static_cast(sdk_or_err));
+
+  ASSERT_TRUE(sdk_or_err->IsAppleInternalSDK());
+  ASSERT_NE(sdk_or_err->GetString().find("Internal.sdk"), std::string::npos);
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  Ab

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 marked 2 inline comments as not done.
Michael137 added inline comments.



Comment at: lldb/include/lldb/Target/Platform.h:479
+/// to an internal SDK
+bool found_internal_sdk = false;
+

Michael137 wrote:
> aprantl wrote:
> > These flags really only make sense in the context of an XcodeSDK, so why 
> > not just return an XcodeSDK or XcodeSDK::Info object here? Otherwise we'll 
> > probably introduce subtle bugs due to a lossy translation between the flags.
> Yup I think that'd be better. That'll also make it easier to use from the 
> Swift plugin
Actually on second look, the `XcodeSDK` and `XcodeSDK::Info` objects represent 
information about a single (possibly parsed) SDK path. Whereas what the 
intention here was is to let the caller know whether we encountered a 
public/internal SDK while scanning all the CUs. Since we only return a single 
`XcodeSDK` (not all the ones we looked at) in my opinion it isn't quite right 
to store that information in it.

This is all really only used to [[ 
https://github.com/apple/llvm-project/blob/6c39bfc9d521dd8af03ca5e9e6ec7d5d4a6e5e6e/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp#L1700-L1704
 | print a Swift health ]]. Maybe we could instead just log this to 
`LLDBLog::Types`? Then we don't need to worry about returning any of this 
information. @aprantl 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

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

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1363
+  XcodeSDK sdk;
+  for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i)
+if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i))

Only remaining question is whether we want to limit this to just Objective-C 
and Swift. Going through each compile unit for C++ seems like a lot of work for 
something that we won't use

@aprantl 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

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

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 543875.
Michael137 added a comment.

- Remove more headers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,169 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InternalAndPublicSDK) {
+  // Tests that we can parse the SDK path from debug-info.
+  // In the presence of multiple compile units, one of which
+  // points to an internal SDK, we should pick the internal SDK.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+- Code:0x0002
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0010
+- CStr:"iPhoneOS14.0.Internal.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast(dwarf_unit);
+  ASSERT_TRUE(static_cast(dwarf_cu));
+  SymbolFileDWARF &sym_file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), 2U);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module);
+  ASSERT_TRUE(static_cast(sdk_or_err));
+
+  ASSERT_TRUE(sdk_or_err->IsAppleInternalSDK());
+  ASSERT_NE(sdk_or_err->GetString().find("Internal.sdk"), std::string::npos);
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  A

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 543874.
Michael137 added a comment.

- Remove redundant header


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Target/Platform.cpp
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,169 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InternalAndPublicSDK) {
+  // Tests that we can parse the SDK path from debug-info.
+  // In the presence of multiple compile units, one of which
+  // points to an internal SDK, we should pick the internal SDK.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+- Code:0x0002
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0010
+- CStr:"iPhoneOS14.0.Internal.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast(dwarf_unit);
+  ASSERT_TRUE(static_cast(dwarf_cu));
+  SymbolFileDWARF &sym_file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), 2U);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module);
+  ASSERT_TRUE(static_cast(sdk_or_err));
+
+  ASSERT_TRUE(sdk_or_err->IsAppleInternalSDK());
+  ASSERT_NE(sdk_or_err->GetString().find("Internal.sdk"), std::string::npos);
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::Reso

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 543870.
Michael137 added a comment.

- Move into `PlatformDarwin`
- Return `XcodeSDK` from `GetSDKPathFromDebugInfo` so it's easier to re-use 
from Swift plugin
- Introduce `ResolveSDKPathFromDebugInfo` to be used from `PlatformDarwin`
- Adjust tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Target/Platform.cpp
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,169 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InternalAndPublicSDK) {
+  // Tests that we can parse the SDK path from debug-info.
+  // In the presence of multiple compile units, one of which
+  // points to an internal SDK, we should pick the internal SDK.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+- Code:0x0002
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0010
+- CStr:"iPhoneOS14.0.Internal.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast(dwarf_unit);
+  ASSERT_TRUE(static_cast(dwarf_cu));
+  SymbolFileDWARF &sym_file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), 2U);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module);
+  ASSERT_TRUE(static_cast(sdk_or_err));
+
+  ASSERT_TRUE(sdk_or_err->IsAppleInternalSDK());
+  ASSERT_NE(sdk_or_err->GetString().find("Internal.sdk"), std::string::npos);
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+  

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/include/lldb/Target/Platform.h:479
+/// to an internal SDK
+bool found_internal_sdk = false;
+

aprantl wrote:
> These flags really only make sense in the context of an XcodeSDK, so why not 
> just return an XcodeSDK or XcodeSDK::Info object here? Otherwise we'll 
> probably introduce subtle bugs due to a lossy translation between the flags.
Yup I think that'd be better. That'll also make it easier to use from the Swift 
plugin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

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

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/include/lldb/Target/Platform.h:492
+  static llvm::Expected
+  GetSDKPathFromDebugInfo(Module &module);
+

Michael137 wrote:
> I'm open to suggestions on where to put this API. `Platform` seems a bit too 
> generic. But the intention is to share it with the Swift plugin. I could put 
> it into `PlatformDarwin`.
See how I intend to use it from the Swift plugin [[ 
https://github.com/apple/llvm-project/pull/7094/files#diff-35515cd82b78e2f7bbeff1d175f83a9dbe48066615f29cd377bd9cbf4591cb70
 | here ]]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

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

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 543160.
Michael137 added a comment.

- Remove redundant headers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Target/Platform.cpp
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,174 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InternalAndPublicSDK) {
+  // Tests that we can parse the SDK path from debug-info.
+  // In the presence of multiple compile units, one of which
+  // points to an internal SDK, we should pick the internal SDK.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+- Code:0x0002
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0010
+- CStr:"iPhoneOS14.0.Internal.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast(dwarf_unit);
+  ASSERT_TRUE(static_cast(dwarf_cu));
+  SymbolFileDWARF &sym_file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), 2U);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = Platform::GetSDKPathFromDebugInfo(*module);
+  ASSERT_TRUE(!!path_or_err);
+
+  auto [sdk_path, found_internal, found_public] = *path_or_err;
+
+  ASSERT_TRUE(found_internal);
+  ASSERT_TRUE(found_public);
+  ASSERT_NE(sdk_path.find("Internal.sdk"), std::string::npos);
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto p

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: lldb/include/lldb/Target/Platform.h:492
+  static llvm::Expected
+  GetSDKPathFromDebugInfo(Module &module);
+

I'm open to suggestions on where to put this API. `Platform` seems a bit too 
generic. But the intention is to share it with the Swift plugin. I could put it 
into `PlatformDarwin`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020

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

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When we build the Clang module compilation command (e.g., when
a user requests import of a module via `expression @import Foundation`),
LLDB will try to determine which SDK directory to use as the `sysroot`.
However, it currently does so by simply enumerating the `SDKs` directory
and picking the last one that's appropriate for module compilation
(see `PlatformDarwin::GetSDKDirectoryForModules`). That means if we have
multiple platform SDKs installed (e.g., a public and internal one), we
may pick the wrong one by chance.

On Darwin platforms we emit the SDK path that a object
file was compiled against into DWARF (using `DW_AT_LLVM_sysroot`
and `DW_AT_APPLE_sdk`). For Swift debugging, we already parse the SDK
path from debug-info if we can.

This patch mimicks the Swift behaviour for non-Swift languages. I.e.,
if we can get the SDK path from debug-info, do so. Otherwise, fall back
to the old heuristic.

rdar://110407148


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156020

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Target/Platform.cpp
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,174 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InternalAndPublicSDK) {
+  // Tests that we can parse the SDK path from debug-info.
+  // In the presence of multiple compile units, one of which
+  // points to an internal SDK, we should pick the internal SDK.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+- Code:0x0002
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_no
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+- Attribute:   DW_AT_APPLE_sdk
+  Form:DW_FORM_string
+- Attribute:   DW_AT_LLVM_sysroot
+  Form:DW_FORM_string
+  debug_info:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0010
+- CStr:"iPhoneOS14.0.Internal.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast(dwarf_unit);
+  ASSERT_TRUE(static_cast(dwarf_cu));
+  SymbolFileDWARF &sym_file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), 2U);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = Platform::GetSDKPathFromDebugInfo(*module);
+  ASSERT_TRUE(!!path_or_err);
+
+  auto [sdk_path, found_internal, found_public] = *path_or_err;
+
+  ASSERT_TRUE(found_internal);
+  ASSERT_TRUE(found_public);
+  ASSERT_NE(sdk_path.find("Internal.sdk"), std::string::npos);
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory 

[Lldb-commits] [PATCH] D154730: [lldb] Consider TAG_imported_declaration in DebugNamesIndex

[Michael Buch via Phabricator via lldb-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154730

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

[Lldb-commits] [PATCH] D153043: [lldb] Fix handling of cfi_restore in the unwinder

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

This is now failing with:

  clang: warning: argument unused during compilation: 
'-fmodules-cache-path=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-clang/lldb-shell'
 [-Wunused-command-line-argument]
  Undefined symbols for architecture x86_64:
"abort", referenced from:
asm_main in eh-frame-dwarf-unwind-abort-edbc93.o
   (maybe you meant: g_hard_abort)
  ld: symbol(s) not found for architecture x86_64

https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/56648/execution/node/74/log/?consoleFull


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153043

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

[Lldb-commits] [PATCH] D153043: [lldb] Fix handling of cfi_restore in the unwinder

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Looks like this is failing on the Darwin x86_64 buildbots: 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/56510/execution/node/74/log/

  Exit Code: 1
  
  Command Output (stderr):
  --
  clang: warning: argument unused during compilation: 
'-fmodules-cache-path=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-clang/lldb-shell'
 [-Wunused-command-line-argument]
  
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s:25:2:
 error: unknown directive
   .size g_hard_abort, 1
   ^
  
  
  Failed Tests (1):
lldb-shell :: Unwind/eh-frame-dwarf-unwind-abort.test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153043

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

[Lldb-commits] [PATCH] D152806: [lldb][test] Re-XFAIL prefer-debug-over-eh-frame.test

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfae704bad900: [lldb][test] Re-XFAIL 
prefer-debug-over-eh-frame.test (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152806

Files:
  lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test


Index: lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
===
--- lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
+++ lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
@@ -5,6 +5,7 @@
 # be thrown.
 
 # UNSUPPORTED: system-windows
+# XFAIL: system-darwin
 # REQUIRES: target-x86_64, native
 
 # RUN: %clang_host -g %p/Inputs/call-asm.c 
%p/Inputs/prefer-debug-over-eh-frame.s -o %t


Index: lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
===
--- lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
+++ lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
@@ -5,6 +5,7 @@
 # be thrown.
 
 # UNSUPPORTED: system-windows
+# XFAIL: system-darwin
 # REQUIRES: target-x86_64, native
 
 # RUN: %clang_host -g %p/Inputs/call-asm.c %p/Inputs/prefer-debug-over-eh-frame.s -o %t
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152806: [lldb][test] Re-XFAIL prefer-debug-over-eh-frame.test

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This was un-XFAILed in `83cb2123be487302070562c45e6eb4955b22c2b4`
due to D144999 . Since then D152540 
 fixed emission of eh_frame's
on Darwin, causing this test to fail again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152806

Files:
  lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test


Index: lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
===
--- lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
+++ lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
@@ -5,6 +5,7 @@
 # be thrown.
 
 # UNSUPPORTED: system-windows
+# XFAIL: system-darwin
 # REQUIRES: target-x86_64, native
 
 # RUN: %clang_host -g %p/Inputs/call-asm.c 
%p/Inputs/prefer-debug-over-eh-frame.s -o %t


Index: lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
===
--- lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
+++ lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
@@ -5,6 +5,7 @@
 # be thrown.
 
 # UNSUPPORTED: system-windows
+# XFAIL: system-darwin
 # REQUIRES: target-x86_64, native
 
 # RUN: %clang_host -g %p/Inputs/call-asm.c %p/Inputs/prefer-debug-over-eh-frame.s -o %t
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152454: [lldb][test] LogTest: Fix stack overflow

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c4c67a682f9: [lldb][test] LogTest: Fix stack overflow 
(authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152454

Files:
  lldb/unittests/Utility/LogTest.cpp


Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -309,7 +309,7 @@
   Err));
 llvm::StringRef Msg = logAndTakeOutputf("Hello World");
 char File[12];
-char Function[17];
+char Function[18];
 
 sscanf(Msg.str().c_str(),
"%[^:]:%s Hello World", File,


Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -309,7 +309,7 @@
   Err));
 llvm::StringRef Msg = logAndTakeOutputf("Hello World");
 char File[12];
-char Function[17];
+char Function[18];
 
 sscanf(Msg.str().c_str(),
"%[^:]:%s Hello World", File,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152454: [lldb][test] LogTest: Fix stack overflow

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The expected function name requires 18 bytes of storage.

Caught by the public ASAN buildbot


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152454

Files:
  lldb/unittests/Utility/LogTest.cpp


Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -309,7 +309,7 @@
   Err));
 llvm::StringRef Msg = logAndTakeOutputf("Hello World");
 char File[12];
-char Function[17];
+char Function[18];
 
 sscanf(Msg.str().c_str(),
"%[^:]:%s Hello World", File,


Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -309,7 +309,7 @@
   Err));
 llvm::StringRef Msg = logAndTakeOutputf("Hello World");
 char File[12];
-char Function[17];
+char Function[18];
 
 sscanf(Msg.str().c_str(),
"%[^:]:%s Hello World", File,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151811: [lldb] Take StringRef name in GetIndexOfChildWithName (NFC)

[Michael Buch via Phabricator via lldb-commits]

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

lgtm




Comment at: lldb/source/Core/ValueObjectSyntheticFilter.cpp:338
   if (!did_find && m_synth_filter_up != nullptr) {
 uint32_t index = m_synth_filter_up->GetIndexOfChildWithName(name);
 if (index == UINT32_MAX)

Could pass name_ref directly now, instead of doing the implicit conversion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151811

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

[Lldb-commits] [PATCH] D151813: [lldb] Take StringRef names in GetChildAtNamePath (NFC)

[Michael Buch via Phabricator via lldb-commits]

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

lgtm




Comment at: lldb/include/lldb/Core/ValueObject.h:483
   // this will always create the children if necessary
-  lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef names,
- ConstString *name_of_error = nullptr);
+  lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef 
names);
 

Guessing removing the parameter is fine because no callers were actually 
passing it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151813

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

[Lldb-commits] [PATCH] D151748: [lldb] Consult summary provider before printing children of root references

[Michael Buch via Phabricator via lldb-commits]

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

makes sense


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151748

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

[Lldb-commits] [PATCH] D151603: [lldb][NFCI] Refactor Language::GetFormatterPrefixSuffix

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

modulo Felipe's comment, LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151603

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

[Lldb-commits] [PATCH] D151268: [lldb][DataFormatter] Add dereference support to libstdcpp std::shared_ptr formatter

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a subscriber: compnerd.
Michael137 added a comment.

Any chance someone with a working aarch64 windows 10 setup could take a look? 
(@compnerd ?) Or let me know how I might be able to set this up.

The place where we seem to crash in `TestVarPath.py` is:

  # Make sure we don't crash when looking for non existant child 
  # in type with synthetic children. This used to cause a crash. 
  v = frame.GetValueForVariablePath('pt_sp->not_valid_child')
  self.assertTrue(v.GetError().Fail(),   
  "Make sure we don't find 'pt_sp->not_valid_child'")

A crash that was previously addressed in `0d6f681292d5a`. Interestingly, the 
code path that was added in that patch is the one that we used to take before 
adding support for `$$dereference$$` in this patch.


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

https://reviews.llvm.org/D151268

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

[Lldb-commits] [PATCH] D151268: [lldb][DataFormatter] Add dereference support to libstdcpp std::shared_ptr formatter

[Michael Buch via Phabricator via lldb-commits]

Michael137 closed this revision.
Michael137 added a comment.

Committed in https://reviews.llvm.org/rG44bb442fd5be


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

https://reviews.llvm.org/D151268

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

[Lldb-commits] [PATCH] D151268: [lldb][DataFormatter] Add dereference support to libstdcpp std::shared_ptr formatter

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 525110.
Michael137 added a comment.

Fix expected string in test


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

https://reviews.llvm.org/D151268

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
@@ -1,12 +1,17 @@
 #include 
 #include 
 
+struct Foo {
+  int mem = 5;
+};
+
 int
 main()
 {
 std::shared_ptr nsp;
 std::shared_ptr isp(new int{123});
 std::shared_ptr ssp = std::make_shared("foobar");
+std::shared_ptr fsp = std::make_shared();
 
 std::weak_ptr nwp;
 std::weak_ptr iwp = isp;
@@ -15,6 +20,7 @@
 nsp.reset(); // Set break point at this line.
 isp.reset();
 ssp.reset();
+fsp.reset();
 
 return 0; // Set break point at this line.
 }
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
@@ -33,6 +33,13 @@
 self.expect("frame variable iwp", substrs=['iwp = 123'])
 self.expect("frame variable swp", substrs=['swp = "foobar"'])
 
+self.expect("frame variable *nsp", substrs=['*nsp = '])
+self.expect("frame variable *isp", substrs=['*isp = 123'])
+self.expect("frame variable *ssp", substrs=['*ssp = "foobar"'])
+self.expect("frame variable *fsp", substrs=['*fsp = (mem = 5)'])
+
+self.expect("frame variable fsp->mem", substrs=['(int) fsp->mem = 5'])
+
 self.runCmd("continue")
 
 self.expect("frame variable nsp", substrs=['nsp = nullptr'])
Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -73,6 +73,15 @@
   bool MightHaveChildren() override;
 
   size_t GetIndexOfChildWithName(ConstString name) override;
+private:
+
+  // The lifetime of a ValueObject and all its derivative ValueObjects
+  // (children, clones, etc.) is managed by a ClusterManager. These
+  // objects are only destroyed when every shared pointer to any of them
+  // is destroyed, so we must not store a shared pointer to any ValueObject
+  // derived from our backend ValueObject (since we're in the same cluster).
+  ValueObject* m_ptr_obj = nullptr; // Underlying pointer (held, not owned)
+  ValueObject* m_obj_obj = nullptr; // Underlying object (held, not owned)
 };
 
 } // end of anonymous namespace
@@ -367,24 +376,48 @@
 
 lldb::ValueObjectSP
 LibStdcppSharedPtrSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
-  ValueObjectSP valobj_sp = m_backend.GetSP();
-  if (!valobj_sp)
-return lldb::ValueObjectSP();
-
   if (idx == 0)
-return valobj_sp->GetChildMemberWithName(ConstString("_M_ptr"), true);
-  else
-return lldb::ValueObjectSP();
+return m_ptr_obj->GetSP();
+  if (idx == 1)
+return m_obj_obj->GetSP();
+
+  return lldb::ValueObjectSP();
 }
 
-bool LibStdcppSharedPtrSyntheticFrontEnd::Update() { return false; }
+bool LibStdcppSharedPtrSyntheticFrontEnd::Update() {
+  auto backend = m_backend.GetSP();
+  if (!backend)
+return false;
+
+  auto valobj_sp = backend->GetNonSyntheticValue();
+  if (!valobj_sp)
+return false;
+
+  auto ptr_obj_sp = valobj_sp->GetChildMemberWithName(ConstString("_M_ptr"), true);
+  if (!ptr_obj_sp)
+return false;
+
+  m_ptr_obj = ptr_obj_sp->Clone(ConstString("pointer")).get();
+
+  if (m_ptr_obj) {
+Status error;
+ValueObjectSP obj_obj = m_ptr_obj->Dereference(error);
+if (error.Success()) {
+  m_obj_obj = obj_obj->Clone(ConstString("object")).get();
+}
+  }
+
+  return false;
+}
 
 bool LibStdcppSharedPtrSyntheticFrontEnd::MightHaveChildren() { return true; }
 
 size_t LibStdcppSharedPtrSyntheticFrontEnd::GetIndexOfChildWithName(
 ConstString name) {
-  if (name == "_M_ptr")
+  if (name == "pointer")
 return 0;
+  if (name == "object" || name == "$$dereference$$")
+return 1;
   return UINT32_MAX;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mail

[Lldb-commits] [PATCH] D151268: [lldb][DataFormatter] Add dereference support to libstdcpp std::shared_ptr formatter

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This mimicks the implementation of the libstdcpp std::unique_ptr
formatter.

This has been attempted several years ago in
`0789722d85cf1f1fdbe2ffb2245ea0ba034a9f94` but was reverted in
`e7dd3972094c2f2fb42dc9d4d5344e54a431e2ce`.

The difference to the original patch is that we now maintain
a `$$dereference$$` member and we only store weak pointers
to the other children inside the synthetic frontend. This is
what the libc++ formatters do to prevent the recursion mentioned
in the revert commit.

This patch addresses https://github.com/llvm/llvm-project/issues/62825


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151268

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
@@ -1,12 +1,17 @@
 #include 
 #include 
 
+struct Foo {
+  int mem = 5;
+};
+
 int
 main()
 {
 std::shared_ptr nsp;
 std::shared_ptr isp(new int{123});
 std::shared_ptr ssp = std::make_shared("foobar");
+std::shared_ptr fsp = std::make_shared();
 
 std::weak_ptr nwp;
 std::weak_ptr iwp = isp;
@@ -15,6 +20,7 @@
 nsp.reset(); // Set break point at this line.
 isp.reset();
 ssp.reset();
+fsp.reset();
 
 return 0; // Set break point at this line.
 }
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
@@ -33,6 +33,13 @@
 self.expect("frame variable iwp", substrs=['iwp = 123'])
 self.expect("frame variable swp", substrs=['swp = "foobar"'])
 
+self.expect("frame variable *nsp", substrs=['*nsp = '])
+self.expect("frame variable *isp", substrs=['*isp = 123'])
+self.expect("frame variable *ssp", substrs=['*ssp = "foobar"'])
+self.expect("frame variable *fsp", substrs=['*fsp = (mem = 5)'])
+
+self.expect("frame variable fsp->mem", substrs=['(int) f->mem = 5'])
+
 self.runCmd("continue")
 
 self.expect("frame variable nsp", substrs=['nsp = nullptr'])
Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -73,6 +73,15 @@
   bool MightHaveChildren() override;
 
   size_t GetIndexOfChildWithName(ConstString name) override;
+private:
+
+  // The lifetime of a ValueObject and all its derivative ValueObjects
+  // (children, clones, etc.) is managed by a ClusterManager. These
+  // objects are only destroyed when every shared pointer to any of them
+  // is destroyed, so we must not store a shared pointer to any ValueObject
+  // derived from our backend ValueObject (since we're in the same cluster).
+  ValueObject* m_ptr_obj = nullptr; // Underlying pointer (held, not owned)
+  ValueObject* m_obj_obj = nullptr; // Underlying object (held, not owned)
 };
 
 } // end of anonymous namespace
@@ -367,24 +376,48 @@
 
 lldb::ValueObjectSP
 LibStdcppSharedPtrSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
-  ValueObjectSP valobj_sp = m_backend.GetSP();
-  if (!valobj_sp)
-return lldb::ValueObjectSP();
-
   if (idx == 0)
-return valobj_sp->GetChildMemberWithName(ConstString("_M_ptr"), true);
-  else
-return lldb::ValueObjectSP();
+return m_ptr_obj->GetSP();
+  if (idx == 1)
+return m_obj_obj->GetSP();
+
+  return lldb::ValueObjectSP();
 }
 
-bool LibStdcppSharedPtrSyntheticFrontEnd::Update() { return false; }
+bool LibStdcppSharedPtrSyntheticFrontEnd::Update() {
+  auto backend = m_backend.GetSP();
+  if (!backend)
+return false;
+
+  auto valobj_sp = backend->GetNonSyntheticValue();
+  if (!valobj_sp)
+return false;
+
+  auto ptr_obj_sp = valobj_sp->GetChildMemberWithName(ConstString("_M_ptr"), true);
+  if (!ptr_obj_sp)
+return false;
+
+  m_ptr_obj = ptr_obj_sp->Clone(ConstString("pointer")).get();
+
+  if (m_ptr_obj) {
+Status error;
+ValueObjectSP obj_obj = m_ptr_obj->Dereferen

[Lldb-commits] [PATCH] D150954: [lldb][cmake] Allow specifying custom libcxx for tests in standalone builds

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG55acb70b211a: [lldb][cmake] Allow specifying custom libcxx 
for tests in standalone builds (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150954

Files:
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/test/CMakeLists.txt


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -169,9 +169,18 @@
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)
   # For now check that the include directory exists.
-  set(cxx_dir "${LLVM_BINARY_DIR}/include/c++")
-  if(NOT EXISTS ${cxx_dir})
-message(WARNING "LLDB test suite requires libc++ in 
llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
+  set(cxx_dir "${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++")
+  if(EXISTS ${cxx_dir})
+# These variables make sure the API tests can run against a custom
+# build of libcxx even for standalone builds.
+set(LLDB_HAS_LIBCXX ON)
+set(LIBCXX_LIBRARY_DIR 
"${LLDB_TEST_LIBCXX_ROOT_DIR}/lib${LIBCXX_LIBDIR_SUFFIX}")
+set(LIBCXX_GENERATED_INCLUDE_DIR 
"${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++/v1")
+  else()
+message(FATAL_ERROR
+"Couldn't find libcxx build in '${LLDB_TEST_LIBCXX_ROOT_DIR}'. To 
run the "
+"test-suite for a standalone LLDB build please build libcxx and 
point "
+"LLDB_TEST_LIBCXX_ROOT_DIR to it.")
   endif()
 else()
   # We require libcxx for the test suite, so if we aren't building it,
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -16,6 +16,9 @@
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to 
llvm/include")
 set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR} CACHE PATH "Path to LLVM build tree")
 
+set(LLDB_TEST_LIBCXX_ROOT_DIR "${LLVM_BINARY_DIR}" CACHE PATH
+"The build root for libcxx. Used in standalone builds to point the API 
tests to a custom build of libcxx.")
+
 set(LLVM_LIT_ARGS "-sv" CACHE STRING "Default options for lit")
 
 set(lit_file_name "llvm-lit")


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -169,9 +169,18 @@
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)
   # For now check that the include directory exists.
-  set(cxx_dir "${LLVM_BINARY_DIR}/include/c++")
-  if(NOT EXISTS ${cxx_dir})
-message(WARNING "LLDB test suite requires libc++ in llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
+  set(cxx_dir "${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++")
+  if(EXISTS ${cxx_dir})
+# These variables make sure the API tests can run against a custom
+# build of libcxx even for standalone builds.
+set(LLDB_HAS_LIBCXX ON)
+set(LIBCXX_LIBRARY_DIR "${LLDB_TEST_LIBCXX_ROOT_DIR}/lib${LIBCXX_LIBDIR_SUFFIX}")
+set(LIBCXX_GENERATED_INCLUDE_DIR "${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++/v1")
+  else()
+message(FATAL_ERROR
+"Couldn't find libcxx build in '${LLDB_TEST_LIBCXX_ROOT_DIR}'. To run the "
+"test-suite for a standalone LLDB build please build libcxx and point "
+"LLDB_TEST_LIBCXX_ROOT_DIR to it.")
   endif()
 else()
   # We require libcxx for the test suite, so if we aren't building it,
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -16,6 +16,9 @@
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to llvm/include")
 set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR} CACHE PATH "Path to LLVM build tree")
 
+set(LLDB_TEST_LIBCXX_ROOT_DIR "${LLVM_BINARY_DIR}" CACHE PATH
+"The build root for libcxx. Used in standalone builds to point the API tests to a custom build of libcxx.")
+
 set(LLVM_LIT_ARGS "-sv" CACHE STRING "Default options for lit")
 
 set(lit_file_name "llvm-lit")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150954: [lldb][cmake] Allow specifying custom libcxx for tests in standalone builds

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 523804.
Michael137 added a comment.

- Move variable into `LLDBStandalone.cmake`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150954

Files:
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/test/CMakeLists.txt


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -169,9 +169,18 @@
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)
   # For now check that the include directory exists.
-  set(cxx_dir "${LLVM_BINARY_DIR}/include/c++")
-  if(NOT EXISTS ${cxx_dir})
-message(WARNING "LLDB test suite requires libc++ in 
llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
+  set(cxx_dir "${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++")
+  if(EXISTS ${cxx_dir})
+# These variables make sure the API tests can run against a custom
+# build of libcxx even for standalone builds.
+set(LLDB_HAS_LIBCXX ON)
+set(LIBCXX_LIBRARY_DIR 
"${LLDB_TEST_LIBCXX_ROOT_DIR}/lib${LIBCXX_LIBDIR_SUFFIX}")
+set(LIBCXX_GENERATED_INCLUDE_DIR 
"${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++/v1")
+  else()
+message(FATAL_ERROR
+"Couldn't find libcxx build in '${LLDB_TEST_LIBCXX_ROOT_DIR}'. To 
run the "
+"test-suite for a standalone LLDB build please build libcxx and 
point "
+"LLDB_TEST_LIBCXX_ROOT_DIR to it.")
   endif()
 else()
   # We require libcxx for the test suite, so if we aren't building it,
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -16,6 +16,9 @@
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to 
llvm/include")
 set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR} CACHE PATH "Path to LLVM build tree")
 
+set(LLDB_TEST_LIBCXX_ROOT_DIR "${LLVM_BINARY_DIR}" CACHE PATH
+"The build root for libcxx. Used in standalone builds to point the API 
tests to a custom build of libcxx.")
+
 set(LLVM_LIT_ARGS "-sv" CACHE STRING "Default options for lit")
 
 set(lit_file_name "llvm-lit")


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -169,9 +169,18 @@
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)
   # For now check that the include directory exists.
-  set(cxx_dir "${LLVM_BINARY_DIR}/include/c++")
-  if(NOT EXISTS ${cxx_dir})
-message(WARNING "LLDB test suite requires libc++ in llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
+  set(cxx_dir "${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++")
+  if(EXISTS ${cxx_dir})
+# These variables make sure the API tests can run against a custom
+# build of libcxx even for standalone builds.
+set(LLDB_HAS_LIBCXX ON)
+set(LIBCXX_LIBRARY_DIR "${LLDB_TEST_LIBCXX_ROOT_DIR}/lib${LIBCXX_LIBDIR_SUFFIX}")
+set(LIBCXX_GENERATED_INCLUDE_DIR "${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++/v1")
+  else()
+message(FATAL_ERROR
+"Couldn't find libcxx build in '${LLDB_TEST_LIBCXX_ROOT_DIR}'. To run the "
+"test-suite for a standalone LLDB build please build libcxx and point "
+"LLDB_TEST_LIBCXX_ROOT_DIR to it.")
   endif()
 else()
   # We require libcxx for the test suite, so if we aren't building it,
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -16,6 +16,9 @@
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to llvm/include")
 set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR} CACHE PATH "Path to LLVM build tree")
 
+set(LLDB_TEST_LIBCXX_ROOT_DIR "${LLVM_BINARY_DIR}" CACHE PATH
+"The build root for libcxx. Used in standalone builds to point the API tests to a custom build of libcxx.")
+
 set(LLVM_LIT_ARGS "-sv" CACHE STRING "Default options for lit")
 
 set(lit_file_name "llvm-lit")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150954: [lldb][cmake] Allow specifying custom libcxx for tests in standalone builds

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Outstanding question would be whether we want to mirror this change in 
`utils/lldb-dotest/CMakeLists.txt`. If so, we should probably extract all this 
libcxx logic into a common helper between the two.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150954

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

[Lldb-commits] [PATCH] D150954: [lldb][cmake] Allow specifying custom libcxx for tests in standalone builds

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, bulbazord, JDevlieghere.
Herald added a subscriber: ekilmer.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Standalone builds currently do not set the `LLDB_HAS_LIBCXX`,
`LIBCXX_LIBRARY_DIR`, `LIBCXX_GENERATED_INCLUDE_DIR`.
These are necessary for API tests with `USE_LIBCPP` to run against
a custom built libcxx. Thus on all buildbots using standalone builds
(most notably the public swift-ci), the API tests always run against
the libcxx headers in the system SDK.

This patch introduces a new cmake variable `LLDB_TEST_LIBCXX_ROOT_DIR`
that allows us to point the tests in standalone builds to a custom
libcxx directory.

Since the user can control the libcxx location we can hard error if
no such custom libcxx build exists.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150954

Files:
  lldb/test/CMakeLists.txt


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -139,6 +139,9 @@
   )
 endif()
 
+set(LLDB_TEST_LIBCXX_ROOT_DIR "${LLVM_BINARY_DIR}" CACHE PATH
+"The build root for libcxx. Used in standalone builds to point the API 
tests to a custom build of libcxx.")
+
 # Add dependencies if we test with the in-tree clang.
 # This works with standalone builds as they import the clang target.
 if(TARGET clang)
@@ -169,9 +172,18 @@
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)
   # For now check that the include directory exists.
-  set(cxx_dir "${LLVM_BINARY_DIR}/include/c++")
-  if(NOT EXISTS ${cxx_dir})
-message(WARNING "LLDB test suite requires libc++ in 
llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
+  set(cxx_dir "${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++")
+  if(EXISTS ${cxx_dir})
+# These variables make sure the API tests can run against a custom
+# build of libcxx even for standalone builds.
+set(LLDB_HAS_LIBCXX ON)
+set(LIBCXX_LIBRARY_DIR 
"${LLDB_TEST_LIBCXX_ROOT_DIR}/lib${LIBCXX_LIBDIR_SUFFIX}")
+set(LIBCXX_GENERATED_INCLUDE_DIR 
"${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++/v1")
+  else()
+message(FATAL_ERROR
+"Couldn't find libcxx build in '${LLDB_TEST_LIBCXX_ROOT_DIR}'. To 
run the "
+"test-suite for a standalone LLDB build please build libcxx and 
point "
+"LLDB_TEST_LIBCXX_ROOT_DIR to it.")
   endif()
 else()
   # We require libcxx for the test suite, so if we aren't building it,


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -139,6 +139,9 @@
   )
 endif()
 
+set(LLDB_TEST_LIBCXX_ROOT_DIR "${LLVM_BINARY_DIR}" CACHE PATH
+"The build root for libcxx. Used in standalone builds to point the API tests to a custom build of libcxx.")
+
 # Add dependencies if we test with the in-tree clang.
 # This works with standalone builds as they import the clang target.
 if(TARGET clang)
@@ -169,9 +172,18 @@
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)
   # For now check that the include directory exists.
-  set(cxx_dir "${LLVM_BINARY_DIR}/include/c++")
-  if(NOT EXISTS ${cxx_dir})
-message(WARNING "LLDB test suite requires libc++ in llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
+  set(cxx_dir "${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++")
+  if(EXISTS ${cxx_dir})
+# These variables make sure the API tests can run against a custom
+# build of libcxx even for standalone builds.
+set(LLDB_HAS_LIBCXX ON)
+set(LIBCXX_LIBRARY_DIR "${LLDB_TEST_LIBCXX_ROOT_DIR}/lib${LIBCXX_LIBDIR_SUFFIX}")
+set(LIBCXX_GENERATED_INCLUDE_DIR "${LLDB_TEST_LIBCXX_ROOT_DIR}/include/c++/v1")
+  else()
+message(FATAL_ERROR
+"Couldn't find libcxx build in '${LLDB_TEST_LIBCXX_ROOT_DIR}'. To run the "
+"test-suite for a standalone LLDB build please build libcxx and point "
+"LLDB_TEST_LIBCXX_ROOT_DIR to it.")
   endif()
 else()
   # We require libcxx for the test suite, so if we aren't building it,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D150772#4349592 , @tbaeder wrote:

> Can you tell me what I have to do so `ninja check-lldb-api` actually runs the 
> test? All 1114 tests are currently marked "unsupported" for me.

Did you configure LLDB with `-DLLDB_ENABLE_PYTHON`? We should probably add that 
info to https://lldb.llvm.org/resources/test.html#running-the-full-test-suite


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150772

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

[Lldb-commits] [PATCH] D150591: [lldb][DWARFASTParserClang] Don't create unnamed bitfields to account for vtable pointer

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c30f224005e: [lldb][DWARFASTParserClang] Don't create 
unnamed bitfields to account for… (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150591

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
  lldb/test/API/lang/cpp/bitfields/main.cpp

Index: lldb/test/API/lang/cpp/bitfields/main.cpp
===
--- lldb/test/API/lang/cpp/bitfields/main.cpp
+++ lldb/test/API/lang/cpp/bitfields/main.cpp
@@ -113,6 +113,12 @@
 };
 HasBaseWithVTable base_with_vtable;
 
+struct DerivedWithVTable : public Base {
+  virtual ~DerivedWithVTable() {}
+  unsigned a : 1;
+};
+DerivedWithVTable derived_with_vtable;
+
 int main(int argc, char const *argv[]) {
   lba.a = 2;
 
@@ -153,5 +159,8 @@
   base_with_vtable.b = 0;
   base_with_vtable.c = 5;
 
+  derived_with_vtable.b_a = 2;
+  derived_with_vtable.a = 1;
+
   return 0; // break here
 }
Index: lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
===
--- lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
+++ lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
@@ -168,3 +168,14 @@
  result_children=with_vtable_and_unnamed_children)
 self.expect_var_path("with_vtable_and_unnamed",
  children=with_vtable_and_unnamed_children)
+
+derived_with_vtable_children = [
+ValueCheck(name="Base", children=[
+  ValueCheck(name="b_a", value="2", type="uint32_t")
+]),
+ValueCheck(name="a", value="1", type="unsigned int:1")
+]
+self.expect_expr("derived_with_vtable",
+ result_children=derived_with_vtable_children)
+self.expect_var_path("derived_with_vtable",
+ children=derived_with_vtable_children)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -223,12 +223,16 @@
 uint64_t bit_size = 0;
 uint64_t bit_offset = 0;
 bool is_bitfield = false;
+bool is_artificial = false;
 
 FieldInfo() = default;
 
 void SetIsBitfield(bool flag) { is_bitfield = flag; }
 bool IsBitfield() { return is_bitfield; }
 
+void SetIsArtificial(bool flag) { is_artificial = flag; }
+bool IsArtificial() const { return is_artificial; }
+
 bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
   // Any subsequent bitfields must not overlap and must be at a higher
   // bit offset than any previous bitfield + size.
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2934,8 +2934,10 @@
   // artificial member with (unnamed bitfield) padding.
   // FIXME: This check should verify that this is indeed an artificial member
   // we are supposed to ignore.
-  if (attrs.is_artificial)
+  if (attrs.is_artificial) {
+last_field_info.SetIsArtificial(true);
 return;
+  }
 
   if (!member_clang_type.IsCompleteType())
 member_clang_type.GetCompleteType();
@@ -3699,17 +3701,23 @@
 return false;
 
   // If we have a base class, we assume there is no unnamed
-  // bit-field if this is the first field since the gap can be
-  // attributed to the members from the base class. This assumption
-  // is not correct if the first field of the derived class is
-  // indeed an unnamed bit-field. We currently do not have the
-  // machinary to track the offset of the last field of classes we
-  // have seen before, so we are not handling this case.
+  // bit-field if either of the following is true:
+  // (a) this is the first field since the gap can be
+  // attributed to the members from the base class.
+  // FIXME: This assumption is not correct if the first field of
+  // the derived class is indeed an unnamed bit-field. We currently
+  // do not have the machinary to track the offset of the last field
+  // of classes we have seen before, so we are not handling this case.
+  // (b) Or, the first member of the derived class was a vtable pointer.
+  // In this case we don't want to create an unnamed bitfield either
+  // since those will be inserted by clang later.
   const bool have_base = layout_info.base_offsets.size() != 0;
   const bool this_is_first_field =
   last_field_info.bit_offset == 0 && last_field

[Lldb-commits] [PATCH] D150589: [lldb][DWARFASTParserClang][NFC] Simplify unnamed bitfield condition

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGca64f9af0447: [lldb][DWARFASTParserClang][NFC] Simplify 
unnamed bitfield condition (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150589

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


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2880,9 +2880,8 @@
 
 if (detect_unnamed_bitfields) {
   std::optional unnamed_field_info;
-  uint64_t last_field_end = 0;
-
-  last_field_end = last_field_info.bit_offset + last_field_info.bit_size;
+  uint64_t last_field_end =
+  last_field_info.bit_offset + last_field_info.bit_size;
 
   if (!last_field_info.IsBitfield()) {
 // The last field was not a bit-field...
@@ -2902,10 +2901,8 @@
   // indeed an unnamed bit-field. We currently do not have the
   // machinary to track the offset of the last field of classes we
   // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset != last_field_end &&
-  this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 &&
-last_field_info.bit_size == 0 &&
+  if (this_field_info.bit_offset > last_field_end &&
+  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
 layout_info.base_offsets.size() != 0)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2880,9 +2880,8 @@
 
 if (detect_unnamed_bitfields) {
   std::optional unnamed_field_info;
-  uint64_t last_field_end = 0;
-
-  last_field_end = last_field_info.bit_offset + last_field_info.bit_size;
+  uint64_t last_field_end =
+  last_field_info.bit_offset + last_field_info.bit_size;
 
   if (!last_field_info.IsBitfield()) {
 // The last field was not a bit-field...
@@ -2902,10 +2901,8 @@
   // indeed an unnamed bit-field. We currently do not have the
   // machinary to track the offset of the last field of classes we
   // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset != last_field_end &&
-  this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 &&
-last_field_info.bit_size == 0 &&
+  if (this_field_info.bit_offset > last_field_end &&
+  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
 layout_info.base_offsets.size() != 0)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150590: [lldb][DWARFASTParserClang][NFC] Extract condition for unnamed bitfield creation into helper function

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG56eff197d0d6: [lldb][DWARFASTParserClang][NFC] Extract 
condition for unnamed bitfield… (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150590

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


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -236,6 +236,22 @@
 }
   };
 
+  /// Returns 'true' if we should create an unnamed bitfield
+  /// and add it to the parser's current AST.
+  ///
+  /// \param[in] last_field_info FieldInfo of the previous DW_TAG_member
+  ///we parsed.
+  /// \param[in] last_field_end Offset (in bits) where the last parsed field
+  ///ended.
+  /// \param[in] this_field_info FieldInfo of the current DW_TAG_member
+  ///being parsed.
+  /// \param[in] layout_info Layout information of all decls parsed by the
+  ///current parser.
+  bool ShouldCreateUnnamedBitfield(
+  FieldInfo const &last_field_info, uint64_t last_field_end,
+  FieldInfo const &this_field_info,
+  lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const;
+
   /// Parses a DW_TAG_APPLE_property DIE and appends the parsed data to the
   /// list of delayed Objective-C properties.
   ///
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2892,18 +2892,8 @@
   last_field_end += word_width - (last_field_end % word_width);
   }
 
-  // If we have a gap between the last_field_end and the current
-  // field we have an unnamed bit-field.
-  // If we have a base class, we assume there is no unnamed
-  // bit-field if this is the first field since the gap can be
-  // attributed to the members from the base class. This assumption
-  // is not correct if the first field of the derived class is
-  // indeed an unnamed bit-field. We currently do not have the
-  // machinary to track the offset of the last field of classes we
-  // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
-layout_info.base_offsets.size() != 0)) {
+  if (ShouldCreateUnnamedBitfield(last_field_info, last_field_end,
+  this_field_info, layout_info)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =
 this_field_info.bit_offset - last_field_end;
@@ -3698,3 +3688,29 @@
 
   return !failures.empty();
 }
+
+bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
+FieldInfo const &last_field_info, uint64_t last_field_end,
+FieldInfo const &this_field_info,
+lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const {
+  // If we have a gap between the last_field_end and the current
+  // field we have an unnamed bit-field.
+  if (this_field_info.bit_offset <= last_field_end)
+return false;
+
+  // If we have a base class, we assume there is no unnamed
+  // bit-field if this is the first field since the gap can be
+  // attributed to the members from the base class. This assumption
+  // is not correct if the first field of the derived class is
+  // indeed an unnamed bit-field. We currently do not have the
+  // machinary to track the offset of the last field of classes we
+  // have seen before, so we are not handling this case.
+  const bool have_base = layout_info.base_offsets.size() != 0;
+  const bool this_is_first_field =
+  last_field_info.bit_offset == 0 && last_field_info.bit_size == 0;
+
+  if (have_base && this_is_first_field)
+return false;
+
+  return true;
+}


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -236,6 +236,22 @@
 }
   };
 
+  /// Returns 'true' if we should create an unnamed bitfield
+  /// and add it to the parser's current AST.
+  ///
+  /// \param[in] last_field_info FieldInfo of the previous DW_TAG_member
+  ///we parsed.
+  /// \param[in] last_field_end Offset (in bits) where the last parsed field
+  ///ended.
+  /// \param[in] this_field_info FieldInfo of the current DW_TAG_member
+  /// 

[Lldb-commits] [PATCH] D150591: [lldb][DWARFASTParserClang] Don't create unnamed bitfields to account for vtable pointer

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 522383.
Michael137 added a comment.

- Rephrase comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150591

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
  lldb/test/API/lang/cpp/bitfields/main.cpp

Index: lldb/test/API/lang/cpp/bitfields/main.cpp
===
--- lldb/test/API/lang/cpp/bitfields/main.cpp
+++ lldb/test/API/lang/cpp/bitfields/main.cpp
@@ -113,6 +113,12 @@
 };
 HasBaseWithVTable base_with_vtable;
 
+struct DerivedWithVTable : public Base {
+  virtual ~DerivedWithVTable() {}
+  unsigned a : 1;
+};
+DerivedWithVTable derived_with_vtable;
+
 int main(int argc, char const *argv[]) {
   lba.a = 2;
 
@@ -153,5 +159,8 @@
   base_with_vtable.b = 0;
   base_with_vtable.c = 5;
 
+  derived_with_vtable.b_a = 2;
+  derived_with_vtable.a = 1;
+
   return 0; // break here
 }
Index: lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
===
--- lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
+++ lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
@@ -168,3 +168,14 @@
  result_children=with_vtable_and_unnamed_children)
 self.expect_var_path("with_vtable_and_unnamed",
  children=with_vtable_and_unnamed_children)
+
+derived_with_vtable_children = [
+ValueCheck(name="Base", children=[
+  ValueCheck(name="b_a", value="2", type="uint32_t")
+]),
+ValueCheck(name="a", value="1", type="unsigned int:1")
+]
+self.expect_expr("derived_with_vtable",
+ result_children=derived_with_vtable_children)
+self.expect_var_path("derived_with_vtable",
+ children=derived_with_vtable_children)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -223,12 +223,16 @@
 uint64_t bit_size = 0;
 uint64_t bit_offset = 0;
 bool is_bitfield = false;
+bool is_artificial = false;
 
 FieldInfo() = default;
 
 void SetIsBitfield(bool flag) { is_bitfield = flag; }
 bool IsBitfield() { return is_bitfield; }
 
+void SetIsArtificial(bool flag) { is_artificial = flag; }
+bool IsArtificial() const { return is_artificial; }
+
 bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
   // Any subsequent bitfields must not overlap and must be at a higher
   // bit offset than any previous bitfield + size.
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2935,8 +2935,10 @@
   // artificial member with (unnamed bitfield) padding.
   // FIXME: This check should verify that this is indeed an artificial member
   // we are supposed to ignore.
-  if (attrs.is_artificial)
+  if (attrs.is_artificial) {
+last_field_info.SetIsArtificial(true);
 return;
+  }
 
   if (!member_clang_type.IsCompleteType())
 member_clang_type.GetCompleteType();
@@ -3697,17 +3699,23 @@
 return false;
 
   // If we have a base class, we assume there is no unnamed
-  // bit-field if this is the first field since the gap can be
-  // attributed to the members from the base class. This assumption
-  // is not correct if the first field of the derived class is
-  // indeed an unnamed bit-field. We currently do not have the
-  // machinary to track the offset of the last field of classes we
-  // have seen before, so we are not handling this case.
+  // bit-field if either of the following is true:
+  // (a) this is the first field since the gap can be
+  // attributed to the members from the base class.
+  // FIXME: This assumption is not correct if the first field of
+  // the derived class is indeed an unnamed bit-field. We currently
+  // do not have the machinary to track the offset of the last field
+  // of classes we have seen before, so we are not handling this case.
+  // (b) Or, the first member of the derived class was a vtable pointer.
+  // In this case we don't want to create an unnamed bitfield either
+  // since those will be inserted by clang later.
   const bool have_base = layout_info.base_offsets.size() != 0;
   const bool this_is_first_field =
   last_field_info.bit_offset == 0 && last_field_info.bit_size == 0;
+  const bool first_field_is_vptr =
+  last_field_info.bit_offset == 0 && last_field_i

[Lldb-commits] [PATCH] D150590: [lldb][DWARFASTParserClang][NFC] Extract condition for unnamed bitfield creation into helper function

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 522382.
Michael137 added a comment.

- Add doxygen comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150590

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


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -236,6 +236,22 @@
 }
   };
 
+  /// Returns 'true' if we should create an unnamed bitfield
+  /// and add it to the parser's current AST.
+  ///
+  /// \param[in] last_field_info FieldInfo of the previous DW_TAG_member
+  ///we parsed.
+  /// \param[in] last_field_end Offset (in bits) where the last parsed field
+  ///ended.
+  /// \param[in] this_field_info FieldInfo of the current DW_TAG_member
+  ///being parsed.
+  /// \param[in] layout_info Layout information of all decls parsed by the
+  ///current parser.
+  bool ShouldCreateUnnamedBitfield(
+  FieldInfo const &last_field_info, uint64_t last_field_end,
+  FieldInfo const &this_field_info,
+  lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const;
+
   /// Parses a DW_TAG_APPLE_property DIE and appends the parsed data to the
   /// list of delayed Objective-C properties.
   ///
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2893,18 +2893,8 @@
   last_field_end += word_width - (last_field_end % word_width);
   }
 
-  // If we have a gap between the last_field_end and the current
-  // field we have an unnamed bit-field.
-  // If we have a base class, we assume there is no unnamed
-  // bit-field if this is the first field since the gap can be
-  // attributed to the members from the base class. This assumption
-  // is not correct if the first field of the derived class is
-  // indeed an unnamed bit-field. We currently do not have the
-  // machinary to track the offset of the last field of classes we
-  // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
-layout_info.base_offsets.size() != 0)) {
+  if (ShouldCreateUnnamedBitfield(last_field_info, last_field_end,
+  this_field_info, layout_info)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =
 this_field_info.bit_offset - last_field_end;
@@ -3696,3 +3686,29 @@
 
   return !failures.empty();
 }
+
+bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
+FieldInfo const &last_field_info, uint64_t last_field_end,
+FieldInfo const &this_field_info,
+lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const {
+  // If we have a gap between the last_field_end and the current
+  // field we have an unnamed bit-field.
+  if (this_field_info.bit_offset <= last_field_end)
+return false;
+
+  // If we have a base class, we assume there is no unnamed
+  // bit-field if this is the first field since the gap can be
+  // attributed to the members from the base class. This assumption
+  // is not correct if the first field of the derived class is
+  // indeed an unnamed bit-field. We currently do not have the
+  // machinary to track the offset of the last field of classes we
+  // have seen before, so we are not handling this case.
+  const bool have_base = layout_info.base_offsets.size() != 0;
+  const bool this_is_first_field =
+  last_field_info.bit_offset == 0 && last_field_info.bit_size == 0;
+
+  if (have_base && this_is_first_field)
+return false;
+
+  return true;
+}


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -236,6 +236,22 @@
 }
   };
 
+  /// Returns 'true' if we should create an unnamed bitfield
+  /// and add it to the parser's current AST.
+  ///
+  /// \param[in] last_field_info FieldInfo of the previous DW_TAG_member
+  ///we parsed.
+  /// \param[in] last_field_end Offset (in bits) where the last parsed field
+  ///ended.
+  /// \param[in] this_field_info FieldInfo of the current DW_TAG_member
+  ///being parsed.
+  /// \param[in] layout_info Layout information of all decls parsed by the
+  ///   

[Lldb-commits] [PATCH] D150591: [lldb][DWARFASTParserClang] Don't create unnamed bitfields to account for vtable pointer

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

**Summary**

When filling out the LayoutInfo for a structure with the offsets
from DWARF, LLDB fills gaps in the layout by creating unnamed
bitfields and adding them to the AST. If we don't do this correctly
and our layout has overlapping fields, we will hat an assertion
in `clang::CGRecordLowering::lower()`. Specifically, if we have
a derived class with a VTable and a bitfield immediately following
the vtable pointer, we create a layout with overlapping fields.

This is an oversight in some of the previous cleanups done around this
area.

In `D76808`, we prevented LLDB from creating unnamed bitfields if there
was a gap between the last field of a base class and the start of a bitfield
in the derived class.

In `D112697`, we started accounting for the vtable pointer. The intention
there was to make sure the offset bookkeeping accounted for the
existence of a vtable pointer (but we didn't actually want to create
any AST nodes for it). Now that `last_field_info.bit_size` was being
set even for artifical fields, the previous fix `D76808` broke
specifically for cases where the bitfield was the first member of a
derived class with a vtable (this scenario wasn't tested so we didn't
notice it). I.e., we started creating redundant unnamed bitfields for
where the vtable pointer usually sits. This confused the lowering logic
in clang.

This patch adds a condition to `ShouldCreateUnnamedBitfield` which
checks whether the first field in the derived class is a vtable ptr.

**Testing**

- Added API test case


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150591

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
  lldb/test/API/lang/cpp/bitfields/main.cpp

Index: lldb/test/API/lang/cpp/bitfields/main.cpp
===
--- lldb/test/API/lang/cpp/bitfields/main.cpp
+++ lldb/test/API/lang/cpp/bitfields/main.cpp
@@ -113,6 +113,12 @@
 };
 HasBaseWithVTable base_with_vtable;
 
+struct DerivedWithVTable : public Base {
+  virtual ~DerivedWithVTable() {}
+  unsigned a : 1;
+};
+DerivedWithVTable derived_with_vtable;
+
 int main(int argc, char const *argv[]) {
   lba.a = 2;
 
@@ -153,5 +159,8 @@
   base_with_vtable.b = 0;
   base_with_vtable.c = 5;
 
+  derived_with_vtable.b_a = 2;
+  derived_with_vtable.a = 1;
+
   return 0; // break here
 }
Index: lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
===
--- lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
+++ lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
@@ -168,3 +168,14 @@
  result_children=with_vtable_and_unnamed_children)
 self.expect_var_path("with_vtable_and_unnamed",
  children=with_vtable_and_unnamed_children)
+
+derived_with_vtable_children = [
+ValueCheck(name="Base", children=[
+  ValueCheck(name="b_a", value="2", type="uint32_t")
+]),
+ValueCheck(name="a", value="1", type="unsigned int:1")
+]
+self.expect_expr("derived_with_vtable",
+ result_children=derived_with_vtable_children)
+self.expect_var_path("derived_with_vtable",
+ children=derived_with_vtable_children)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -223,12 +223,16 @@
 uint64_t bit_size = 0;
 uint64_t bit_offset = 0;
 bool is_bitfield = false;
+bool is_artificial = false;
 
 FieldInfo() = default;
 
 void SetIsBitfield(bool flag) { is_bitfield = flag; }
 bool IsBitfield() { return is_bitfield; }
 
+void SetIsArtificial(bool flag) { is_artificial = flag; }
+bool IsArtificial() const { return is_artificial; }
+
 bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
   // Any subsequent bitfields must not overlap and must be at a higher
   // bit offset than any previous bitfield + size.
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2935,8 +2935,10 @@
   // artificial member with (unnamed bitfield) padding.
   // FIXME: This check should verify that this is indeed an artificial member
   // we are supposed

[Lldb-commits] [PATCH] D150590: [lldb][DWARFASTParserClang][NFC] Extract condition for unnamed bitfield creation into helper function

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch adds a new private helper
`DWARFASTParserClang::ShouldCreateUnnamedBitfield` which
`ParseSingleMember` whether we should fill the current gap
in a structure layout with unnamed bitfields.

Extracting this logic will allow us to add additional
conditions in upcoming patches without jeoperdizing readability
of `ParseSingleMember`.

We also store some of the boolean conditions in local variables
to make the intent more obvious.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150590

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


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -236,6 +236,11 @@
 }
   };
 
+  bool ShouldCreateUnnamedBitfield(
+  FieldInfo const &last_field_info, uint64_t last_field_end,
+  FieldInfo const &this_field_info,
+  lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const;
+
   /// Parses a DW_TAG_APPLE_property DIE and appends the parsed data to the
   /// list of delayed Objective-C properties.
   ///
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2893,18 +2893,8 @@
   last_field_end += word_width - (last_field_end % word_width);
   }
 
-  // If we have a gap between the last_field_end and the current
-  // field we have an unnamed bit-field.
-  // If we have a base class, we assume there is no unnamed
-  // bit-field if this is the first field since the gap can be
-  // attributed to the members from the base class. This assumption
-  // is not correct if the first field of the derived class is
-  // indeed an unnamed bit-field. We currently do not have the
-  // machinary to track the offset of the last field of classes we
-  // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
-layout_info.base_offsets.size() != 0)) {
+  if (ShouldCreateUnnamedBitfield(last_field_info, last_field_end,
+  this_field_info, layout_info)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =
 this_field_info.bit_offset - last_field_end;
@@ -3696,3 +3686,29 @@
 
   return !failures.empty();
 }
+
+bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
+FieldInfo const &last_field_info, uint64_t last_field_end,
+FieldInfo const &this_field_info,
+lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const {
+  // If we have a gap between the last_field_end and the current
+  // field we have an unnamed bit-field.
+  if (this_field_info.bit_offset <= last_field_end)
+return false;
+
+  // If we have a base class, we assume there is no unnamed
+  // bit-field if this is the first field since the gap can be
+  // attributed to the members from the base class. This assumption
+  // is not correct if the first field of the derived class is
+  // indeed an unnamed bit-field. We currently do not have the
+  // machinary to track the offset of the last field of classes we
+  // have seen before, so we are not handling this case.
+  const bool have_base = layout_info.base_offsets.size() != 0;
+  const bool this_is_first_field =
+  last_field_info.bit_offset == 0 && last_field_info.bit_size == 0;
+
+  if (have_base && this_is_first_field)
+return false;
+
+  return true;
+}


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -236,6 +236,11 @@
 }
   };
 
+  bool ShouldCreateUnnamedBitfield(
+  FieldInfo const &last_field_info, uint64_t last_field_end,
+  FieldInfo const &this_field_info,
+  lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const;
+
   /// Parses a DW_TAG_APPLE_property DIE and appends the parsed data to the
   /// list of delayed Objective-C properties.
   ///
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWA

[Lldb-commits] [PATCH] D150589: [lldb][DWARFASTParserClang][NFC] Simplify unnamed bitfield condition

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Minor cleanup of redundant variable initialization and
if-condition. These are leftovers/oversights from previous
cleanup in this area:

- https://reviews.llvm.org/D72953
- https://reviews.llvm.org/D76808


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150589

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


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2881,9 +2881,8 @@
 
 if (detect_unnamed_bitfields) {
   std::optional unnamed_field_info;
-  uint64_t last_field_end = 0;
-
-  last_field_end = last_field_info.bit_offset + last_field_info.bit_size;
+  uint64_t last_field_end =
+  last_field_info.bit_offset + last_field_info.bit_size;
 
   if (!last_field_info.IsBitfield()) {
 // The last field was not a bit-field...
@@ -2903,10 +2902,8 @@
   // indeed an unnamed bit-field. We currently do not have the
   // machinary to track the offset of the last field of classes we
   // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset != last_field_end &&
-  this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 &&
-last_field_info.bit_size == 0 &&
+  if (this_field_info.bit_offset > last_field_end &&
+  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
 layout_info.base_offsets.size() != 0)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2881,9 +2881,8 @@
 
 if (detect_unnamed_bitfields) {
   std::optional unnamed_field_info;
-  uint64_t last_field_end = 0;
-
-  last_field_end = last_field_info.bit_offset + last_field_info.bit_size;
+  uint64_t last_field_end =
+  last_field_info.bit_offset + last_field_info.bit_size;
 
   if (!last_field_info.IsBitfield()) {
 // The last field was not a bit-field...
@@ -2903,10 +2902,8 @@
   // indeed an unnamed bit-field. We currently do not have the
   // machinary to track the offset of the last field of classes we
   // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset != last_field_end &&
-  this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 &&
-last_field_info.bit_size == 0 &&
+  if (this_field_info.bit_offset > last_field_end &&
+  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
 layout_info.base_offsets.size() != 0)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149949: [lldb][TypeSystem] ForEach: Don't hold the TypeSystemMap lock across callback

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D149949#4341608 , @labath wrote:

> Just as a drive-by, since we're constructing a copy anyway it would be 
> possible to merge this construction with the "uniqueing" behavior of the 
> `visited` set. I.e., instead of making a literal copy of the map, just 
> construct a `to_be_visited` set of /unique/ objects, and then iterate over 
> that.

Agreed that would be a nice cleanup. I considered it initially but thought I'd 
split it out into a separate patch (but never did)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149949

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

[Lldb-commits] [PATCH] D149949: [lldb][TypeSystem] ForEach: Don't hold the TypeSystemMap lock across callback

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D149949#431 , @aprantl wrote:

> Do we have other ForEach methods that contain a similar footgun?

Not that I can tell


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149949

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

[Lldb-commits] [PATCH] D149949: [lldb][TypeSystem] ForEach: Don't hold the TypeSystemMap lock across callback

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe74f03dde5d8: [lldb][TypeSystem] ForEach: Don't hold 
the TypeSystemMap lock across callback (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149949

Files:
  lldb/source/Symbol/TypeSystem.cpp


Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -217,11 +217,21 @@
 
 void TypeSystemMap::ForEach(
 std::function const &callback) {
-  std::lock_guard guard(m_mutex);
+
+  // The callback may call into this function again causing
+  // us to lock m_mutex twice if we held it across the callback.
+  // Since we just care about guarding access to 'm_map', make
+  // a local copy and iterate over that instead.
+  collection map_snapshot;
+  {
+  std::lock_guard guard(m_mutex);
+  map_snapshot = m_map;
+  }
+
   // Use a std::set so we only call the callback once for each unique
   // TypeSystem instance.
   llvm::DenseSet visited;
-  for (auto &pair : m_map) {
+  for (auto &pair : map_snapshot) {
 TypeSystem *type_system = pair.second.get();
 if (!type_system || visited.count(type_system))
   continue;


Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -217,11 +217,21 @@
 
 void TypeSystemMap::ForEach(
 std::function const &callback) {
-  std::lock_guard guard(m_mutex);
+
+  // The callback may call into this function again causing
+  // us to lock m_mutex twice if we held it across the callback.
+  // Since we just care about guarding access to 'm_map', make
+  // a local copy and iterate over that instead.
+  collection map_snapshot;
+  {
+  std::lock_guard guard(m_mutex);
+  map_snapshot = m_map;
+  }
+
   // Use a std::set so we only call the callback once for each unique
   // TypeSystem instance.
   llvm::DenseSet visited;
-  for (auto &pair : m_map) {
+  for (auto &pair : map_snapshot) {
 TypeSystem *type_system = pair.second.get();
 if (!type_system || visited.count(type_system))
   continue;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149949: [lldb][TypeSystem] ForEach: Don't hold the TypeSystemMap lock across callback

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 519803.
Michael137 added a comment.
Herald added a subscriber: JDevlieghere.

- Add back change dropped in cherry-pick


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149949

Files:
  lldb/source/Symbol/TypeSystem.cpp


Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -217,11 +217,21 @@
 
 void TypeSystemMap::ForEach(
 std::function const &callback) {
-  std::lock_guard guard(m_mutex);
+
+  // The callback may call into this function again causing
+  // us to lock m_mutex twice if we held it across the callback.
+  // Since we just care about guarding access to 'm_map', make
+  // a local copy and iterate over that instead.
+  collection map_snapshot;
+  {
+  std::lock_guard guard(m_mutex);
+  map_snapshot = m_map;
+  }
+
   // Use a std::set so we only call the callback once for each unique
   // TypeSystem instance.
   llvm::DenseSet visited;
-  for (auto &pair : m_map) {
+  for (auto &pair : map_snapshot) {
 TypeSystem *type_system = pair.second.get();
 if (!type_system || visited.count(type_system))
   continue;


Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -217,11 +217,21 @@
 
 void TypeSystemMap::ForEach(
 std::function const &callback) {
-  std::lock_guard guard(m_mutex);
+
+  // The callback may call into this function again causing
+  // us to lock m_mutex twice if we held it across the callback.
+  // Since we just care about guarding access to 'm_map', make
+  // a local copy and iterate over that instead.
+  collection map_snapshot;
+  {
+  std::lock_guard guard(m_mutex);
+  map_snapshot = m_map;
+  }
+
   // Use a std::set so we only call the callback once for each unique
   // TypeSystem instance.
   llvm::DenseSet visited;
-  for (auto &pair : m_map) {
+  for (auto &pair : map_snapshot) {
 TypeSystem *type_system = pair.second.get();
 if (!type_system || visited.count(type_system))
   continue;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149949: [lldb][TypeSystem] ForEach: Don't hold the TypeSystemMap lock across callback

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, jingham.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The `TypeSystemMap::m_mutex` guards against concurrent modifications
of members of `TypeSystemMap`. In particular, `m_map`.

`TypeSystemMap::ForEach` iterates through the entire `m_map` calling
a user-specified callback for each entry. This is all done while
`m_mutex` is locked. However, there's nothing that guarantees that
the callback itself won't call back into `TypeSystemMap` APIs on the
same thread. This lead to double-locking `m_mutex`, which is undefined
behaviour. We've seen this cause a deadlock in the swift plugin with
following backtrace:

  int main() {
  std::unique_ptr up = std::make_unique(5);
  
  volatile int val = *up;
  return val;
  }
  
  clang++ -std=c++2a -g -O1 main.cpp
  
  ./bin/lldb -o “br se -p return” -o run -o “v *up” -o “expr *up” -b



  frame #4: std::lock_guard::lock_guard
  frame #5: lldb_private::TypeSystemMap::GetTypeSystemForLanguage  Lock #2
  frame #6: lldb_private::TypeSystemMap::GetTypeSystemForLanguage
  frame #7: lldb_private::Target::GetScratchTypeSystemForLanguage
  ...
  frame #26: lldb_private::SwiftASTContext::LoadLibraryUsingPaths
  frame #27: lldb_private::SwiftASTContext::LoadModule
  frame #30: swift::ModuleDecl::collectLinkLibraries
  frame #31: lldb_private::SwiftASTContext::LoadModule
  frame #34: lldb_private::SwiftASTContext::GetCompileUnitImportsImpl
  frame #35: lldb_private::SwiftASTContext::PerformCompileUnitImports
  frame #36: 
lldb_private::TypeSystemSwiftTypeRefForExpressions::GetSwiftASTContext
  frame #37: 
lldb_private::TypeSystemSwiftTypeRefForExpressions::GetPersistentExpressionState
  frame #38: lldb_private::Target::GetPersistentSymbol
  frame #41: lldb_private::TypeSystemMap::ForEach  Lock #1
  frame #42: lldb_private::Target::GetPersistentSymbol
  frame #43: lldb_private::IRExecutionUnit::FindInUserDefinedSymbols
  frame #44: lldb_private::IRExecutionUnit::FindSymbol
  frame #45: 
lldb_private::IRExecutionUnit::MemoryManager::GetSymbolAddressAndPresence
  frame #46: lldb_private::IRExecutionUnit::MemoryManager::findSymbol
  frame #47: non-virtual thunk to 
lldb_private::IRExecutionUnit::MemoryManager::findSymbol
  frame #48: llvm::LinkingSymbolResolver::findSymbol
  frame #49: llvm::LegacyJITSymbolResolver::lookup
  frame #50: llvm::RuntimeDyldImpl::resolveExternalSymbols
  frame #51: llvm::RuntimeDyldImpl::resolveRelocations
  frame #52: llvm::MCJIT::finalizeLoadedModules
  frame #53: llvm::MCJIT::finalizeObject
  frame #54: lldb_private::IRExecutionUnit::ReportAllocations
  frame #55: lldb_private::IRExecutionUnit::GetRunnableInfo
  frame #56: lldb_private::ClangExpressionParser::PrepareForExecution
  frame #57: lldb_private::ClangUserExpression::TryParse
  frame #58: lldb_private::ClangUserExpression::Parse

Our solution is to simply iterate over a local copy of `m_map`.

**Testing**

- Confirmed on manual reproducer (would reproduce 100% of the time before the 
patch)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149949

Files:
  lldb/source/Symbol/TypeSystem.cpp


Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -217,7 +217,17 @@
 
 void TypeSystemMap::ForEach(
 std::function const &callback) {
-  std::lock_guard guard(m_mutex);
+
+  // The callback may call into this function again causing
+  // us to lock m_mutex twice if we held it across the callback.
+  // Since we just care about guarding access to 'm_map', make
+  // a local copy and iterate over that instead.
+  collection map_snapshot;
+  {
+  std::lock_guard guard(m_mutex);
+  map_snapshot = m_map;
+  }
+
   // Use a std::set so we only call the callback once for each unique
   // TypeSystem instance.
   llvm::DenseSet visited;


Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -217,7 +217,17 @@
 
 void TypeSystemMap::ForEach(
 std::function const &callback) {
-  std::lock_guard guard(m_mutex);
+
+  // The callback may call into this function again causing
+  // us to lock m_mutex twice if we held it across the callback.
+  // Since we just care about guarding access to 'm_map', make
+  // a local copy and iterate over that instead.
+  collection map_snapshot;
+  {
+  std::lock_guard guard(m_mutex);
+  map_snapshot = m_map;
+  }
+
   // Use a std::set so we only call the callback once for each unique
   // TypeSystem instance.
   llvm::DenseSet visited;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/l

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Thanks for fixing

Just checked and it works on my M1  with 
assertions.

I relanded the latest version.


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

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

I tried the new test case on my Mac but it's now hitting an assertion:

   TEST 'lldb-shell :: 
SymbolFile/DWARF/x86/DW_OP_div-with-signed.s' FAILED 
  Script:
  --
  : 'RUN: at line 2';   
/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang 
--target=specify-a-target-or-use-a-_host-substitution -c 
--target=x86_64-pc-linux -o /Users/michaelbuch/Git/lldb-build-main-no-mod
  
ules/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/DW_OP_div-with-signed.s.tmp
 
/Users/michaelbuch/Git/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
  : 'RUN: at line 3';   
/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/lldb --no-lldbinit -S 
/Users/michaelbuch/Git/lldb-build-main-no-modules/tools/lldb/test/Shell/lit-lldb-init-quiet
 /Users/michaelbuch/Git
  
/lldb-build-main-no-modules/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/DW_OP_div-with-signed.s.tmp
 -o "expression -T -- g" -o "exit" | 
/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/FileCheck /Users
  
/michaelbuch/Git/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  Assertion failed: ((!RootFile.Name.empty() || MCDwarfFiles.size() >= 1) && 
"No root file and no .file directives"), function emitV5FileDirTables, file 
MCDwarf.cpp, line 473.
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: 
/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang 
--target=specify-a-target-or-use-a-_host-substitution -c 
--target=x86_64-pc-linux -o /Users/michaelbuch/Git/lldb-build-main-n
  
o-modules/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/DW_OP_div-with-signed.s.tmp
 
/Users/michaelbuch/Git/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
   #0 0x00010940 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x104682660)
   #1 0x000109466bf8 PrintStackTraceSignalHandler(void*) 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x104682bf8)
   #2 0x0001094649d0 llvm::sys::RunSignalHandlers() 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x1046809d0)
   #3 0x000109465e10 llvm::sys::CleanupOnSignal(unsigned long) 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x104681e10)
   #4 0x0001093245c8 (anonymous 
namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x1045405c8)
   #5 0x000109324a64 CrashRecoverySignalHandler(int) 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x104540a64)
   #6 0x00018ddc2a24 (/usr/lib/system/libsystem_platform.dylib+0x18042ea24)
   #7 0x00018dd93c28 (/usr/lib/system/libsystem_pthread.dylib+0x1803ffc28)
   #8 0x00018dca1ae8 (/usr/lib/system/libsystem_c.dylib+0x18030dae8)
   #9 0x00018dca0e44 (/usr/lib/system/libsystem_c.dylib+0x18030ce44)
  #10 0x000108c3ad04 
llvm::MCDwarfLineTableHeader::emitV5FileDirTables(llvm::MCStreamer*, 
std::__1::optional&) const 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103e5
  6d04)
  #11 0x000108c39ff8 llvm::MCDwarfLineTableHeader::Emit(llvm::MCStreamer*, 
llvm::MCDwarfLineTableParams, llvm::ArrayRef, 
std::__1::optional&) const 
(/Users/michaelbuch/Git/lldb-build
  -main-no-modules/bin/clang-17+0x103e55ff8)
  #12 0x000108c3a178 llvm::MCDwarfLineTableHeader::Emit(llvm::MCStreamer*, 
llvm::MCDwarfLineTableParams, std::__1::optional&) const 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/c
  lang-17+0x103e56178)
  #13 0x000108c39920 llvm::MCDwarfLineTable::emitCU(llvm::MCStreamer*, 
llvm::MCDwarfLineTableParams, std::__1::optional&) const 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang
  -17+0x103e55920)
  #14 0x000108c397b4 llvm::MCDwarfLineTable::emit(llvm::MCStreamer*, 
llvm::MCDwarfLineTableParams) 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103e557b4)
  #15 0x000108c80320 llvm::MCObjectStreamer::finishImpl() 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103e9c320)
  #16 0x000108c528e4 llvm::MCELFStreamer::finishImpl() 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103e6e8e4)
  #17 0x000108cb0b88 llvm::MCStreamer::finish(llvm::SMLoc) 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103eccb88)
  #18 0x000108d51fb4 (anonymous namespace)::AsmParser::Run(bool, bool) 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x103f6dfb4)
  #19 0x000104e15b74 ExecuteAssemblerImpl((anonymous 
namespace)::AssemblerInvocation&, clang::DiagnosticsEngine&) 
(/Users/michaelbuch/Git/lldb-build-main-no-modules/bin/clang-17+0x100031b74)

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

Reverted for now until we find fix for test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D147370#4313196 , @asl wrote:

> This seems to fail on ARM: 
> https://lab.llvm.org/buildbot/#/builders/17/builds/37130

Investigating


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[Michael Buch via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe15d6b520e1e: [lldb][DWARFExpression] Fix DW_OP_div to use 
signed division (authored by jwnhy, committed by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147370

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
@@ -0,0 +1,468 @@
+  # Test handling of values represented via DW_OP_div
+
+  # RUN: %clang --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "b f" -o "r" -o "c" -o "c" -o "expression -T -- i" \
+  # RUN: -o "exit" | FileCheck %s
+
+  # Failing case was:
+  # (uint32_t) $0 = 0
+  # CHECK: (uint32_t) $0 = 2
+	
+  # This case is generated from the following code:
+  # #include "stdint.h"
+  # static volatile uint64_t g = 0;
+  #  static const int32_t f()
+  #  {
+  #uint32_t i;
+  #for (i = 0; (i != 10); i++)
+  #  ++g;
+  #return 0;
+  #  
+  #  }
+  #
+  #  int main()
+  #  {
+  #f();
+  #return 0;
+  #
+  #  }
+
+  .text
+	.file	"signed_dw_op_div.c"
+	.file	1 "/usr/local/include/bits" "types.h" md5 0x96c0983c9cdaf387938a8268d00aa594
+	.file	2 "/usr/local/include/bits" "stdint-uintn.h" md5 0xbedfab747425222bb150968c14e40abd
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 13 0  # signed_dw_op_div.c:13:0
+	.cfi_startproc
+# %bb.0:
+	movl	$3, %eax
+	#DEBUG_VALUE: f:i <- 0
+	.p2align	4, 0x90
+.LBB0_1:# =>This Inner Loop Header: Depth=1
+.Ltmp0:
+	#DEBUG_VALUE: f:i <- [DW_OP_consts 3, DW_OP_minus, DW_OP_consts 18446744073709551615, DW_OP_div, DW_OP_stack_value] $eax
+	.loc	0 8 7 prologue_end  # signed_dw_op_div.c:8:7
+	incq	g(%rip)
+.Ltmp1:
+	#DEBUG_VALUE: f:i <- [DW_OP_consts 3, DW_OP_minus, DW_OP_consts 18446744073709551615, DW_OP_div, DW_OP_consts 1, DW_OP_plus, DW_OP_stack_value] $eax
+	.loc	0 7 20  # signed_dw_op_div.c:7:20
+	decl	%eax
+.Ltmp2:
+	.loc	0 7 5 is_stmt 0 # signed_dw_op_div.c:7:5
+	jne	.LBB0_1
+.Ltmp3:
+# %bb.2:
+	.loc	0 15 5 is_stmt 1# signed_dw_op_div.c:15:5
+	xorl	%eax, %eax
+	retq
+.Ltmp4:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+	.file	3 "/usr/local/include/bits" "stdint-intn.h" md5 0x90039fb90b44dcbf118222513050fe57
+# -- End function
+	.type	g,@object   # @g
+	.local	g
+	.comm	g,8,8
+	.section	.debug_loclists,"",@progbits
+	.long	.Ldebug_list_header_end0-.Ldebug_list_header_start0 # Length
+.Ldebug_list_header_start0:
+	.short	5   # Version
+	.byte	8   # Address size
+	.byte	0   # Segment selector size
+	.long	1   # Offset entry count
+.Lloclists_table_base0:
+	.long	.Ldebug_loc0-.Lloclists_table_base0
+.Ldebug_loc0:
+	.byte	4   # DW_LLE_offset_pair
+	.uleb128 .Ltmp0-.Lfunc_begin0   #   starting offset
+	.uleb128 .Ltmp1-.Lfunc_begin0   #   ending offset
+	.byte	16  # Loc expr size
+	.byte	112 # DW_OP_breg0
+	.byte	0   # 0
+	.byte	16  # DW_OP_constu
+	.byte	255 # 4294967295
+	.byte	255 # 
+	.byte	255 # 
+	.byte	255 # 
+	.byte	15  # 
+	.byte	26  # DW_OP_and
+	.byte	17  # DW_OP_consts
+	.byte	3   # 3
+	.byte	28  # DW_OP_minus
+	.byte	17  # DW_OP_consts
+	.byte	127 # -1
+	.byte	27  # DW_OP_div
+	.byte	159 # DW_OP_stack_value
+	.byte	4   # DW_LLE_offset_pair
+	.uleb128 .Ltmp1-.Lfunc_begin0   #   starting offset
+	.uleb128 .Ltmp2-.Lfunc_begin0   #   ending offset
+	.byte	19  # Loc expr size
+	.byte	112 # DW_OP_breg0
+	.byte	0   # 0
+	.byte	16  # DW_OP_constu
+	.byte	255 # 4294967295
+	.byte	255 # 
+	.byte	255 

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

@jwnhy Let me know which name and email I should attribute the patch to


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

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D149394: Finish the job of D145569

[Michael Buch via Phabricator via lldb-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149394

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[Michael Buch via Phabricator via lldb-commits]

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

LGTM




Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s:3
+
+  # RUN: %clang --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "b f" -o "r" -o "c" -o "c" -o "expression -T -- i" \

other tests use `llvm-mc` as the assembler
E.g., `llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s`

I think it'll marginally speed things up



Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s:6
+  # RUN: -o "exit" | FileCheck %s
+
+  # Failing case was:

Can you paste the source code here which you used to derive this test case? For 
future readers


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

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D143347#4298613 , @kpdev42 wrote:

> So what are next steps? Are we going for implementation of 
> `DW_AT_no_unique_address` (which is going to be a non-standard extension) ? 
> @dblaikie @aprantl @Michael137

in my opinion that would be the most convenient way forward. Perhaps something 
like `DW_AT_LLVM_no_unique_address` even?

Btw what does GCC/gdb do in this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D148531: [lldb][DataFormatter] Fix libcxx std::deque formatter for references and pointers

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1414a5bdfeff: [lldb][DataFormatter] Fix libcxx std::deque 
formatter for references and… (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148531

Files:
  lldb/examples/synthetic/libcxx.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/main.cpp
@@ -0,0 +1,30 @@
+#include 
+#include 
+typedef std::deque int_deq;
+
+void by_ref_and_ptr(std::deque &ref, std::deque *ptr) {
+  printf("stop here");
+  return;
+}
+
+int main() {
+  int_deq numbers;
+  printf("break here");
+
+  (numbers.push_back(1));
+  printf("break here");
+
+  (numbers.push_back(12));
+  (numbers.push_back(123));
+  (numbers.push_back(1234));
+  (numbers.push_back(12345));
+  (numbers.push_back(123456));
+  (numbers.push_back(1234567));
+  by_ref_and_ptr(numbers, &numbers);
+  printf("break here");
+
+  numbers.clear();
+  printf("break here");
+
+  return 0;
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
@@ -0,0 +1,75 @@
+"""
+Test LLDB's data formatter for libcxx's std::deque.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxDequeDataFormatterTestCase(TestBase):
+
+def check_numbers(self, var_name):
+self.expect("frame variable " + var_name,
+substrs=[var_name + ' = size=7',
+ '[0] = 1',
+ '[1] = 12',
+ '[2] = 123',
+ '[3] = 1234',
+ '[4] = 12345',
+ '[5] = 123456',
+ '[6] = 1234567',
+ '}'])
+
+self.expect_expr(var_name, result_summary="size=7", result_children=[
+ValueCheck(value="1"),
+ValueCheck(value="12"),
+ValueCheck(value="123"),
+ValueCheck(value="1234"),
+ValueCheck(value="12345"),
+ValueCheck(value="123456"),
+ValueCheck(value="1234567"),
+])
+
+@add_test_categories(["libc++"])
+def test_with_run_command(self):
+"""Test basic formatting of std::deque"""
+self.build()
+(self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.cpp", False))
+
+self.expect("frame variable numbers",
+substrs=['numbers = size=0'])
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+# first value added
+self.expect("frame variable numbers",
+substrs=['numbers = size=1',
+ '[0] = 1',
+ '}'])
+
+# add remaining values
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+self.check_numbers("numbers")
+
+# clear out the deque
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+self.expect("frame variable numbers",
+substrs=['numbers = size=0'])
+
+@add_test_categories(["libc++"])
+def test_ref_and_ptr(self):
+"""Test formatting of std::deque& and std::deque*"""
+self.build()
+(self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "stop here", lldb.SBFileSpec("main.cpp", False))
+
+# The reference should display the same was as the value did
+self.check_numbers("ref")
+
+# The pointer should just show the right number of elements:
+self.expect("frame variable ptr", substrs=['ptr =', ' size=7'])
+self.expect("expression ptr", substrs=['$', 'size=7'])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/Makefile
@@ -0,0 +1,4 @@
+USE_LIBCPP := 1
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/examples/synthetic/libcxx.py
=

[Lldb-commits] [PATCH] D148531: [lldb][DataFormatter] Fix libcxx std::deque formatter for references and pointers

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, jingham.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

(Addresses GH#62153)

The `SBType` APIs to retrieve details about template arguments,
such as `GetTemplateArgumentType` or `GetTemplateArgumentKind`
don't "desugar" LValueReferences/RValueReferences or pointers.
So when we try to format a `std::deque&`, the python call to
`GetTemplateArgumentType` fails to get a type, leading to
an `element_size` of `0` and a division-by-zero python exception
(which gets caught by the summary provider silently). This leads
to the contents of such `std::deque&` to be printed incorrectly.

This patch dereferences the reference/pointer before calling
into the above SBAPIs.

**Testing**

- Add API test


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148531

Files:
  lldb/examples/synthetic/libcxx.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/main.cpp
@@ -0,0 +1,30 @@
+#include 
+#include 
+typedef std::deque int_deq;
+
+void by_ref_and_ptr(std::deque &ref, std::deque *ptr) {
+  printf("stop here");
+  return;
+}
+
+int main() {
+  int_deq numbers;
+  printf("break here");
+
+  (numbers.push_back(1));
+  printf("break here");
+
+  (numbers.push_back(12));
+  (numbers.push_back(123));
+  (numbers.push_back(1234));
+  (numbers.push_back(12345));
+  (numbers.push_back(123456));
+  (numbers.push_back(1234567));
+  by_ref_and_ptr(numbers, &numbers);
+  printf("break here");
+
+  numbers.clear();
+  printf("break here");
+
+  return 0;
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
@@ -0,0 +1,75 @@
+"""
+Test LLDB's data formatter for libcxx's std::deque.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxDequeDataFormatterTestCase(TestBase):
+
+def check_numbers(self, var_name):
+self.expect("frame variable " + var_name,
+substrs=[var_name + ' = size=7',
+ '[0] = 1',
+ '[1] = 12',
+ '[2] = 123',
+ '[3] = 1234',
+ '[4] = 12345',
+ '[5] = 123456',
+ '[6] = 1234567',
+ '}'])
+
+self.expect_expr(var_name, result_summary="size=7", result_children=[
+ValueCheck(value="1"),
+ValueCheck(value="12"),
+ValueCheck(value="123"),
+ValueCheck(value="1234"),
+ValueCheck(value="12345"),
+ValueCheck(value="123456"),
+ValueCheck(value="1234567"),
+])
+
+@add_test_categories(["libc++"])
+def test_with_run_command(self):
+"""Test basic formatting of std::deque"""
+self.build()
+(self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.cpp", False))
+
+self.expect("frame variable numbers",
+substrs=['numbers = size=0'])
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+# first value added
+self.expect("frame variable numbers",
+substrs=['numbers = size=1',
+ '[0] = 1',
+ '}'])
+
+# add remaining values
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+self.check_numbers("numbers")
+
+# clear out the deque
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+self.expect("frame variable numbers",
+substrs=['numbers = size=0'])
+
+@add_test_categories(["libc++"])
+def test_ref_and_ptr(self):
+"""Test formatting of std::deque& and std::deque*"""
+self.build()
+(self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "stop here", lldb.SBFileSpec("main.cpp", False))
+
+# The reference should display the same was as the value did
+self.check_numbers("ref")
+
+# The pointer should

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6cdfa2957437: [lldb][ClangExpression] Filter out non-root 
namespaces in FindNamespace (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147436

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolFileOnDemand.cpp
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/cpp/namespace/main.cpp

Index: lldb/test/API/lang/cpp/namespace/main.cpp
===
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,31 @@
 return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
 }
 
+namespace B {
+struct Bar {
+int x() { return 42; }
+};
+Bar bar;
+} // namespace B
+
+namespace A::B {
+struct Bar {
+int y() { return 137; }
+};
+} // namespace A::B
+
+namespace NS1::NS2 {
+struct Foo {
+int bar() { return -2; }
+};
+} // namespace NS1::NS2
+
+namespace NS2 {
+struct Foo {
+int bar() { return -3; }
+};
+} // namespace NS2
+
 int
 main (int argc, char const *argv[])
 {
@@ -112,5 +137,8 @@
 A::B::test_lookup_at_nested_ns_scope_after_using();
 test_lookup_before_using_directive();
 test_lookup_after_using_directive();
-return Foo::myfunc(12);
+::B::Bar bb;
+A::B::Bar ab;
+return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
+   NS2::Foo{}.bar();
 }
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,11 @@
 
 self.expect("expression variadic_sum", patterns=[
 '\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
+self.expect_expr("::NS1::NS2::Foo{}.bar() == -2 && ::NS2::Foo{}.bar() == -3",
+ result_type="bool", result_value="true")
+# FIXME: C++ unqualified namespace lookups currently not supported when instantiating types.
+self.expect_expr("NS2::Foo{}.bar() == -3", result_type="bool", result_value="false")
+self.expect_expr("((::B::Bar*)&::B::bar)->x()", result_type="int", result_value="42")
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
 
 CompilerDeclContext
 SymbolFileOnDemand::FindNamespace(ConstString name,
-  const CompilerDeclContext &parent_decl_ctx) {
+  const CompilerDeclContext &parent_decl_ctx,
+  bool only_root_namespaces) {
   if (!m_debug_info_enabled) {
 LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
  __FUNCTION__, name);
-return SymbolFile::FindNamespace(name, parent_decl_ctx);
+return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
   }
-  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx);
+  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx,
+only_root_namespaces);
 }
 
 std::vector>
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -157,9 +157,10 @@
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
-  lldb_private::CompilerDeclContext FindNamespace(
-  lldb_private::ConstString name,
-  const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+  lldb_private::CompilerDeclContext
+  FindNamespace(lldb_private::ConstString name,
+const lldb_private

[Lldb-commits] [PATCH] D143061: [lldb][Language] Add more language types

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0301a492f43e: [lldb][Language] Add more language types 
(authored by Michael137).

Changed prior to commit:
  https://reviews.llvm.org/D143061?vs=493896&id=513622#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143061

Files:
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Target/Language.cpp


Index: lldb/source/Target/Language.cpp
===
--- lldb/source/Target/Language.cpp
+++ lldb/source/Target/Language.cpp
@@ -194,6 +194,21 @@
 {"c++14", eLanguageTypeC_plus_plus_14},
 {"fortran03", eLanguageTypeFortran03},
 {"fortran08", eLanguageTypeFortran08},
+{"renderscript", eLanguageTypeRenderScript},
+{"bliss", eLanguageTypeBLISS},
+{"kotlin", eLanguageTypeKotlin},
+{"zig", eLanguageTypeZig},
+{"crystal", eLanguageTypeCrystal},
+{"",
+ static_cast(
+ 0x0029)}, // Not yet taken by any language in the DWARF spec
+   // and thus has no entry in LanguageType
+{"c++17", eLanguageTypeC_plus_plus_17},
+{"c++20", eLanguageTypeC_plus_plus_20},
+{"c17", eLanguageTypeC17},
+{"fortran18", eLanguageTypeFortran18},
+{"ada2005", eLanguageTypeAda2005},
+{"ada2012", eLanguageTypeAda2012},
 // Vendor Extensions
 {"assembler", eLanguageTypeMipsAssembler},
 // Now synonyms, in arbitrary order
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -479,12 +479,24 @@
   eLanguageTypeC_plus_plus_14 = 0x0021, ///< ISO C++:2014.
   eLanguageTypeFortran03 = 0x0022,  ///< ISO Fortran 2003.
   eLanguageTypeFortran08 = 0x0023,  ///< ISO Fortran 2008.
+  eLanguageTypeRenderScript = 0x0024,
+  eLanguageTypeBLISS = 0x0025,
+  eLanguageTypeKotlin = 0x0026,
+  eLanguageTypeZig = 0x0027,
+  eLanguageTypeCrystal = 0x0028,
+  eLanguageTypeC_plus_plus_17 = 0x002a, ///< ISO C++:2017.
+  eLanguageTypeC_plus_plus_20 = 0x002b, ///< ISO C++:2020.
+  eLanguageTypeC17 = 0x002c,
+  eLanguageTypeFortran18 = 0x002d,
+  eLanguageTypeAda2005 = 0x002e,
+  eLanguageTypeAda2012 = 0x002f,
+
   // Vendor Extensions
   // Note: Language::GetNameForLanguageType
   // assumes these can be used as indexes into array language_names, and
   // Language::SetLanguageFromCString and Language::AsCString assume these can
   // be used as indexes into array g_languages.
-  eLanguageTypeMipsAssembler = 0x0024,   ///< Mips_Assembler.
+  eLanguageTypeMipsAssembler,   ///< Mips_Assembler.
   eNumLanguageTypes
 };
 


Index: lldb/source/Target/Language.cpp
===
--- lldb/source/Target/Language.cpp
+++ lldb/source/Target/Language.cpp
@@ -194,6 +194,21 @@
 {"c++14", eLanguageTypeC_plus_plus_14},
 {"fortran03", eLanguageTypeFortran03},
 {"fortran08", eLanguageTypeFortran08},
+{"renderscript", eLanguageTypeRenderScript},
+{"bliss", eLanguageTypeBLISS},
+{"kotlin", eLanguageTypeKotlin},
+{"zig", eLanguageTypeZig},
+{"crystal", eLanguageTypeCrystal},
+{"",
+ static_cast(
+ 0x0029)}, // Not yet taken by any language in the DWARF spec
+   // and thus has no entry in LanguageType
+{"c++17", eLanguageTypeC_plus_plus_17},
+{"c++20", eLanguageTypeC_plus_plus_20},
+{"c17", eLanguageTypeC17},
+{"fortran18", eLanguageTypeFortran18},
+{"ada2005", eLanguageTypeAda2005},
+{"ada2012", eLanguageTypeAda2012},
 // Vendor Extensions
 {"assembler", eLanguageTypeMipsAssembler},
 // Now synonyms, in arbitrary order
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -479,12 +479,24 @@
   eLanguageTypeC_plus_plus_14 = 0x0021, ///< ISO C++:2014.
   eLanguageTypeFortran03 = 0x0022,  ///< ISO Fortran 2003.
   eLanguageTypeFortran08 = 0x0023,  ///< ISO Fortran 2008.
+  eLanguageTypeRenderScript = 0x0024,
+  eLanguageTypeBLISS = 0x0025,
+  eLanguageTypeKotlin = 0x0026,
+  eLanguageTypeZig = 0x0027,
+  eLanguageTypeCrystal = 0x0028,
+  eLanguageTypeC_plus_plus_17 = 0x002a, ///< ISO C++:2017.
+  eLanguageTypeC_plus_plus_20 = 0x002b, ///< ISO C++:2020.
+  eLanguageTypeC17 = 0x002c,
+  eLanguageTypeFortran18 = 0x002d,
+  eLanguageTypeAda2005 = 0x002e,
+  eLanguageTypeAda2012 = 0x002f,
+
   // Vendor Extensions
   // Note: Language::GetNameForLanguageType
   // assumes these can be used as indexes into array language_names, and
   // Language::SetLanguageFromCString and Language::AsCString assume these can
   // be used as indexes into array g_languages.
-  eLanguageTypeMipsAssembler = 0x0024,   ///< Mips_A

[Lldb-commits] [PATCH] D143062: [lldb] Allow evaluating expressions in C++20 mode

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 513621.
Michael137 added a comment.

- Add FIXME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143062

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Target/Language.cpp
  lldb/test/API/lang/cpp/standards/cpp20/Makefile
  lldb/test/API/lang/cpp/standards/cpp20/TestCPP20Standard.py
  lldb/test/API/lang/cpp/standards/cpp20/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
@@ -205,7 +205,11 @@
 
   {"auto Foo[abi:abc]::operator<<>(int) &", "auto",
"Foo[abi:abc]", "operator<<>", "(int)", "&",
-   "Foo[abi:abc]::operator<<>"}};
+   "Foo[abi:abc]::operator<<>"},
+
+  {"auto A::operator<=>[abi:tag]()", "auto", "A",
+   "operator<=>[abi:tag]", "()", "",
+   "A::operator<=>[abi:tag]"}};
 
   for (const auto &test : test_cases) {
 CPlusPlusLanguage::MethodName method(ConstString(test.input));
@@ -227,7 +231,6 @@
   std::string test_cases[] = {
   "int Foo::operator[]<[10>()",
   "Foo::operator bool[10]()",
-  "auto A::operator<=>[abi:tag]()",
   "auto A::operator<<<(int)",
   "auto A::operator>>>(int)",
   "auto A::operator<<(int)",
@@ -356,10 +359,9 @@
   EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
   "f>", context, basename));
 
-  // We expect these cases to fail until we turn on C++2a
-  EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+  EXPECT_TRUE(CPlusPlusLanguage::ExtractContextAndIdentifier(
   "A::operator<=>", context, basename));
-  EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+  EXPECT_TRUE(CPlusPlusLanguage::ExtractContextAndIdentifier(
   "operator<=>", context, basename));
 }
 
Index: lldb/test/API/lang/cpp/standards/cpp20/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp20/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+struct Foo {
+  friend auto operator<=>(Foo const &, Foo const &) { return true; }
+};
+
+int main() { return Foo{} <=> Foo{}; }
Index: lldb/test/API/lang/cpp/standards/cpp20/TestCPP20Standard.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp20/TestCPP20Standard.py
@@ -0,0 +1,16 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCPP20Standard(TestBase):
+def test_cpp20(self):
+"""
+Tests that we can evaluate an expression in C++20 mode
+"""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "Foo{}", lldb.SBFileSpec("main.cpp"))
+
+self.expect("expr -l c++11 -- Foo{} <=> Foo{}", error=True, substrs=["'<=>' is a single token in C++20; add a space to avoid a change in behavior"])
+
+self.expect("expr -l c++20 -- Foo{} <=> Foo{}", substrs=["(bool) $0 = true"])
Index: lldb/test/API/lang/cpp/standards/cpp20/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp20/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++20
+
+include Makefile.rules
Index: lldb/source/Target/Language.cpp
===
--- lldb/source/Target/Language.cpp
+++ lldb/source/Target/Language.cpp
@@ -267,6 +267,8 @@
   case eLanguageTypeC_plus_plus_03:
   case eLanguageTypeC_plus_plus_11:
   case eLanguageTypeC_plus_plus_14:
+  case eLanguageTypeC_plus_plus_17:
+  case eLanguageTypeC_plus_plus_20:
   case eLanguageTypeObjC_plus_plus:
 return true;
   default:
@@ -306,6 +308,8 @@
   case eLanguageTypeC_plus_plus_03:
   case eLanguageTypeC_plus_plus_11:
   case eLanguageTypeC_plus_plus_14:
+  case eLanguageTypeC_plus_plus_17:
+  case eLanguageTypeC_plus_plus_20:
   case eLanguageTypeObjC_plus_plus:
   case eLanguageTypeObjC:
 return true;
@@ -329,6 +333,8 @@
   case eLanguageTypeC_plus_plus_03:
   case eLanguageTypeC_plus_plus_11:
   case eLanguageTypeC_plus_plus_14:
+  case eLanguageTypeC_plus_plus_17:
+  case eLanguageTypeC_plus_plus_20:
 return eLanguageTypeC_plus_plus;
   case eLanguageTypeC:
   case eLanguageTypeC89:
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/Ty

[Lldb-commits] [PATCH] D143062: [lldb] Allow evaluating expressions in C++20 mode

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:548
+[[fallthrough]];
   case lldb::eLanguageTypeC_plus_plus:
   case lldb::eLanguageTypeC_plus_plus_11:

aprantl wrote:
> Why no case C++17?
Fair point, can add one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143062

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

[Lldb-commits] [PATCH] D143062: [lldb] Allow evaluating expressions in C++20 mode

[Michael Buch via Phabricator via lldb-commits]

Michael137 marked an inline comment as not done.
Michael137 added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:551
   case lldb::eLanguageTypeC_plus_plus_14:
 lang_opts.CPlusPlus11 = true;
 m_compiler->getHeaderSearchOpts().UseLibcxx = true;

Michael137 wrote:
> aprantl wrote:
> > Why does this not set C++14? Because it's effectively equivalent?
> I think it's because we haven't done the work to make c++14 in the expression 
> evaluator work, but the default clang language is c++14. So we downgrade it 
> silently to c++11 here.
> 
> We should really fix that, but I think this broke some tests in the past when 
> we tried enabling it (see `8cb75745412e4bc9592d2409cc6cfa4a2940d9e7`, 
> https://reviews.llvm.org/D80308)
But looking at the specific test failures from back then there doesn't seem 
like a good reason for why we can't re-enable that patch now. And also add a 
C++17 version.

Let me try that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143062

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

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 513547.
Michael137 added a comment.

- Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147436

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolFileOnDemand.cpp
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/cpp/namespace/main.cpp

Index: lldb/test/API/lang/cpp/namespace/main.cpp
===
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,31 @@
 return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
 }
 
+namespace B {
+struct Bar {
+int x() { return 42; }
+};
+Bar bar;
+} // namespace B
+
+namespace A::B {
+struct Bar {
+int y() { return 137; }
+};
+} // namespace A::B
+
+namespace NS1::NS2 {
+struct Foo {
+int bar() { return -2; }
+};
+} // namespace NS1::NS2
+
+namespace NS2 {
+struct Foo {
+int bar() { return -3; }
+};
+} // namespace NS2
+
 int
 main (int argc, char const *argv[])
 {
@@ -112,5 +137,8 @@
 A::B::test_lookup_at_nested_ns_scope_after_using();
 test_lookup_before_using_directive();
 test_lookup_after_using_directive();
-return Foo::myfunc(12);
+::B::Bar bb;
+A::B::Bar ab;
+return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
+   NS2::Foo{}.bar();
 }
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,11 @@
 
 self.expect("expression variadic_sum", patterns=[
 '\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
+self.expect_expr("::NS1::NS2::Foo{}.bar() == -2 && ::NS2::Foo{}.bar() == -3",
+ result_type="bool", result_value="true")
+# FIXME: C++ unqualified namespace lookups currently not supported when instantiating types.
+self.expect_expr("NS2::Foo{}.bar() == -3", result_type="bool", result_value="false")
+self.expect_expr("((::B::Bar*)&::B::bar)->x()", result_type="int", result_value="42")
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
 
 CompilerDeclContext
 SymbolFileOnDemand::FindNamespace(ConstString name,
-  const CompilerDeclContext &parent_decl_ctx) {
+  const CompilerDeclContext &parent_decl_ctx,
+  bool only_root_namespaces) {
   if (!m_debug_info_enabled) {
 LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
  __FUNCTION__, name);
-return SymbolFile::FindNamespace(name, parent_decl_ctx);
+return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
   }
-  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx);
+  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx,
+only_root_namespaces);
 }
 
 std::vector>
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -157,9 +157,10 @@
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
-  lldb_private::CompilerDeclContext FindNamespace(
-  lldb_private::ConstString name,
-  const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+  lldb_private::CompilerDeclContext
+  FindNamespace(lldb_private::ConstString name,
+const lldb_private::CompilerDeclContext &parent_decl_ctx,
+bool only_root_namespaces) override;
 
   llvm::StringRef G

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 513542.
Michael137 added a comment.

- Make condition more readable
- Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147436

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolFileOnDemand.cpp
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/cpp/namespace/main.cpp

Index: lldb/test/API/lang/cpp/namespace/main.cpp
===
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,30 @@
 return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
 }
 
+namespace B {
+struct Bar {
+int x() { return 42; }
+};
+} // namespace B
+
+namespace A::B {
+struct Bar {
+int y() { return 137; }
+};
+} // namespace A::B
+
+namespace NS1::NS2 {
+struct Foo {
+int bar() { return -2; }
+};
+} // namespace NS1::NS2
+
+namespace NS2 {
+struct Foo {
+int bar() { return -3; }
+};
+} // namespace NS2
+
 int
 main (int argc, char const *argv[])
 {
@@ -112,5 +136,9 @@
 A::B::test_lookup_at_nested_ns_scope_after_using();
 test_lookup_before_using_directive();
 test_lookup_after_using_directive();
-return Foo::myfunc(12);
+::B::Bar bb;
+A::B::Bar ab;
+void *bbp = &bb;
+return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
+   NS2::Foo{}.bar();
 }
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,10 @@
 
 self.expect("expression variadic_sum", patterns=[
 '\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
+self.expect_expr("::NS1::NS2::Foo{}.bar() == -2 && ::NS2::Foo{}.bar() == -3",
+ result_type="bool", result_value="true")
+self.expect_expr("NS2::Foo{}.bar() == -3", result_type="int", result_value="-3")
+self.expect_expr("((::B::Foo*)&bb).x()", result_type="int", result_value="42")
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
 
 CompilerDeclContext
 SymbolFileOnDemand::FindNamespace(ConstString name,
-  const CompilerDeclContext &parent_decl_ctx) {
+  const CompilerDeclContext &parent_decl_ctx,
+  bool only_root_namespaces) {
   if (!m_debug_info_enabled) {
 LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
  __FUNCTION__, name);
-return SymbolFile::FindNamespace(name, parent_decl_ctx);
+return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
   }
-  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx);
+  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx,
+only_root_namespaces);
 }
 
 std::vector>
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -157,9 +157,10 @@
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
-  lldb_private::CompilerDeclContext FindNamespace(
-  lldb_private::ConstString name,
-  const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+  lldb_private::CompilerDeclContext
+  FindNamespace(lldb_private::ConstString name,
+const lldb_private::CompilerDeclContext &parent_decl_ctx,
+bool only_root_namespaces) override;
 
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 
Index: lld

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 513539.
Michael137 added a comment.

- Move doxygen comment
- Add source comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147436

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolFileOnDemand.cpp
  lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/cpp/namespace/main.cpp

Index: lldb/test/API/lang/cpp/namespace/main.cpp
===
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,30 @@
 return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
 }
 
+namespace B {
+struct Bar {
+int x() { return 42; }
+};
+} // namespace B
+
+namespace A::B {
+struct Bar {
+int y() { return 137; }
+};
+} // namespace A::B
+
+namespace NS1::NS2 {
+struct Foo {
+int bar() { return -2; }
+};
+} // namespace NS1::NS2
+
+namespace NS2 {
+struct Foo {
+int bar() { return -3; }
+};
+} // namespace NS2
+
 int
 main (int argc, char const *argv[])
 {
@@ -112,5 +136,9 @@
 A::B::test_lookup_at_nested_ns_scope_after_using();
 test_lookup_before_using_directive();
 test_lookup_after_using_directive();
-return Foo::myfunc(12);
+::B::Bar bb;
+A::B::Bar ab;
+void *bbp = &bb;
+return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
+   NS2::Foo{}.bar();
 }
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,10 @@
 
 self.expect("expression variadic_sum", patterns=[
 '\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
+self.expect_expr("::NS1::NS2::Foo{}.bar() == -2 && ::NS2::Foo{}.bar() == -3",
+ result_type="bool", result_value="true")
+self.expect_expr("NS2::Foo{}.bar() == -3", result_type="int", result_value="-3")
+self.expect_expr("((::B::Foo*)&bb).x()", result_type="int", result_value="42")
Index: lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
===
--- lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
+++ lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
@@ -46,7 +46,7 @@
 
 self.expect("expr A::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A'"])
 self.expect("expr A::B::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A::B'"])
-self.expect("expr B::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A::B'"])
+self.expect("expr B::other_var", error=True, substrs=["use of undeclared identifier 'B'"])
 
 # 'frame variable' can correctly distinguish between A::B::global_var and A::global_var
 gvars = self.target().FindGlobalVariables("A::global_var", 10)
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
 
 CompilerDeclContext
 SymbolFileOnDemand::FindNamespace(ConstString name,
-  const CompilerDeclContext &parent_decl_ctx) {
+  const CompilerDeclContext &parent_decl_ctx,
+  bool only_root_namespaces) {
   if (!m_debug_info_enabled) {
 LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
  __FUNCTION__, name);
-return SymbolFile::FindNamespace(name, parent_decl_ctx);
+return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
   }
-  return 

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 513531.
Michael137 added a comment.

- Only search for root namespace if we're doing a qualified lookup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147436

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolFileOnDemand.cpp
  lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/cpp/namespace/main.cpp

Index: lldb/test/API/lang/cpp/namespace/main.cpp
===
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,18 @@
 return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
 }
 
+namespace B {
+struct Bar {
+int x() { return 42; }
+};
+} // namespace B
+
+namespace A::B {
+struct Bar {
+int y() { return 137; }
+};
+} // namespace A::B
+
 int
 main (int argc, char const *argv[])
 {
@@ -112,5 +124,7 @@
 A::B::test_lookup_at_nested_ns_scope_after_using();
 test_lookup_before_using_directive();
 test_lookup_after_using_directive();
-return Foo::myfunc(12);
+::B::Bar bb;
+A::B::Bar ab;
+return Foo::myfunc(12) + bb.x() + ab.y();
 }
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,6 @@
 
 self.expect("expression variadic_sum", patterns=[
 '\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
Index: lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
===
--- lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
+++ lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
@@ -46,7 +46,7 @@
 
 self.expect("expr A::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A'"])
 self.expect("expr A::B::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A::B'"])
-self.expect("expr B::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A::B'"])
+self.expect("expr B::other_var", error=True, substrs=["use of undeclared identifier 'B'"])
 
 # 'frame variable' can correctly distinguish between A::B::global_var and A::global_var
 gvars = self.target().FindGlobalVariables("A::global_var", 10)
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
 
 CompilerDeclContext
 SymbolFileOnDemand::FindNamespace(ConstString name,
-  const CompilerDeclContext &parent_decl_ctx) {
+  const CompilerDeclContext &parent_decl_ctx,
+  bool only_root_namespaces) {
   if (!m_debug_info_enabled) {
 LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
  __FUNCTION__, name);
-return SymbolFile::FindNamespace(name, parent_decl_ctx);
+return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
   }
-  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx);
+  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx,
+only_root_namespaces);
 }
 
 std::vector>
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -157,9 +157,10 @@
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
-  lldb_private::Compiler

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2212
 m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
-if (record_decl)
+if (record_decl) {
+  bool is_empty = true;

Generally I'm not sure if attaching a `clang::NoUniqueAddressAttr` to every 
empty field is the right approach. That goes slightly against our attempts to 
construct an AST that's faithful to the source to avoid unpredictable behaviour 
(which isn't always possible but for the most part we try). This approach was 
considered in https://reviews.llvm.org/D101237 but concern was raised about it 
affecting ABI, etc., leading to subtle issues down the line.

Based on the the discussion in https://reviews.llvm.org/D101237 it seemed to me 
like the only two viable solutions are:
1. Add a `DW_AT_byte_size` of `0` to the empty field
2. Add a `DW_AT_no_unique_address`

AFAICT Jan tried to implement (1) but never seemed to be able to fully add 
support for this in the ASTImporter/LLDB. Another issue I see with this is that 
sometimes the byte-size of said field is not `0`, depending on the context in 
which the structure is used.

I'm still leaning towards proposing a `DW_AT_no_unique_address`. Which is 
pretty easy to implement and also reason about from LLDB's perspective. 
@dblaikie @aprantl does that sound reasonable to you?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2219
+if (is_empty_field)
+  field->addAttr(clang::NoUniqueAddressAttr::Create(
+  m_ast.getASTContext(), clang::SourceRange()));

Typically the call to `record_decl->fields()` below would worry me, because if 
the decl `!hasLoadedFieldsFromExternalStorage()` then we'd start another 
`ASTImport` process, which could lead to some unpredictable behaviour if we are 
already in the middle of an import. But since 
`CompleteTagDeclarationDefinition` sets 
`setHasLoadedFieldsFromExternalStorage(true)` I *think* we'd be ok. Might 
warrant a comment.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2231
+if (is_empty)
+  record_decl->markEmpty();
+  }

Why do we need to mark the parents empty here again? Wouldn't they have been 
marked in `ParseStructureLikeDIE`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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