[Lldb-commits] [PATCH] D56170: RangeMap.h: merge RangeDataArray and RangeDataVector

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer accepted this revision.
tberghammer added a comment.

LGTM




Comment at: include/lldb/Core/RangeMap.h:636
 
-template  class RangeDataArray 
{
+template 
+class RangeDataVector {

Would it make sense to have a default value of 0 for N so people don't have to 
specify it explicitly?



Comment at: source/Plugins/Process/elf-core/ProcessElfCore.h:140
+  typedef lldb_private::RangeDataVector
   VMRangeToFileOffset;

Should this be 1 to keep the previous semantics?


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

https://reviews.llvm.org/D56170



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

[Lldb-commits] [PATCH] D54059: Remove Java debugger plugin

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added inline comments.



Comment at: tools/lldb-test/SystemInitializerTest.cpp:194
   SystemRuntimeMacOSX::Initialize();
   RenderScriptRuntime::Initialize();
-  JavaLanguageRuntime::Initialize();

davide wrote:
> Aside, do you know why we have renderscript support? maybe that should go 
> away too.
You should ask Stephen Hines (srhi...@google.com) about that one as he either 
know what is its status or know who to ask,


https://reviews.llvm.org/D54059



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

[Lldb-commits] [PATCH] D54060: Remove OCaml debugger plugin

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

I have no memory about the history/background of the OCaml plugin but no 
objection from my side.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54060



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

[Lldb-commits] [PATCH] D54059: Remove Java debugger plugin

[Tamas Berghammer via Phabricator via lldb-commits]

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

LGTM. Thank you for removing it.




Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:84
   Token = 19,
-  JavascriptData = 20,
   SystemMemoryInfo = 21,

This is JavaScript and not Java. Also I think this is a constant what is coming 
from the minidump spec so I would leave it even after we removed Java support 
from LLDB.



Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:155
   EDSP = (1 << 7),
-  Java = (1 << 8),
   IWMMXT = (1 << 9),

I think this is a constant what is coming from the minidump spec so I would 
leave it even after we removed Java support from LLDB


https://reviews.llvm.org/D54059



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

[Lldb-commits] [PATCH] D54057: Remove Go debugger plugin

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a reviewer: ribrdb.
tberghammer accepted this revision.
tberghammer added a comment.
This revision is now accepted and ready to land.

LGTM. Adding Ryan as a reviewer as he was the original implementer but I am not 
sure if his account (and the linked e-mail) is still active.


https://reviews.llvm.org/D54057



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

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer accepted this revision.
tberghammer added a comment.

Thank you for looking into this. The change looks good on Linux with both 
normal-dwarf, split-dwarf and mixed-dwarf (as well as with dwp) but can you (or 
somebody from Apple) please make sure it doesn't break MachO and/or dSym (I 
don't expect any issue but I know little about that implementation)?




Comment at: 
packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile:4
+C_SOURCES := a.c b.c
+CFLAGS_EXTRAS += -g -O0
+

I think these flags are specified by default in Makefile.rules



Comment at: 
packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile:10-12
+.PHONY: clean
+clean::
+   $(RM) -f a.dwo a.o b.o main

Do you need this? I think Makefile.rules should generate it for you and looking 
at that rule it should be correct.


Repository:
  rL LLVM

https://reviews.llvm.org/D42563



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

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

Thanks for the explanation, sorry I haven't read your commit message carefully.

In case of non-split-dwarf a die_offset is sufficient to uniquely identify a 
DIE because it is an offset from the beginning of the debug_info section (we 
assume we have at most 1 debug_info section everywhere in LLDB) and we are 
already relying on this as we don't store the compile unit offset in the 
non-split dwarf case. Looking at the code setting the cu_offset to 
DW_INVALID_OFFSET will be more correct then what we are doing at the moment 
because DWARFDebugInfo::GetDIE will call DWARFDebugInfo::GetCompileUnit and 
that function will look for a compile unit based on the cu_offset if cu_offset 
!= DW_INVALID_OFFSET what is currently the case so it will return an incorrect 
compile unit in the non-split-dwarf case. Setting cu_offset to 
DW_INVALID_OFFSET would solve the issue as it will look for a compile unit 
based on the DIE offset what is correct in the non-split dwarf case (I think 
this change of behavior is the one fixing your test case).

My only concern with setting the SymbolFileUID to DW_INVALID_OFFSET is how will 
it work in case of MachO (I have very little knowledge about that code path) 
but I really hope we don't implicitly depend on the offset being set to 0 
instead of DW_INVALID_OFFSET what is meant to represent unused/invalid values 
so I expect it to work.


Repository:
  rL LLVM

https://reviews.llvm.org/D42563



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

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

Sorry, I misunderstood your test-case in my first comment so what I wrote there 
doesn't make sense. When you are trying to debug binaries (executable or shared 
library) where half of it is compiled with split-dwarf while the other half is 
compiled with non-split-dwarf you are generally stepping into completely 
untested territory and that is a situation I haven't really thought about when 
designing this future.

The main issue is that split-dwarf and normal-dwarf uses a different invariant 
for the DIE UID where the non-split-dwarf case leaves the upper 32bit as 0 (and 
don't use it for anything) while the split-dwarf case uses it to find the 
correct dwo symbol file. I would prefer to see the fix somewhere inside 
SymbolFileDWARF or SymbolFileDWARFDwo instead and definitely don't want to see 
something ELF specific as nothing really ties SymbolFileDWARF with the ELF 
format.

Do you know who is using the partially incorrect value returned from here? The 
interesting part is that when we don't have split-dwarf then having 0 as the 
upper 32bit is fine but here obviously that is ambiguous because it can mean 
that we are not using split-dwarf or we are using split dwarf and we are 
referencing the compile unit at offset 0.

One possible idea what can provide a nice fix is to change 
SymbolFileDWARF::GetID() to return (DW_INVALID_OFFSET << 32) and then treat 
that value as a special case indicating that the compile unit offset is invalid 
and we should look for the DIE based on the die_offset part of the ID only. 
Changing that value might be more intrusive change but I don't expect too many 
thing to fail over and I think that would provide a cleaner invariant for 
fixing all sort of random issues coming up when mixing split-dwarf and 
non-split-dwarf in the same file (I expect quite a few bugs in this area).

Just changing SymbolFileDWARF::SymbolFileDWARF to call 
UserID(0xull) instead of UserID(0) fixes your test but will 
need more thoughts and tests to make sure it doesn't break other things (have 
to be tested at least on Linux and on OSX as they have different debug info 
formats)


Repository:
  rL LLVM

https://reviews.llvm.org/D42563



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

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer requested changes to this revision.
tberghammer added a comment.

Who is calling this method with a SimbolFileDWARF?

Instead of adding a hack to this function what looks horrible to me considering 
that different debug info formats are using the upper 32bit of the UID for 
different purposes can we change the caller to pass in the correct 
SymbolFileDWARFDwo instance instead?


Repository:
  rL LLVM

https://reviews.llvm.org/D42563



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

[Lldb-commits] [PATCH] D41909: Fix deadlock in dwarf logging

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

I think it isn't an A/B locking issue. The problem is that 2 different thread 
(main thread and 1 of the DWARF indexer thread) want to lock the mutex at the 
same time.

I have a mixed feeling about this change because introducing any sort of race 
condition (even if it is used only during logging) is problematic as it can 
read to undefined behavior and then crashes for example by reading and then 
dereferencing an  incorrect const char*. In this specific case I am pretty the 
code is actually thread safe on all architectures we care about (unless the 
compiler do something very crazy) because we read a set of ConstString's what 
is equivalent to reading a single pointer what I think is atomic on all 
architectures we care about (assuming it is aligned) and then we read data from 
the ConstString data pool what is read only and thread safe. My concern is that 
changing something fairly trivial (e.g. change a ConstString to an std::string 
inside FileSpec) can make this code not thread safe and introduce some new 
crashes so if possible I think we should try to keep the mutex here.

Why do we need to lock the Module mutex in SymbolVendor::FindFunctions? I think 
a better option would be to remove that lock and if it is needed then lock it 
just for the calls where it necessary. The fact that SymbolVendor locks a mutex 
inside a Module feels like a pretty bad layering violation for me what can 
cause many other deadlocks so it would be nice to fix that instead of hacking 
it around here.


https://reviews.llvm.org/D41909



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

[Lldb-commits] [PATCH] D39825: [lldb] Fix cu_offset for dwo/dwp used by DWARFCompileUnit::Index

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

In https://reviews.llvm.org/D39825#920739, @alexshap wrote:

> @tberghammer, SymbolFileDWARF (the base class of SymbolFileDWARFDwo) calls 
> Index()
>  "lazily" in may places, so indexing of dwo happens almost inevitably (at the 
> moment) 
>  (FindCompleteObjCDefinitionTypeForDIE  is just one example).


The main point is that you should almost always interact with the 
SymbolFileDWARF instance belonging to the main object file and it (actually 
DWARFCompileUnit) will know when to access data from the Dwo file instead. The 
original goal was to not leak pointers to SymbolFileDWARFDwo instances out from 
symbol file dwarf and the dwarf parsing logic.

>> because this way you will index the compile unit twice (once from the main 
>> object file and once from the dwo), 
>> then create 2 CompilerType for the 2 indexed version and will start hitting 
>> random issues in 
>> expression evaluation when clang will get confused by 2 declaration for the 
>> same type.
> 
> there are two separate CompileUnits here, not one,
>  There is a compile unit represented by m_base_dwarf_cu (roughly speaking for 
> .o) and and there is another one
>  (which we can get, for example, via SymbolFileDWARFDwo::GetCompileUnit()) 
> (roughly speaking for dwo).
>  (please, correct me if i'm wrong). For the latter GetOffset() would 
> typically return 0, 
>  for the former GetOffset() typically returns the higher 32 bits of Id.

We have 2 DWARFCompileUnit but actually they belong to the same "compile time 
compile unit" so when you call Index on the one in the main object file 
(SymbolFileDWARF instance) it will read the data in from the Dwo file as well, 
add that data into its index and create some stub types in clang (they will be 
filled in later lazily).


Repository:
  rL LLVM

https://reviews.llvm.org/D39825



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

[Lldb-commits] [PATCH] D39825: [lldb] Fix cu_offset for dwo/dwp used by DWARFCompileUnit::Index

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

I never tried debugging Objective-C using dwo but I am pretty sure this won't 
fix the issue you are seeing for FindCompleteObjCDefinitionTypeForDIE correctly 
because this way you will index the compile unit twice (once from the main 
object file and once from the dwo), then create 2 CompilerType for the 2 
indexed version and will start hitting random issues in expression evaluation 
when clang will get confused by 2 declaration for the same type. If I am not 
mistaken then because of this, calling Index on a Dwo file is a pretty bad 
idea. Instead of trying to get it work we should change it to be an assert so 
people don't call it by accident.

I am not sure if it makes sense to call FindCompleteObjCDefinitionTypeForDIE on 
a SymbolFileDWARFDwo because generally you want to do the type lookup in a full 
object file and not only in a single dwo. Do you have a full stack trace for a 
scenario where this happens? I suspect that the problem is at a higher layer 
then you are trying to fix it.

If the problem is actually with FindCompleteObjCDefinitionTypeForDIE then the 
correct solution would be to override it in SymbolFileDWARFDwo with "return 
GetBaseSymbolFile()->FindCompleteObjCDefinitionTypeForDIE(...)" as we are 
already doing it for a few methods.


Repository:
  rL LLVM

https://reviews.llvm.org/D39825



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

[Lldb-commits] [PATCH] D39825: [lldb] Fix cu_offset for dwo/dwp used by DWARFCompileUnit::Index

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

How are you end up calling SymbolFileDWARFDwo::Index? If I remember correctly 
you are not supposed to index a dwo file directly because without the main 
object file you won't have all of the necessary information.


Repository:
  rL LLVM

https://reviews.llvm.org/D39825



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

[Lldb-commits] [PATCH] D39545: Support scoped enums in the DWARF AST parser

[Tamas Berghammer via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317563: Support scoped enums in the DWARF AST parser 
(authored by tberghammer).

Repository:
  rL LLVM

https://reviews.llvm.org/D39545

Files:
  lldb/trunk/include/lldb/Symbol/ClangASTContext.h
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/trunk/source/Symbol/ClangASTContext.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -109,7 +109,7 @@
 m_ast.GetBuiltinTypeForEncodingAndBitSize(encoding, bytes * 8);
 
 CompilerType ast_enum = m_ast.CreateEnumerationType(
-name.c_str(), tu_decl_ctx, decl, builtin_type);
+name.c_str(), tu_decl_ctx, decl, builtin_type, false);
 auto enum_values = enum_type->findAllChildren();
 while (auto enum_value = enum_values->getNext()) {
   if (enum_value->getDataKind() != PDB_DataKind::Constant)
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -927,6 +927,7 @@
 // Set a bit that lets us know that we are currently parsing this
 dwarf->GetDIEToType()[die.GetDIE()] = DIE_IS_BEING_PARSED;
 
+bool is_scoped = false;
 DWARFFormValue encoding_form;
 
 const size_t num_attributes = die.GetAttributes(attributes);
@@ -963,6 +964,9 @@
   case DW_AT_declaration:
 is_forward_declaration = form_value.Boolean();
 break;
+  case DW_AT_enum_class:
+is_scoped = form_value.Boolean();
+break;
   case DW_AT_allocated:
   case DW_AT_associated:
   case DW_AT_bit_stride:
@@ -1052,7 +1056,7 @@
 
 clang_type = m_ast.CreateEnumerationType(
 type_name_cstr, GetClangDeclContextContainingDIE(die, nullptr),
-decl, enumerator_clang_type);
+decl, enumerator_clang_type, is_scoped);
   } else {
 enumerator_clang_type =
 m_ast.GetEnumerationIntegerType(clang_type.GetOpaqueQualType());
Index: lldb/trunk/source/Symbol/ClangASTContext.cpp
===
--- lldb/trunk/source/Symbol/ClangASTContext.cpp
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp
@@ -2169,20 +2169,20 @@
 CompilerType
 ClangASTContext::CreateEnumerationType(const char *name, DeclContext *decl_ctx,
const Declaration ,
-   const CompilerType _clang_type) {
+   const CompilerType _clang_type,
+   bool is_scoped) {
   // TODO: Do something intelligent with the Declaration object passed in
   // like maybe filling in the SourceLocation with it...
   ASTContext *ast = getASTContext();
 
   // TODO: ask about these...
-  //const bool IsScoped = false;
   //const bool IsFixed = false;
 
   EnumDecl *enum_decl = EnumDecl::Create(
   *ast, decl_ctx, SourceLocation(), SourceLocation(),
   name && name[0] ? >Idents.get(name) : nullptr, nullptr,
-  false,  // IsScoped
-  false,  // IsScopedUsingClassTag
+  is_scoped,
+  true,   // IsScopedUsingClassTag
   false); // IsFixed
 
   if (enum_decl) {
Index: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
===
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h
@@ -397,7 +397,8 @@
   CompilerType CreateEnumerationType(const char *name,
  clang::DeclContext *decl_ctx,
  const Declaration ,
- const CompilerType _qual_type);
+ const CompilerType _qual_type,
+ bool is_scoped);
 
   //--
   // Integer type functions
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
@@ -99,8 +99,8 @@
 # Look up information about the 'DayType' enum type.
   

[Lldb-commits] [PATCH] D39545: Support scoped enums in the DWARF AST parser

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a reviewer: jasonmolenda.
tberghammer added a subscriber: lldb-commits.
tberghammer added a comment.

This change is needed to make LLDB compatible with 
https://reviews.llvm.org/D39239


https://reviews.llvm.org/D39545



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

[Lldb-commits] [PATCH] D36046: Improve the posix core file triple detection

[Tamas Berghammer via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317411: Improve the posix core file triple detection 
(authored by tberghammer).

Repository:
  rL LLVM

https://reviews.llvm.org/D36046

Files:
  lldb/trunk/source/Core/ArchSpec.cpp
  lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp


Index: lldb/trunk/source/Core/ArchSpec.cpp
===
--- lldb/trunk/source/Core/ArchSpec.cpp
+++ lldb/trunk/source/Core/ArchSpec.cpp
@@ -921,6 +921,9 @@
 m_core = other.GetCore();
 CoreUpdated(true);
   }
+  if (GetFlags() == 0) {
+SetFlags(other.GetFlags());
+  }
 }
 
 bool ArchSpec::SetArchitecture(ArchitectureType arch_type, uint32_t cpu,
Index: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -724,15 +724,18 @@
 }
 
 ArchSpec ProcessElfCore::GetArchitecture() {
-  ObjectFileELF *core_file =
-  (ObjectFileELF *)(m_core_module_sp->GetObjectFile());
   ArchSpec arch;
-  core_file->GetArchitecture(arch);
+  m_core_module_sp->GetObjectFile()->GetArchitecture(arch);
 
   ArchSpec target_arch = GetTarget().GetArchitecture();
-  
-  if (target_arch.IsMIPS())
+  arch.MergeFrom(target_arch);
+
+  // On MIPS there is no way to differentiate betwenn 32bit and 64bit core 
files
+  // and this information can't be merged in from the target arch so we fail
+  // back to unconditionally returning the target arch in this config.
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }
 
   return arch;
 }


Index: lldb/trunk/source/Core/ArchSpec.cpp
===
--- lldb/trunk/source/Core/ArchSpec.cpp
+++ lldb/trunk/source/Core/ArchSpec.cpp
@@ -921,6 +921,9 @@
 m_core = other.GetCore();
 CoreUpdated(true);
   }
+  if (GetFlags() == 0) {
+SetFlags(other.GetFlags());
+  }
 }
 
 bool ArchSpec::SetArchitecture(ArchitectureType arch_type, uint32_t cpu,
Index: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -724,15 +724,18 @@
 }
 
 ArchSpec ProcessElfCore::GetArchitecture() {
-  ObjectFileELF *core_file =
-  (ObjectFileELF *)(m_core_module_sp->GetObjectFile());
   ArchSpec arch;
-  core_file->GetArchitecture(arch);
+  m_core_module_sp->GetObjectFile()->GetArchitecture(arch);
 
   ArchSpec target_arch = GetTarget().GetArchitecture();
-  
-  if (target_arch.IsMIPS())
+  arch.MergeFrom(target_arch);
+
+  // On MIPS there is no way to differentiate betwenn 32bit and 64bit core files
+  // and this information can't be merged in from the target arch so we fail
+  // back to unconditionally returning the target arch in this config.
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }
 
   return arch;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39283: [lldb-dev] Update LLDB test cases for 'inlineStepping'

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

Hi Carlos,

Sorry for not responding to your related e-mails lately. I am still not 
convinced that tis is the right way forward as you are changing the expected 
behavior from LLDB side to make it work with the new debug info emitted by 
clang but I think the current behavior of LLDB is correct in this context. Both 
your reasoning and your compiler fix makes perfect sense for the case you 
mentioned in the commit message (I assume you missed a "inline_value += 1;" 
line from the C code snippet based on the assembly) but I am still concerned by 
the behavior in the original case with the following source code:

  void caller_trivial_1() {
  caller_trivial_2(); // In caller_trivial_1.
  inline_value += 1; 
  }

I think the following debug info is emitted in 32bit mode after your change:

- debug_info: caller_trivial_1 starts at 0x4000
- debug_line: 0x4000 belongs to line X (declaration of caller_trivial_1) and it 
is prologue
- debug_info: inlined caller_trivial_2 starts at 0x4010
- debug_line: 0x4020 belongs to line Y (first line of function 
caller_trivial_1) and it is not prologue
- debug_info: inlined caller_trivial_2 ends at 0x4080

When we step into caller_trivial_1 lldb will step to the first non-prologue 
line entry after the start of f what will be at 0x4020 and when we stop there 
that address is within g so lldb displays that we are stopped in 
caller_trivial_2.

I think the correct debug info would be the following:

- debug_info: caller_trivial_1 starts at 0x4000
- debug_line: 0x4000 belongs to line X (declaration of caller_trivial_1) and it 
is prologue
- debug_line: 0x4010 belongs to line Y (first line of function 
caller_trivial_1) and it is not prologue
- debug_info: inlined caller_trivial_2 starts at 0x4010
- debug_line: 0x4020 belongs to line Y (first line of function 
caller_trivial_2) and it is not prologue
- debug_info: inlined caller_trivial_2 ends at 0x4080

**In case of non-optimized build (it is true for the test) I would expect from 
a compiler that no line entry pointing to a line inside caller_trivial_1 will 
have an address what is inside the address range of any other function inlined 
into caller_trivial_2. I think this held before your change but not after.** 
Can you and others comment on this last expectation? If we agree that this 
expectation should hold then I think changing the test is the wrong and instead 
we should either change the line entry or the inlined function range writing 
code to produce debug info satisfying it.


https://reviews.llvm.org/D39283



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

[Lldb-commits] [PATCH] D38568: [lldb] Enable using out-of-tree dwps

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer accepted this revision.
tberghammer added a comment.

Looks good. Thanks for fixing it.


Repository:
  rL LLVM

https://reviews.llvm.org/D38568



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

[Lldb-commits] [PATCH] D38568: [lldb] Enable using out-of-tree dwps

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added inline comments.



Comment at: source/Host/common/Symbols.cpp:288-294
+  // FIXME: at the moment llvm-dwp doesn't output build ids,
+  // nor does binutils dwp. Thus in the case of DWPs
+  // we skip uuids check. This needs to be fixed
+  // to avoid consistency issues as soon as
+  // llvm-dwp and binutils dwp gain support for build ids.
+  if (file_spec.GetFileNameExtension().GetStringRef() == "dwp" ||
+  mspec.GetUUID() == module_uuid)

What do you think about adding a new argument to 
Symbols::LocateExecutableSymbolFile (with a potential default value) to specify 
if we want to check the UUID and then move this logic to the place where we are 
looking for the dwp file? I think that would make dwp specific logic more 
concise in one place.


Repository:
  rL LLVM

https://reviews.llvm.org/D38568



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

[Lldb-commits] [PATCH] D38492: [lldb] Fix initialization of m_debug_cu_index_map

[Tamas Berghammer via Phabricator via lldb-commits]

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

Looks good, thanks for fixing it.

I am slightly confused why it was working for me when I tested it before 
submission but I must have messed up something during testing.


Repository:
  rL LLVM

https://reviews.llvm.org/D38492



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

[Lldb-commits] [PATCH] D37930: Use ThreadLauncher to launch TaskPool threads

[Tamas Berghammer via Phabricator via lldb-commits]

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

I don't really have an opinion if moving TaskPool to Host is a good or bad idea 
(haven't followed the recent layering efforts) but other then that looks good. 
I also tested it on Linux and it works fine.


https://reviews.llvm.org/D37930



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

[Lldb-commits] [PATCH] D37930: Use ThreadLauncher to launch TaskPool threads

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer accepted this revision.
tberghammer added a comment.

Looks good




Comment at: source/Utility/TaskPool.cpp:61
+lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
+   this, nullptr, 8 * 1024 * 1024)
+.Release();

(nit): Can you create a named constant for the min stack size as just based on 
this line I would have no idea what does that number do?


https://reviews.llvm.org/D37930



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

[Lldb-commits] [PATCH] D36046: Improve the posix core file triple detection

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

Hi Greg, can you take a look sometime? Thanks, Tamas




Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:733-735
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }

nitesh.jain wrote:
> tberghammer wrote:
> > Hi Nitesh,
> > 
> > I tried to remove this MIPS specific code as it shouldn't be necessary if I 
> > add the above MergeFrom for all architecture but if I do it fails LinuxCore
> > TestCase.test_mips_n32.
> > 
> > The issue is that in that case the tripe we get from the core file is 
> > "mipsel--" ("mipsel--linux" after the merge) and the one we get from the 
> > binary is "mips64el--linux". Is it normal to have a seemingly 32bit core 
> > file with a 64bit binary on mips? If not then can I ask you to help me 
> > figure out which one is incorrect?
> > 
> > If it is then I don't see any better short term solution then leaving this 
> > condition here but it feels like a quite big hack what might backfire when 
> > somebody tries to use a core file with a completely incompatible binary.
> > 
> > Thanks,
> > Tamas
> Hi Tamas,
> 
> In MIPS , the core file doesn't contain any Architecture information . It's 
> just specify the generic MIPS architecture. Hence we need to relied on 
> target_arch for correct architecture informations.
> 
> TestCase.test_mips_n32 is "32 bit binary" with triple "mips64el-linux". This 
> binary can only run on 64 bit target as compared to O32 which can run on 
> 32/64 bit target.
Thank you for the explanation. I added a comment to the code to explain it 
better there.


https://reviews.llvm.org/D36046



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

[Lldb-commits] [PATCH] D36046: Improve the posix core file triple detection

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer updated this revision to Diff 111326.
tberghammer added a comment.

Add comment about the MIPS special case.


https://reviews.llvm.org/D36046

Files:
  source/Core/ArchSpec.cpp
  source/Plugins/Process/elf-core/ProcessElfCore.cpp


Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -724,15 +724,18 @@
 }
 
 ArchSpec ProcessElfCore::GetArchitecture() {
-  ObjectFileELF *core_file =
-  (ObjectFileELF *)(m_core_module_sp->GetObjectFile());
   ArchSpec arch;
-  core_file->GetArchitecture(arch);
+  m_core_module_sp->GetObjectFile()->GetArchitecture(arch);
 
   ArchSpec target_arch = GetTarget().GetArchitecture();
-  
-  if (target_arch.IsMIPS())
+  arch.MergeFrom(target_arch);
+
+  // On MIPS there is no way to differentiate betwenn 32bit and 64bit core 
files
+  // and this information can't be merged in from the target arch so we fail
+  // back to unconditionally returning the target arch in this config.
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }
 
   return arch;
 }
Index: source/Core/ArchSpec.cpp
===
--- source/Core/ArchSpec.cpp
+++ source/Core/ArchSpec.cpp
@@ -1002,6 +1002,9 @@
 m_core = other.GetCore();
 CoreUpdated(true);
   }
+  if (GetFlags() == 0) {
+SetFlags(other.GetFlags());
+  }
 }
 
 bool ArchSpec::SetArchitecture(ArchitectureType arch_type, uint32_t cpu,


Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -724,15 +724,18 @@
 }
 
 ArchSpec ProcessElfCore::GetArchitecture() {
-  ObjectFileELF *core_file =
-  (ObjectFileELF *)(m_core_module_sp->GetObjectFile());
   ArchSpec arch;
-  core_file->GetArchitecture(arch);
+  m_core_module_sp->GetObjectFile()->GetArchitecture(arch);
 
   ArchSpec target_arch = GetTarget().GetArchitecture();
-  
-  if (target_arch.IsMIPS())
+  arch.MergeFrom(target_arch);
+
+  // On MIPS there is no way to differentiate betwenn 32bit and 64bit core files
+  // and this information can't be merged in from the target arch so we fail
+  // back to unconditionally returning the target arch in this config.
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }
 
   return arch;
 }
Index: source/Core/ArchSpec.cpp
===
--- source/Core/ArchSpec.cpp
+++ source/Core/ArchSpec.cpp
@@ -1002,6 +1002,9 @@
 m_core = other.GetCore();
 CoreUpdated(true);
   }
+  if (GetFlags() == 0) {
+SetFlags(other.GetFlags());
+  }
 }
 
 bool ArchSpec::SetArchitecture(ArchitectureType arch_type, uint32_t cpu,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D11465: Fix "process load/unload" on android

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

Thanks for all of the data. Based on this I think you are using a preview 
version of android O what reports SDK 25 but the linker works the way an SDK 26 
system linker would do.

I think the proper fix for the problem is to do something like what Greg 
suggested to detect the correct function name based on the list of available 
symbols instead of based on the SDK version. I will try to create a CL to fix 
it in the next couple of days (unless you are happy to do it), but not sure 
when I will have time for that and it is a bit tricky to test it.

Until then you can try to do either update to a newer version of the system 
image what reports SDK 26 (and make sure you are using a sufficiently new LLDB) 
or locally hack the "PlatformAndroid::LoadImage" function to use the SDK 26 
version in case of SDK 25 as well (possibly with a check for 
ro.build.version.release)


Repository:
  rL LLVM

https://reviews.llvm.org/D11465



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

[Lldb-commits] [PATCH] D36068: Add support for base address entries in the .debug_ranges section

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer created this revision.

Clang recently started to emit base address entries into the
.debug_ranges section to reduce the number of relocations needed. Lets
make sure LLDB can read them.


https://reviews.llvm.org/D36068

Files:
  source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp


Index: source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
@@ -15,6 +15,18 @@
 using namespace lldb_private;
 using namespace std;
 
+static dw_addr_t GetBaseAddressMarker(uint32_t addr_size) {
+  switch(addr_size) {
+case 2:
+  return 0x;
+case 4:
+  return 0x;
+case 8:
+  return 0x;
+  }
+  llvm_unreachable("GetBaseAddressMarker unsupported address size.");
+}
+
 DWARFDebugRanges::DWARFDebugRanges() : m_range_map() {}
 
 DWARFDebugRanges::~DWARFDebugRanges() {}
@@ -39,38 +51,27 @@
   const DWARFDataExtractor _ranges_data =
   dwarf2Data->get_debug_ranges_data();
   uint32_t addr_size = debug_ranges_data.GetAddressByteSize();
+  dw_addr_t base_addr = 0;
+  dw_addr_t base_addr_marker = GetBaseAddressMarker(addr_size);
 
   while (
   debug_ranges_data.ValidOffsetForDataOfSize(*offset_ptr, 2 * addr_size)) {
 dw_addr_t begin = debug_ranges_data.GetMaxU64(offset_ptr, addr_size);
 dw_addr_t end = debug_ranges_data.GetMaxU64(offset_ptr, addr_size);
+
 if (!begin && !end) {
   // End of range list
   break;
 }
-// Extend 4 byte addresses that consists of 32 bits of 1's to be 64 bits
-// of ones
-switch (addr_size) {
-case 2:
-  if (begin == 0xull)
-begin = LLDB_INVALID_ADDRESS;
-  break;
-
-case 4:
-  if (begin == 0xull)
-begin = LLDB_INVALID_ADDRESS;
-  break;
 
-case 8:
-  break;
-
-default:
-  llvm_unreachable("DWARFRangeList::Extract() unsupported address size.");
+if (begin == base_addr_marker) {
+  base_addr = end;
+  continue;
 }
 
 // Filter out empty ranges
 if (begin < end)
-  range_list.Append(DWARFRangeList::Entry(begin, end - begin));
+  range_list.Append(DWARFRangeList::Entry(begin + base_addr, end - begin));
   }
 
   // Make sure we consumed at least something


Index: source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
@@ -15,6 +15,18 @@
 using namespace lldb_private;
 using namespace std;
 
+static dw_addr_t GetBaseAddressMarker(uint32_t addr_size) {
+  switch(addr_size) {
+case 2:
+  return 0x;
+case 4:
+  return 0x;
+case 8:
+  return 0x;
+  }
+  llvm_unreachable("GetBaseAddressMarker unsupported address size.");
+}
+
 DWARFDebugRanges::DWARFDebugRanges() : m_range_map() {}
 
 DWARFDebugRanges::~DWARFDebugRanges() {}
@@ -39,38 +51,27 @@
   const DWARFDataExtractor _ranges_data =
   dwarf2Data->get_debug_ranges_data();
   uint32_t addr_size = debug_ranges_data.GetAddressByteSize();
+  dw_addr_t base_addr = 0;
+  dw_addr_t base_addr_marker = GetBaseAddressMarker(addr_size);
 
   while (
   debug_ranges_data.ValidOffsetForDataOfSize(*offset_ptr, 2 * addr_size)) {
 dw_addr_t begin = debug_ranges_data.GetMaxU64(offset_ptr, addr_size);
 dw_addr_t end = debug_ranges_data.GetMaxU64(offset_ptr, addr_size);
+
 if (!begin && !end) {
   // End of range list
   break;
 }
-// Extend 4 byte addresses that consists of 32 bits of 1's to be 64 bits
-// of ones
-switch (addr_size) {
-case 2:
-  if (begin == 0xull)
-begin = LLDB_INVALID_ADDRESS;
-  break;
-
-case 4:
-  if (begin == 0xull)
-begin = LLDB_INVALID_ADDRESS;
-  break;
 
-case 8:
-  break;
-
-default:
-  llvm_unreachable("DWARFRangeList::Extract() unsupported address size.");
+if (begin == base_addr_marker) {
+  base_addr = end;
+  continue;
 }
 
 // Filter out empty ranges
 if (begin < end)
-  range_list.Append(DWARFRangeList::Entry(begin, end - begin));
+  range_list.Append(DWARFRangeList::Entry(begin + base_addr, end - begin));
   }
 
   // Make sure we consumed at least something
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D36046: Improve the posix core file triple detection

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added inline comments.



Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:733-735
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }

Hi Nitesh,

I tried to remove this MIPS specific code as it shouldn't be necessary if I add 
the above MergeFrom for all architecture but if I do it fails LinuxCore
TestCase.test_mips_n32.

The issue is that in that case the tripe we get from the core file is 
"mipsel--" ("mipsel--linux" after the merge) and the one we get from the binary 
is "mips64el--linux". Is it normal to have a seemingly 32bit core file with a 
64bit binary on mips? If not then can I ask you to help me figure out which one 
is incorrect?

If it is then I don't see any better short term solution then leaving this 
condition here but it feels like a quite big hack what might backfire when 
somebody tries to use a core file with a completely incompatible binary.

Thanks,
Tamas


https://reviews.llvm.org/D36046



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

[Lldb-commits] [PATCH] D36046: Improve the posix core file triple detection

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer created this revision.
Herald added subscribers: kristof.beyls, arichardson, sdardis, aemerson.

Posix core files sometime don't contain enough information to correctly
detect the OS. If that is the case we should use the OS from the target
instead as it will contain usable information in more cases and if the
target and the core contain different OS-es then we are already in a
pretty bad state so moving from an unknown OS to a known (but possibly
incorrect) OS will do no harm.

We already had similar code in place for MIPS. This change tries to make
it more generic by using ArchSpec::MergeFrom and extends it to all
architectures but some MIPS specific issue prevent us from getting rid
of special casing MIPS.


https://reviews.llvm.org/D36046

Files:
  source/Core/ArchSpec.cpp
  source/Plugins/Process/elf-core/ProcessElfCore.cpp


Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -724,15 +724,15 @@
 }
 
 ArchSpec ProcessElfCore::GetArchitecture() {
-  ObjectFileELF *core_file =
-  (ObjectFileELF *)(m_core_module_sp->GetObjectFile());
   ArchSpec arch;
-  core_file->GetArchitecture(arch);
+  m_core_module_sp->GetObjectFile()->GetArchitecture(arch);
 
   ArchSpec target_arch = GetTarget().GetArchitecture();
-  
-  if (target_arch.IsMIPS())
+  arch.MergeFrom(target_arch);
+
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }
 
   return arch;
 }
Index: source/Core/ArchSpec.cpp
===
--- source/Core/ArchSpec.cpp
+++ source/Core/ArchSpec.cpp
@@ -1002,6 +1002,9 @@
 m_core = other.GetCore();
 CoreUpdated(true);
   }
+  if (GetFlags() == 0) {
+SetFlags(other.GetFlags());
+  }
 }
 
 bool ArchSpec::SetArchitecture(ArchitectureType arch_type, uint32_t cpu,


Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -724,15 +724,15 @@
 }
 
 ArchSpec ProcessElfCore::GetArchitecture() {
-  ObjectFileELF *core_file =
-  (ObjectFileELF *)(m_core_module_sp->GetObjectFile());
   ArchSpec arch;
-  core_file->GetArchitecture(arch);
+  m_core_module_sp->GetObjectFile()->GetArchitecture(arch);
 
   ArchSpec target_arch = GetTarget().GetArchitecture();
-  
-  if (target_arch.IsMIPS())
+  arch.MergeFrom(target_arch);
+
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }
 
   return arch;
 }
Index: source/Core/ArchSpec.cpp
===
--- source/Core/ArchSpec.cpp
+++ source/Core/ArchSpec.cpp
@@ -1002,6 +1002,9 @@
 m_core = other.GetCore();
 CoreUpdated(true);
   }
+  if (GetFlags() == 0) {
+SetFlags(other.GetFlags());
+  }
 }
 
 bool ArchSpec::SetArchitecture(ArchitectureType arch_type, uint32_t cpu,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D11465: Fix "process load/unload" on android

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

Hi Nitesh,

The way dlopen, dlclose and dl_phdr_iterate is implemented on android is pretty 
strange until (and including) SDK 25. The trick is that these functions are 
implemented inside "/system/bin/linker" with actual function names prefixed 
with "__dl_". The linker is doing some magic (creating a fake shared library 
entry at startup) to resolve these symbols when a library is loaded. This magic 
isn't understand by LLDB so it tries to lookup the symbols in the linker binary 
itself based on there actual function names. The actual libdl.so file located 
on the device isn't used for anything (it is there just for some compatibility 
reasons).

To help me investigate, can you do the followings and attach the output to this 
thread:

- Run "target modules list" in LLDB
- Run "target modules lookup -n dlopen" in LLDB
- Run "target modules lookup -n __dl_dlopen" in LLDB
- Download "/system/bin/linker" from the device and dump the ".symtab" section 
of it

Random guesses for what can cause the problem (will be able to more specific 
based on the above data):

- LLDB fails to detect or load "/system/bin/linker" in the shared library list
- /system/bin/linker doesn't contain a ".symtab" section (e.g. stripped)
- You are using a version of android what have an API 26 /system/bin/linker 
what had some changes breaking LLDB (Pavel fixed it but the fix only applies 
for API 26+)


Repository:
  rL LLVM

https://reviews.llvm.org/D11465



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

[Lldb-commits] [PATCH] D35734: Don't allow LLDB to try and parse .debug_types

[Tamas Berghammer via Phabricator via lldb-commits]

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

Can you file a bug about this issue so we can keep track of it? Also it would 
be nice to include a small test case in the bug (if you have one) what 
demonstrates the crash as so far I only managed to see missing type information 
without an actual LLDB crash.




Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:513
+  return 0;
+}
+

clayborg wrote:
> That patch would be a lot more intrusive as we would still need to be able to 
> parse all compile unit DIEs just so we can get to the line tables. The only 
> way to find the line table for a compile unit is to follow the 
> DW_AT_stmt_list attribute in each top level compile unit DIE. Also, DWARF is 
> useless to us unless we index the DWARF, which means parsing everything in 
> all DIEs. I would like to keep this as simple as possible to just avoid the 
> issue for now. This make it easy to cherry pick this where needed for anyone 
> requiring this patch.
You are right.

I thought it is possible to use debug_lines without any information from the 
debug_info section but it is more complicated then that. In this case it would 
be almost as complicated to skip the problematic entries as to teach LLDB how 
to handle them.


https://reviews.llvm.org/D35734



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

[Lldb-commits] [PATCH] D35734: Don't allow LLDB to try and parse .debug_types

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

This section have been already removed from Dwarf5 so I agree that we shouldn't 
spend too much time adding support for it. Do you know anybody hitting this 
issue? Do you know why they decided to use this flag?




Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:500-513
+// On non Apple platforms we might have .debug_types debug info that
+// is created by using "-fdebug-types". LLDB currently will try to
+// load this debug info, but it causes crashes during debugging when
+// types are missing since it doesn't know how to parse the info in
+// the .debug_types type units. This causes all complex debug info
+// types to be unresolved. Because this causes LLDB to crash and since
+// it really doesn't provide a solid debuggiung experience, we should

Can we make this disabling logic a bit more fine grained? What I am thinking 
about is to disable parsing .debug_info and .debug_types but still parse the 
line table as that shouldn't cause any issue.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:501
+// On non Apple platforms we might have .debug_types debug info that
+// is created by using "-fdebug-types". LLDB currently will try to
+// load this debug info, but it causes crashes during debugging when

I think the name of the flag is "-fdebug-types-section" but it might be 
platform dependent.


https://reviews.llvm.org/D35734



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

[Lldb-commits] [PATCH] D34750: [UnwindAssembly/x86] Add support for "lea imm(%ebp), %esp" pattern

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added inline comments.



Comment at: 
source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp:875-876
+ row->GetCFAValue().GetRegisterNumber() == m_lldb_fp_regnum) {
+  current_sp_bytes_offset_from_cfa =
+  row->GetCFAValue().GetOffset() - stack_offset;
+}

labath wrote:
> tberghammer wrote:
> > Shouldn't you change the unwind information for the CFA here? For me saying 
> > CFA=rbp seems like an incorrect thing to do, but not sure what would be the 
> > correct value (Undefined? IsSame?). The impact is if an other register (or 
> > a local variable) have a location specified as CFA+off then after this 
> > instruction it will point to bogus location.
> I think there has been some misunderstanding, as the your comment makes no 
> sense to me. :)
> 
> This code only fires if CFA=rbp+offset, and that remains valid even after 
> this instruction -- `lea` does not change the value of the rbp register, so 
> any register rule that was valid before this instruction will remain valid 
> after it. This only begins to make a difference after we process the `pop 
> %rbp` instruction -- then we will update the CFA rule to read 
> `CFA=rsp+current_sp_bytes_offset_from_cfa`.
You are right, please ignore my comment. I somehow assumed the `lea` 
instruction will change the value of `rbp` as well not just `rsp`.


https://reviews.llvm.org/D34750



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

[Lldb-commits] [PATCH] D34750: [UnwindAssembly/x86] Add support for "lea imm(%ebp), %esp" pattern

[Tamas Berghammer via Phabricator via lldb-commits]

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

Looks good with one possible question




Comment at: 
source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp:875-876
+ row->GetCFAValue().GetRegisterNumber() == m_lldb_fp_regnum) {
+  current_sp_bytes_offset_from_cfa =
+  row->GetCFAValue().GetOffset() - stack_offset;
+}

Shouldn't you change the unwind information for the CFA here? For me saying 
CFA=rbp seems like an incorrect thing to do, but not sure what would be the 
correct value (Undefined? IsSame?). The impact is if an other register (or a 
local variable) have a location specified as CFA+off then after this 
instruction it will point to bogus location.


https://reviews.llvm.org/D34750



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

[Lldb-commits] [PATCH] D34352: [linux] Change the way we load vdso pseudo-module

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer accepted this revision.
tberghammer added a comment.

Looks good


https://reviews.llvm.org/D34352



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

[Lldb-commits] [PATCH] D34199: Tweak SysV_arm64 function entry unwind plan

[Tamas Berghammer via Phabricator via lldb-commits]

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

Looks good


https://reviews.llvm.org/D34199



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

[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added inline comments.



Comment at: unittests/tools/lldb-server/inferior/thread_inferior.cpp:22
+
+  bool delay = true;
+  std::vector threads;

You have to make this variable atomic (or add a mutex) to make it work. In the 
current implementation there is no guarantee that the new threads will ever see 
the updated value as (in theory) the compiler can optimize away the repeated 
read.



Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:54
+
+ProcessInfo::ProcessInfo() {}
+

(nit): You can use "ProcessInfo() = default;" in the header file (here and in 
every other empty constructor/destructor)



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:33-42
+  lldb::pid_t pid;
+  lldb::pid_t parent_pid;
+  uint32_t real_uid;
+  uint32_t real_gid;
+  uint32_t effective_uid;
+  uint32_t effective_gid;
+  std::string triple;

A large part of these variables are never read by anybody. Do we want to keep 
them around just in case or should we remove them?


https://reviews.llvm.org/D32930



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

[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added inline comments.



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:83
+// Common functions for parsing packet data.
+std::unordered_map SplitPairList(const std::string& 
s);
+std::vector SplitList(const std::string& s, char delimeter);

jmajors wrote:
> zturner wrote:
> > tberghammer wrote:
> > > What if the key isn't unique?
> > Return an `llvm::StringMap` here.  Also the argument should be a 
> > `StringRef`.
> I was working on the assumption that the keys in lldb response packets would 
> be unique, and that it would map a key to a list of values if it needed to 
> have a one:many relationship. Are there any that have duplicate keys?
I think in an lldb-server test we should make as little assumption about 
lldb-server as possible. I am not aware of any packet where duplicated keys are 
allowed but if we want to rely on it I think we should add a test expectation 
verifying this as having repeated keys would mean lldb-server is buggy (what is 
exactly what we want to catch here)



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:41
+private:
+  std::string source;
+};

The LLDB coding convention is to prefix member variables with "m_". Do we want 
to follow that one here or should we follow the LLVM one what is CapitalCase 
(you are following neither of them at the moment)?



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:52
+
+  unsigned long ReadRegister(unsigned long register_id) const;
+

"unsigned long" is only 32 bit on some systems but a register can be arbitrary 
large so the return value should be something more generic. This is true for 
every location you are working with registers.



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:63
+ public:
+  static llvm::Expected Create(
+  llvm::StringRef response, llvm::support::endianness endian);

Why do we need the unique_ptr here? Can it return llvm::Expected 
instead? Same question for the other Create functions.


https://reviews.llvm.org/D32930



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

[Lldb-commits] [PATCH] D32813: ABISysV_arm64: compute return value for large vectors correctly

[Tamas Berghammer via Phabricator via lldb-commits]

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

Makes sense. Thank you for the explanation (I assumed homogeneous aggregate and 
vector are the same).


https://reviews.llvm.org/D32813



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

[Lldb-commits] [PATCH] D32813: ABISysV_arm64: compute return value for large vectors correctly

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

I am a bit confused by the correlation between your change and commit message. 
In the commit message you say that 32 byte structs are passed as x8 pointers 
but the implementation of LoadValueFromConsecutiveGPRRegisters seems to read it 
out from the v0-v8 registers for vectors of up to 8 elements independently of 
there size. Also based on that code I have the suspicion that the first branch 
(where byte_size <= 16) is not actually used or necessary and also I don't see 
anything in the ABI documentation indicating otherwise (it would be a pretty 
crazy ABI if they say that if you have 4 double then passed in a single 32 byte 
register while if you have 8 double then passed in 8 different 32 byte 
registers). Can you make sure that branch is necessary (e.g. removing it breaks 
at least 1 test)?




Comment at: 
packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py:194
+exe = os.path.join(os.getcwd(), "a.out")
+error = lldb.SBError()
+

(nit): Not needed


https://reviews.llvm.org/D32813



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

[Lldb-commits] [PATCH] D32757: Add TaskMap for iterating a function over a set of integers

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

Personally I never really liked TaskRunner (even though I was the one 
implemented it) because I think it adds a lot of extra complexity for fairly 
little gain and thinking about it a bit more I think in most use case a more 
targeted solution at the call site will probably give better results. Also if 
we need it in the future it can always be restored based on git/svn history. 
Based on that I am happy to delete it if we have no use case for it at the 
moment.

Regarding Apple accelerator tables, giving it a try can be interesting (pass 
'-glldb' to a sufficiently new clang) but as far as I know they are not 
compatible with split dwarf what can be show stopper for very large 
applications (e.g. linking chromium on linux with "no-limit-debug-info" and 
without split dwarf requires unreasonably large amount of memory).


Repository:
  rL LLVM

https://reviews.llvm.org/D32757



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

[Lldb-commits] [PATCH] D32503: Remove unused code related to CPlusPlusLanguage::FindEquivalentNames

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

I am fully support deleting it (and pretty much any unused code). If we need it 
in the future then we will still have it in the svn history.


Repository:
  rL LLVM

https://reviews.llvm.org/D32503



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

[Lldb-commits] [PATCH] D32503: Remove unused code related to CPlusPlusLanguage::FindEquivalentNames

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

I looked into the history of this code once and my understanding is that Enrico 
added this code in 
https://github.com/llvm-mirror/lldb/commit/bad9753828b6e0e415e38094bb9627e41d57874c
 but it have never been used (at least in upstream). The original commit 
message also indicates this.


Repository:
  rL LLVM

https://reviews.llvm.org/D32503



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

[Lldb-commits] [PATCH] D32441: Remove the home-grown android toolchain file and all references to it

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer accepted this revision.
tberghammer added a comment.

Looks good


https://reviews.llvm.org/D32441



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

[Lldb-commits] [PATCH] D32441: Remove the home-grown android toolchain file and all references to it

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added inline comments.



Comment at: www/build.html:474-475
 
-  from inside the unzipped NDK. Toolchains for other architectures 
can be produced in
-  a similar manner.
+  The NDK also contains a cmake toolchain file, which makes 
configuring the build much simpler.
+  The compiler, include and library paths will be configured by the
+  toolchain file and all you need to do is to select the

Can you mention the minimum version of the NDK supported by this approach?



Comment at: www/build.html:488-493
+  
-DCMAKE_TOOLCHAIN_FILE=$ANDROID_NDK_HOME/build/cmake/android.toolchain.cmake 
\
+  -DANDROID_ABI=arm64-v8a \
+  -DANDROID_PLATFORM=android-21 \
+  -DANDROID_ALLOW_UNDEFINED_SYMBOLS=On \
+  -DLLVM_HOST_TRIPLE=aarch64-unknown-linux-android \
+  
-DCROSS_TOOLCHAIN_FLAGS_NATIVE='-DCMAKE_C_COMPILER=cc;-DCMAKE_CXX_COMPILER=c++' 


Can you add some instructions about the supported ANDROID_ABI and 
LLVM_HOST_TRIPLE pairs? I would find it a bit hard to figure them out on my own 
(especially the correct triple)



Comment at: www/build.html:488-493
+  
-DCMAKE_TOOLCHAIN_FILE=$ANDROID_NDK_HOME/build/cmake/android.toolchain.cmake 
\
+  -DANDROID_ABI=arm64-v8a \
+  -DANDROID_PLATFORM=android-21 \
+  -DANDROID_ALLOW_UNDEFINED_SYMBOLS=On \
+  -DLLVM_HOST_TRIPLE=aarch64-unknown-linux-android \
+  
-DCROSS_TOOLCHAIN_FLAGS_NATIVE='-DCMAKE_C_COMPILER=cc;-DCMAKE_CXX_COMPILER=c++' 


tberghammer wrote:
> Can you add some instructions about the supported ANDROID_ABI and 
> LLVM_HOST_TRIPLE pairs? I would find it a bit hard to figure them out on my 
> own (especially the correct triple)
Shouldn't we specify LLVM_TARGETS_TO_BUILD as well to reduce the size of the 
executable?



Comment at: www/build.html:488-493
+  
-DCMAKE_TOOLCHAIN_FILE=$ANDROID_NDK_HOME/build/cmake/android.toolchain.cmake 
\
+  -DANDROID_ABI=arm64-v8a \
+  -DANDROID_PLATFORM=android-21 \
+  -DANDROID_ALLOW_UNDEFINED_SYMBOLS=On \
+  -DLLVM_HOST_TRIPLE=aarch64-unknown-linux-android \
+  
-DCROSS_TOOLCHAIN_FLAGS_NATIVE='-DCMAKE_C_COMPILER=cc;-DCMAKE_CXX_COMPILER=c++' 


tberghammer wrote:
> tberghammer wrote:
> > Can you add some instructions about the supported ANDROID_ABI and 
> > LLVM_HOST_TRIPLE pairs? I would find it a bit hard to figure them out on my 
> > own (especially the correct triple)
> Shouldn't we specify LLVM_TARGETS_TO_BUILD as well to reduce the size of the 
> executable?
Previously we specified LLVM_TARGET_ARCH as well. Is it not needed anymore?



Comment at: www/build.html:498
+  Note that currently only lldb-server is functional on android. 
The
+  lldb client is supported and unlikely to work.
 

is *not* supported


https://reviews.llvm.org/D32441



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

[Lldb-commits] [PATCH] D31882: Fix the libc++ std::vector data formatter

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer abandoned this revision.
tberghammer added a comment.

Pavel created a separate fix as https://reviews.llvm.org/D31880. Abandoning 
this one.


https://reviews.llvm.org/D31882



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

[Lldb-commits] [PATCH] D31880: Fix libc++ vector data formatter (bug #32553)

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

The previous version of the data formatter was triggering for 
std::vector as well. Jason, do you know why was it the 
case? Do we need that functionality because of a broken compiler version or can 
it be removed?

Note: I added a few comments about the data formatter itself but feel free to 
ignore them if you want to keep this change small.




Comment at: source/Plugins/Language/CPlusPlus/LibCxxVector.cpp:52-53
+  LibcxxVectorBoolSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
+
+  ~LibcxxVectorBoolSyntheticFrontEnd() override = default;
+

Not needed



Comment at: source/Plugins/Language/CPlusPlus/LibCxxVector.cpp:201-228
+  switch (bit_index) {
+  case 0:
+mask = 1;
+break;
+  case 1:
+mask = 2;
+break;

This switch seems silly. You can just replace it with "mask = (1 << bit_index)"



Comment at: source/Plugins/Language/CPlusPlus/LibCxxVector.cpp:233-234
+  if (bit_set && buffer_sp && buffer_sp->GetBytes())
+*(buffer_sp->GetBytes()) =
+1; // regardless of endianness, anything non-zero is true
+  StreamString name;

I think the formatting here makes the code pretty hard to read



Comment at: source/Plugins/Language/CPlusPlus/LibCxxVector.cpp:301-304
+  TypeImpl type = valobj_sp->GetTypeImpl();
+  if (!type.IsValid())
+return nullptr;
+  CompilerType compiler_type = type.GetCompilerType(false);

Is there a reason you are not using ValueObject::GetCompilerType()?


https://reviews.llvm.org/D31880



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

[Lldb-commits] [PATCH] D31882: Fix the libc++ std::vector data formatter

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer created this revision.
Herald added a reviewer: EricWF.

Fix the libc++ std::vector data formatter

Previously we randomly triggered either the std::vector<...> or the
std::vector data formatter causing an issue when the former got
triggered for an std::vector.

This change moves the logic to decide which one to trigger from the
regular expression used to register the formatter into the formatter
creation logic as the former had no way to specify precedence.

Bug: llvm.org/pr32553


https://reviews.llvm.org/D31882

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxVector.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
@@ -23,6 +23,7 @@
 
 namespace lldb_private {
 namespace formatters {
+namespace {
 class LibcxxStdVectorSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
 public:
   LibcxxStdVectorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
@@ -45,6 +46,29 @@
   CompilerType m_element_type;
   uint32_t m_element_size;
 };
+
+class LibcxxStdVectorBoolSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  LibcxxStdVectorBoolSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
+
+  size_t CalculateNumChildren() override;
+
+  lldb::ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+  bool Update() override;
+
+  bool MightHaveChildren() override;
+
+  size_t GetIndexOfChildWithName(const ConstString ) override;
+
+private:
+  CompilerType m_bool_type;
+  ExecutionContextRef m_exe_ctx_ref;
+  uint64_t m_count;
+  lldb::addr_t m_base_data_address;
+  std::map m_children;
+};
+} // anonymous namespace
 } // namespace formatters
 } // namespace lldb_private
 
@@ -133,9 +157,129 @@
   return ExtractIndexFromString(name.GetCString());
 }
 
+lldb_private::formatters::LibcxxStdVectorBoolSyntheticFrontEnd::
+LibcxxStdVectorBoolSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
+: SyntheticChildrenFrontEnd(*valobj_sp), m_bool_type(), m_exe_ctx_ref(),
+  m_count(0), m_base_data_address(0), m_children() {
+  if (valobj_sp) {
+Update();
+m_bool_type =
+valobj_sp->GetCompilerType().GetBasicTypeFromAST(lldb::eBasicTypeBool);
+  }
+}
+
+size_t lldb_private::formatters::LibcxxStdVectorBoolSyntheticFrontEnd::
+CalculateNumChildren() {
+  return m_count;
+}
+
+lldb::ValueObjectSP
+lldb_private::formatters::LibcxxStdVectorBoolSyntheticFrontEnd::GetChildAtIndex(
+size_t idx) {
+  auto iter = m_children.find(idx), end = m_children.end();
+  if (iter != end)
+return iter->second;
+  if (idx >= m_count)
+return ValueObjectSP();
+  if (m_base_data_address == 0 || m_count == 0)
+return ValueObjectSP();
+  if (!m_bool_type)
+return ValueObjectSP();
+  size_t byte_idx = (idx >> 3); // divide by 8 to get byte index
+  size_t bit_index = (idx & 7); // efficient idx % 8 for bit index
+  lldb::addr_t byte_location = m_base_data_address + byte_idx;
+  ProcessSP process_sp(m_exe_ctx_ref.GetProcessSP());
+  if (!process_sp)
+return ValueObjectSP();
+  uint8_t byte = 0;
+  Error err;
+  size_t bytes_read = process_sp->ReadMemory(byte_location, , 1, err);
+  if (err.Fail() || bytes_read == 0)
+return ValueObjectSP();
+  bool bit_set = ((byte & (1 << bit_index)) != 0);
+  DataBufferSP buffer_sp(
+  new DataBufferHeap(m_bool_type.GetByteSize(nullptr), 0));
+  if (bit_set && buffer_sp && buffer_sp->GetBytes()) {
+// regardless of endianness, anything non-zero is true
+*(buffer_sp->GetBytes()) = 1;
+  }
+  StreamString name;
+  name.Printf("[%" PRIu64 "]", (uint64_t)idx);
+  ValueObjectSP retval_sp(CreateValueObjectFromData(
+  name.GetString(), DataExtractor(buffer_sp, process_sp->GetByteOrder(),
+  process_sp->GetAddressByteSize()),
+  m_exe_ctx_ref, m_bool_type));
+  if (retval_sp)
+m_children[idx] = retval_sp;
+  return retval_sp;
+}
+
+/*(std::__1::vector) vBool = {
+ __begin_ = 0x0001001000e0
+ __size_ = 56
+ __cap_alloc_ = {
+ std::__1::__libcpp_compressed_pair_imp > = {
+ __first_ = 1
+ }
+ }
+ }*/
+
+bool lldb_private::formatters::LibcxxStdVectorBoolSyntheticFrontEnd::Update() {
+  m_children.clear();
+  ValueObjectSP valobj_sp = m_backend.GetSP();
+  if (!valobj_sp)
+return false;
+  m_exe_ctx_ref = valobj_sp->GetExecutionContextRef();
+  ValueObjectSP size_sp(
+  valobj_sp->GetChildMemberWithName(ConstString("__size_"), true));
+  if (!size_sp)
+return false;
+  m_count = size_sp->GetValueAsUnsigned(0);
+  if (!m_count)
+return true;
+  ValueObjectSP 

[Lldb-commits] [Diffusion] rL299714: iwyu fixes for lldbCore.

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added subscribers: lldb-commits, tberghammer.
tberghammer added inline comments.

/lldb/trunk/include/lldb/Core/Address.h:21-50 I think we should try to merge 
these namespace definitions as in my view the current syntax makes the top of 
the file very noisy. What do you think?

Users:
  zturner (Author)

https://reviews.llvm.org/rL299714



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

[Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers

[Tamas Berghammer via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299259: Stop calling ValueObject::SetName from synthetic 
child providers (authored by tberghammer).

Changed prior to commit:
  https://reviews.llvm.org/D31371?vs=93528=93704#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31371

Files:
  lldb/trunk/include/lldb/Core/ValueObject.h
  lldb/trunk/source/Core/ValueObject.cpp
  lldb/trunk/source/DataFormatters/VectorType.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

Index: lldb/trunk/source/Core/ValueObject.cpp
===
--- lldb/trunk/source/Core/ValueObject.cpp
+++ lldb/trunk/source/Core/ValueObject.cpp
@@ -2962,6 +2962,10 @@
   return ValueObjectCast::Create(*this, GetName(), compiler_type);
 }
 
+lldb::ValueObjectSP ValueObject::Clone(const ConstString _name) {
+  return ValueObjectCast::Create(*this, new_name, GetCompilerType());
+}
+
 ValueObjectSP ValueObject::CastPointerType(const char *name,
CompilerType _type) {
   ValueObjectSP valobj_sp;
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -70,19 +70,19 @@
   std::unique_ptr tuple_frontend(
   LibStdcppTupleSyntheticFrontEndCreator(nullptr, tuple_sp));
 
-  m_ptr_obj = tuple_frontend->GetChildAtIndex(0);
-  if (m_ptr_obj)
-m_ptr_obj->SetName(ConstString("pointer"));
-
-  m_del_obj = tuple_frontend->GetChildAtIndex(1);
-  if (m_del_obj)
-m_del_obj->SetName(ConstString("deleter"));
+  ValueObjectSP ptr_obj = tuple_frontend->GetChildAtIndex(0);
+  if (ptr_obj)
+m_ptr_obj = ptr_obj->Clone(ConstString("pointer"));
+
+  ValueObjectSP del_obj = tuple_frontend->GetChildAtIndex(1);
+  if (del_obj)
+m_del_obj = del_obj->Clone(ConstString("deleter"));
 
   if (m_ptr_obj) {
 Error error;
-m_obj_obj = m_ptr_obj->Dereference(error);
+ValueObjectSP obj_obj = m_ptr_obj->Dereference(error);
 if (error.Success()) {
-  m_obj_obj->SetName(ConstString("object"));
+  m_obj_obj = obj_obj->Clone(ConstString("object"));
 }
   }
 
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
@@ -73,9 +73,7 @@
 if (value_sp) {
   StreamString name;
   name.Printf("[%zd]", m_members.size());
-  value_sp->SetName(ConstString(name.GetString()));
-
-  m_members.push_back(value_sp);
+  m_members.push_back(value_sp->Clone(ConstString(name.GetString(;
 }
   }
 }
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -406,19 +406,18 @@
 case 1: {
   auto child0_sp = potential_child_sp->GetChildAtIndex(0, true);
   if (child0_sp && child0_sp->GetName() == g___cc)
-potential_child_sp = child0_sp;
+potential_child_sp = child0_sp->Clone(ConstString(name.GetString()));
   break;
 }
 case 2: {
   auto child0_sp = potential_child_sp->GetChildAtIndex(0, true);
   auto child1_sp = potential_child_sp->GetChildAtIndex(1, true);
   if (child0_sp && child0_sp->GetName() == g___cc && child1_sp &&
   child1_sp->GetName() == g___nc)
-potential_child_sp = child0_sp;
+potential_child_sp = child0_sp->Clone(ConstString(name.GetString()));
   break;
 }
 }
-potential_child_sp->SetName(ConstString(name.GetString()));
   }
   m_iterators[idx] = iterator;
   return potential_child_sp;
Index: lldb/trunk/source/DataFormatters/VectorType.cpp
===
--- lldb/trunk/source/DataFormatters/VectorType.cpp
+++ lldb/trunk/source/DataFormatters/VectorType.cpp
@@ -204,14 +204,12 @@
 if (idx >= CalculateNumChildren())
   return lldb::ValueObjectSP();
 auto offset = idx * m_child_type.GetByteSize(nullptr);
-ValueObjectSP child_sp(
-m_backend.GetSyntheticChildAtOffset(offset, m_child_type, true));
-if (!child_sp)
-  return child_sp;
-
 StreamString idx_name;
 idx_name.Printf("[%" PRIu64 "]", (uint64_t)idx);
-child_sp->SetName(ConstString(idx_name.GetString()));
+ValueObjectSP child_sp(m_backend.GetSyntheticChildAtOffset(
+offset, 

[Lldb-commits] [PATCH] D31368: Add support for sythetic operator dereference

[Tamas Berghammer via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299251: Add support for sythetic operator dereference 
(authored by tberghammer).

Changed prior to commit:
  https://reviews.llvm.org/D31368?vs=93529=93701#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31368

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
  lldb/trunk/source/Core/ValueObject.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  lldb/trunk/source/Target/StackFrame.cpp
  lldb/trunk/www/varformats.html

Index: lldb/trunk/source/Core/ValueObject.cpp
===
--- lldb/trunk/source/Core/ValueObject.cpp
+++ lldb/trunk/source/Core/ValueObject.cpp
@@ -2889,6 +2889,11 @@
   child_is_base_class, child_is_deref_of_parent, eAddressTypeInvalid,
   language_flags);
 }
+  } else if (HasSyntheticValue()) {
+m_deref_valobj =
+GetSyntheticValue()
+->GetChildMemberWithName(ConstString("$$dereference$$"), true)
+.get();
   }
 
   if (m_deref_valobj) {
Index: lldb/trunk/source/Target/StackFrame.cpp
===
--- lldb/trunk/source/Target/StackFrame.cpp
+++ lldb/trunk/source/Target/StackFrame.cpp
@@ -606,8 +606,10 @@
 // Calculate the next separator index ahead of time
 ValueObjectSP child_valobj_sp;
 const char separator_type = var_expr[0];
+bool expr_is_ptr = false;
 switch (separator_type) {
 case '-':
+  expr_is_ptr = true;
   if (var_expr.size() >= 2 && var_expr[1] != '>')
 return ValueObjectSP();
 
@@ -624,11 +626,32 @@
   return ValueObjectSP();
 }
   }
+
+  // If we have a non pointer type with a sythetic value then lets check if
+  // we have an sythetic dereference specified.
+  if (!valobj_sp->IsPointerType() && valobj_sp->HasSyntheticValue()) {
+Error deref_error;
+if (valobj_sp->GetCompilerType().IsReferenceType()) {
+  valobj_sp = valobj_sp->GetSyntheticValue()->Dereference(deref_error);
+  if (error.Fail()) {
+error.SetErrorStringWithFormatv(
+"Failed to dereference reference type: %s", deref_error);
+return ValueObjectSP();
+  }
+}
+
+valobj_sp = valobj_sp->Dereference(deref_error);
+if (error.Fail()) {
+  error.SetErrorStringWithFormatv(
+  "Failed to dereference sythetic value: %s", deref_error);
+  return ValueObjectSP();
+}
+expr_is_ptr = false;
+  }
+
   var_expr = var_expr.drop_front(); // Remove the '-'
   LLVM_FALLTHROUGH;
 case '.': {
-  const bool expr_is_ptr = var_expr[0] == '>';
-
   var_expr = var_expr.drop_front(); // Remove the '.' or '>'
   separator_idx = var_expr.find_first_of(".-[");
   ConstString child_name(var_expr.substr(0, var_expr.find_first_of(".-[")));
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -114,7 +114,8 @@
 return 0;
   if (name == ConstString("del") || name == ConstString("deleter"))
 return 1;
-  if (name == ConstString("obj") || name == ConstString("object"))
+  if (name == ConstString("obj") || name == ConstString("object") ||
+  name == ConstString("$$dereference$$"))
 return 2;
   return UINT32_MAX;
 }
Index: lldb/trunk/www/varformats.html
===
--- lldb/trunk/www/varformats.html
+++ lldb/trunk/www/varformats.html
@@ -1068,6 +1068,7 @@
 [2] This method is optional (starting with SVN rev166495/LLDB-175). While implementing it in terms of num_children is acceptable, implementors are encouraged to look for optimized coding alternatives whenever reasonable.
 
 [3] This method is optional (starting with SVN revision 219330). The SBValue you return here will most likely be a numeric type (int, float, ...) as its value bytes will be used as-if they were the value of the root SBValue proper. As a shortcut for this, you can inherit from lldb.SBSyntheticValueProvider, and just define get_value as other methods are defaulted in the superclass as returning default no-children responses.
+If a synthetic child provider supplies a special child named $$dereference$$ then it will be used when evaluating opertaor* and operator- in the frame variable command and related SB API functions.
 		For examples of how synthetic children are created, you are encouraged to look at http://llvm.org/svn/llvm-project/lldb/trunk/examples/synthetic/;>examples/synthetic in the LLDB trunk. Please, be aware that the 

[Lldb-commits] [PATCH] D31366: Do not dereference std::unique_ptr by default

[Tamas Berghammer via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299249: Do not dereference std::unique_ptr by default 
(authored by tberghammer).

Changed prior to commit:
  https://reviews.llvm.org/D31366?vs=93039=93699#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31366

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/main.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -94,29 +94,27 @@
 lldb::ValueObjectSP
 LibStdcppUniquePtrSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
   if (idx == 0)
-return m_obj_obj;
+return m_ptr_obj;
   if (idx == 1)
 return m_del_obj;
   if (idx == 2)
-return m_ptr_obj;
+return m_obj_obj;
   return lldb::ValueObjectSP();
 }
 
 size_t LibStdcppUniquePtrSyntheticFrontEnd::CalculateNumChildren() {
   if (m_del_obj)
 return 2;
-  if (m_ptr_obj && m_ptr_obj->GetValueAsUnsigned(0) != 0)
-return 1;
-  return 0;
+  return 1;
 }
 
 size_t LibStdcppUniquePtrSyntheticFrontEnd::GetIndexOfChildWithName(
 const ConstString ) {
-  if (name == ConstString("obj") || name == ConstString("object"))
+  if (name == ConstString("ptr") || name == ConstString("pointer"))
 return 0;
   if (name == ConstString("del") || name == ConstString("deleter"))
 return 1;
-  if (name == ConstString("ptr") || name == ConstString("pointer"))
+  if (name == ConstString("obj") || name == ConstString("object"))
 return 2;
   return UINT32_MAX;
 }
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
@@ -34,13 +34,13 @@
 self.assertTrue(frame.IsValid())
 
 self.expect("frame variable nup", substrs=['nup = nullptr'])
-self.expect("frame variable iup", substrs=['iup = 0x', 'object = 123'])
-self.expect("frame variable sup", substrs=['sup = 0x', 'object = "foobar"'])
+self.expect("frame variable iup", substrs=['iup = 0x'])
+self.expect("frame variable sup", substrs=['sup = 0x'])
 
 self.expect("frame variable ndp", substrs=['ndp = nullptr'])
-self.expect("frame variable idp", substrs=['idp = 0x', 'object = 456', 'deleter = ', 'a = 1', 'b = 2'])
-self.expect("frame variable sdp", substrs=['sdp = 0x', 'object = "baz"', 'deleter = ', 'a = 3', 'b = 4'])
-
+self.expect("frame variable idp", substrs=['idp = 0x', 'deleter = ', 'a = 1', 'b = 2'])
+self.expect("frame variable sdp", substrs=['sdp = 0x', 'deleter = ', 'a = 3', 'b = 4'])
+
 self.assertEqual(123, frame.GetValueForVariablePath("iup.object").GetValueAsUnsigned())
 self.assertFalse(frame.GetValueForVariablePath("iup.deleter").IsValid())
 
@@ -59,3 +59,32 @@
 self.assertTrue(sdp_deleter.IsValid())
 self.assertEqual(3, sdp_deleter.GetChildMemberWithName("a").GetValueAsUnsigned())
 self.assertEqual(4, sdp_deleter.GetChildMemberWithName("b").GetValueAsUnsigned())
+
+@skipIfFreeBSD
+@skipIfWindows  # libstdcpp not ported to Windows
+@skipIfDarwin  # doesn't compile on Darwin
+def test_recursive_unique_ptr(self):
+# Tests that LLDB can handle when we have a loop in the unique_ptr
+# reference chain and that it correctly handles the different options
+# for the frame variable command in this case.
+self.build()
+self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_source_regexp(
+self, "Set break point at this line.")
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped', 'stop reason = breakpoint'])
+
+self.expect("frame variable f1->fp",
+substrs=['fp = 0x'])
+self.expect("frame variable --ptr-depth=1 f1->fp",
+substrs=['data = 2', 'fp = 0x'])
+self.expect("frame variable --ptr-depth=2 f1->fp",
+substrs=['data = 2', 'fp = 0x', 'data = 1'])
+
+frame = self.frame()
+

[Lldb-commits] [PATCH] D31366: Do not dereference std::unique_ptr by default

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

I tried it out on OSX and the problem is that version of libstdc++ shipping 
with every recent OSX version (don't know about old ones) is 4.2.1 (last 
version with GPL v2) what doesn't support std::unique_ptr (supported since 
4.3). Because of this I think the correct skip condition would be something 
like "skip if libstdc++ is older then 4.3" but I don't think we have a good way 
to specify that.


https://reviews.llvm.org/D31366



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

[Lldb-commits] [PATCH] D31368: Add support for sythetic operator dereference

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer updated this revision to Diff 93529.
tberghammer added a comment.

Fix grammer for the html documentation.


https://reviews.llvm.org/D31368

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
  source/Core/ValueObject.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  source/Target/StackFrame.cpp
  www/varformats.html

Index: www/varformats.html
===
--- www/varformats.html
+++ www/varformats.html
@@ -1068,6 +1068,7 @@
 [2] This method is optional (starting with SVN rev166495/LLDB-175). While implementing it in terms of num_children is acceptable, implementors are encouraged to look for optimized coding alternatives whenever reasonable.
 
 [3] This method is optional (starting with SVN revision 219330). The SBValue you return here will most likely be a numeric type (int, float, ...) as its value bytes will be used as-if they were the value of the root SBValue proper. As a shortcut for this, you can inherit from lldb.SBSyntheticValueProvider, and just define get_value as other methods are defaulted in the superclass as returning default no-children responses.
+If a synthetic child provider supplies a special child named $$dereference$$ then it will be used when evaluating opertaor* and operator- in the frame variable command and related SB API functions.
 		For examples of how synthetic children are created, you are encouraged to look at http://llvm.org/svn/llvm-project/lldb/trunk/examples/synthetic/;>examples/synthetic in the LLDB trunk. Please, be aware that the code in those files (except bitfield/)
 			is legacy code and is not maintained.
 			You may especially want to begin looking at http://llvm.org/svn/llvm-project/lldb/trunk/examples/synthetic/bitfield;>this example to get
Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -606,8 +606,10 @@
 // Calculate the next separator index ahead of time
 ValueObjectSP child_valobj_sp;
 const char separator_type = var_expr[0];
+bool expr_is_ptr = false;
 switch (separator_type) {
 case '-':
+  expr_is_ptr = true;
   if (var_expr.size() >= 2 && var_expr[1] != '>')
 return ValueObjectSP();
 
@@ -624,11 +626,32 @@
   return ValueObjectSP();
 }
   }
+
+  // If we have a non pointer type with a sythetic value then lets check if
+  // we have an sythetic dereference specified.
+  if (!valobj_sp->IsPointerType() && valobj_sp->HasSyntheticValue()) {
+Error deref_error;
+if (valobj_sp->GetCompilerType().IsReferenceType()) {
+  valobj_sp = valobj_sp->GetSyntheticValue()->Dereference(deref_error);
+  if (error.Fail()) {
+error.SetErrorStringWithFormatv(
+"Failed to dereference reference type: %s", deref_error);
+return ValueObjectSP();
+  }
+}
+
+valobj_sp = valobj_sp->Dereference(deref_error);
+if (error.Fail()) {
+  error.SetErrorStringWithFormatv(
+  "Failed to dereference sythetic value: %s", deref_error);
+  return ValueObjectSP();
+}
+expr_is_ptr = false;
+  }
+
   var_expr = var_expr.drop_front(); // Remove the '-'
   LLVM_FALLTHROUGH;
 case '.': {
-  const bool expr_is_ptr = var_expr[0] == '>';
-
   var_expr = var_expr.drop_front(); // Remove the '.' or '>'
   separator_idx = var_expr.find_first_of(".-[");
   ConstString child_name(var_expr.substr(0, var_expr.find_first_of(".-[")));
Index: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -114,7 +114,8 @@
 return 0;
   if (name == ConstString("del") || name == ConstString("deleter"))
 return 1;
-  if (name == ConstString("obj") || name == ConstString("object"))
+  if (name == ConstString("obj") || name == ConstString("object") ||
+  name == ConstString("$$dereference$$"))
 return 2;
   return UINT32_MAX;
 }
Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -2889,6 +2889,11 @@
   child_is_base_class, child_is_deref_of_parent, eAddressTypeInvalid,
   language_flags);
 }
+  } else if (HasSyntheticValue()) {
+m_deref_valobj =
+GetSyntheticValue()
+->GetChildMemberWithName(ConstString("$$dereference$$"), true)
+.get();
   }
 
   if (m_deref_valobj) {
Index: 

[Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer updated this revision to Diff 93528.
tberghammer added a comment.

Fix typos in the comments


https://reviews.llvm.org/D31371

Files:
  include/lldb/Core/ValueObject.h
  source/Core/ValueObject.cpp
  source/DataFormatters/VectorType.cpp
  source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

Index: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -70,19 +70,19 @@
   std::unique_ptr tuple_frontend(
   LibStdcppTupleSyntheticFrontEndCreator(nullptr, tuple_sp));
 
-  m_ptr_obj = tuple_frontend->GetChildAtIndex(0);
-  if (m_ptr_obj)
-m_ptr_obj->SetName(ConstString("pointer"));
+  ValueObjectSP ptr_obj = tuple_frontend->GetChildAtIndex(0);
+  if (ptr_obj)
+m_ptr_obj = ptr_obj->Clone(ConstString("pointer"));
 
-  m_del_obj = tuple_frontend->GetChildAtIndex(1);
-  if (m_del_obj)
-m_del_obj->SetName(ConstString("deleter"));
+  ValueObjectSP del_obj = tuple_frontend->GetChildAtIndex(1);
+  if (del_obj)
+m_del_obj = del_obj->Clone(ConstString("deleter"));
 
   if (m_ptr_obj) {
 Error error;
-m_obj_obj = m_ptr_obj->Dereference(error);
+ValueObjectSP obj_obj = m_ptr_obj->Dereference(error);
 if (error.Success()) {
-  m_obj_obj->SetName(ConstString("object"));
+  m_obj_obj = obj_obj->Clone(ConstString("object"));
 }
   }
 
Index: source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
===
--- source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
@@ -73,9 +73,7 @@
 if (value_sp) {
   StreamString name;
   name.Printf("[%zd]", m_members.size());
-  value_sp->SetName(ConstString(name.GetString()));
-
-  m_members.push_back(value_sp);
+  m_members.push_back(value_sp->Clone(ConstString(name.GetString(;
 }
   }
 }
Index: source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -406,19 +406,18 @@
 case 1: {
   auto child0_sp = potential_child_sp->GetChildAtIndex(0, true);
   if (child0_sp && child0_sp->GetName() == g___cc)
-potential_child_sp = child0_sp;
+potential_child_sp = child0_sp->Clone(ConstString(name.GetString()));
   break;
 }
 case 2: {
   auto child0_sp = potential_child_sp->GetChildAtIndex(0, true);
   auto child1_sp = potential_child_sp->GetChildAtIndex(1, true);
   if (child0_sp && child0_sp->GetName() == g___cc && child1_sp &&
   child1_sp->GetName() == g___nc)
-potential_child_sp = child0_sp;
+potential_child_sp = child0_sp->Clone(ConstString(name.GetString()));
   break;
 }
 }
-potential_child_sp->SetName(ConstString(name.GetString()));
   }
   m_iterators[idx] = iterator;
   return potential_child_sp;
Index: source/DataFormatters/VectorType.cpp
===
--- source/DataFormatters/VectorType.cpp
+++ source/DataFormatters/VectorType.cpp
@@ -204,14 +204,12 @@
 if (idx >= CalculateNumChildren())
   return lldb::ValueObjectSP();
 auto offset = idx * m_child_type.GetByteSize(nullptr);
-ValueObjectSP child_sp(
-m_backend.GetSyntheticChildAtOffset(offset, m_child_type, true));
-if (!child_sp)
-  return child_sp;
-
 StreamString idx_name;
 idx_name.Printf("[%" PRIu64 "]", (uint64_t)idx);
-child_sp->SetName(ConstString(idx_name.GetString()));
+ValueObjectSP child_sp(m_backend.GetSyntheticChildAtOffset(
+offset, m_child_type, true, ConstString(idx_name.GetString(;
+if (!child_sp)
+  return child_sp;
 
 child_sp->SetFormat(m_item_format);
 
Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -2957,6 +2957,10 @@
   return ValueObjectCast::Create(*this, GetName(), compiler_type);
 }
 
+lldb::ValueObjectSP ValueObject::Clone(const ConstString _name) {
+  return ValueObjectCast::Create(*this, new_name, GetCompilerType());
+}
+
 ValueObjectSP ValueObject::CastPointerType(const char *name,
CompilerType _type) {
   ValueObjectSP valobj_sp;
Index: include/lldb/Core/ValueObject.h
===
--- include/lldb/Core/ValueObject.h
+++ include/lldb/Core/ValueObject.h
@@ -553,6 +553,9 @@
 
   lldb::ValueObjectSP GetSP() { return m_manager->GetSharedPointer(this); }
 
+  // 

[Lldb-commits] [PATCH] D31368: Add support for sythetic operator dereference

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer updated this revision to Diff 93525.
tberghammer added a comment.

Add documentation to the website


https://reviews.llvm.org/D31368

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
  source/Core/ValueObject.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  source/Target/StackFrame.cpp
  www/varformats.html

Index: www/varformats.html
===
--- www/varformats.html
+++ www/varformats.html
@@ -1068,6 +1068,7 @@
 [2] This method is optional (starting with SVN rev166495/LLDB-175). While implementing it in terms of num_children is acceptable, implementors are encouraged to look for optimized coding alternatives whenever reasonable.
 
 [3] This method is optional (starting with SVN revision 219330). The SBValue you return here will most likely be a numeric type (int, float, ...) as its value bytes will be used as-if they were the value of the root SBValue proper. As a shortcut for this, you can inherit from lldb.SBSyntheticValueProvider, and just define get_value as other methods are defaulted in the superclass as returning default no-children responses.
+A sythetic child provider can supply a special child named $$dereference$$ what will be used when evaluating opertaor* and operator- in the frame variable command and related SB API functions.
 		For examples of how synthetic children are created, you are encouraged to look at http://llvm.org/svn/llvm-project/lldb/trunk/examples/synthetic/;>examples/synthetic in the LLDB trunk. Please, be aware that the code in those files (except bitfield/)
 			is legacy code and is not maintained.
 			You may especially want to begin looking at http://llvm.org/svn/llvm-project/lldb/trunk/examples/synthetic/bitfield;>this example to get
Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -606,8 +606,10 @@
 // Calculate the next separator index ahead of time
 ValueObjectSP child_valobj_sp;
 const char separator_type = var_expr[0];
+bool expr_is_ptr = false;
 switch (separator_type) {
 case '-':
+  expr_is_ptr = true;
   if (var_expr.size() >= 2 && var_expr[1] != '>')
 return ValueObjectSP();
 
@@ -624,11 +626,32 @@
   return ValueObjectSP();
 }
   }
+
+  // If we have a non pointer type with a sythetic value then lets check if
+  // we have an sythetic dereference specified.
+  if (!valobj_sp->IsPointerType() && valobj_sp->HasSyntheticValue()) {
+Error deref_error;
+if (valobj_sp->GetCompilerType().IsReferenceType()) {
+  valobj_sp = valobj_sp->GetSyntheticValue()->Dereference(deref_error);
+  if (error.Fail()) {
+error.SetErrorStringWithFormatv(
+"Failed to dereference reference type: %s", deref_error);
+return ValueObjectSP();
+  }
+}
+
+valobj_sp = valobj_sp->Dereference(deref_error);
+if (error.Fail()) {
+  error.SetErrorStringWithFormatv(
+  "Failed to dereference sythetic value: %s", deref_error);
+  return ValueObjectSP();
+}
+expr_is_ptr = false;
+  }
+
   var_expr = var_expr.drop_front(); // Remove the '-'
   LLVM_FALLTHROUGH;
 case '.': {
-  const bool expr_is_ptr = var_expr[0] == '>';
-
   var_expr = var_expr.drop_front(); // Remove the '.' or '>'
   separator_idx = var_expr.find_first_of(".-[");
   ConstString child_name(var_expr.substr(0, var_expr.find_first_of(".-[")));
Index: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -114,7 +114,8 @@
 return 0;
   if (name == ConstString("del") || name == ConstString("deleter"))
 return 1;
-  if (name == ConstString("obj") || name == ConstString("object"))
+  if (name == ConstString("obj") || name == ConstString("object") ||
+  name == ConstString("$$dereference$$"))
 return 2;
   return UINT32_MAX;
 }
Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -2889,6 +2889,11 @@
   child_is_base_class, child_is_deref_of_parent, eAddressTypeInvalid,
   language_flags);
 }
+  } else if (HasSyntheticValue()) {
+m_deref_valobj =
+GetSyntheticValue()
+->GetChildMemberWithName(ConstString("$$dereference$$"), true)
+.get();
   }
 
   if (m_deref_valobj) {
Index: 

[Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

SBValue::SetName is not part of the SB API (what is the right decision IMO as 
an SBValue should be mostly immutable) so this issue doesn't effect it. I 
looked through the code in examples/synthetic/gnu_libstdcpp.py and it is always 
using one of the SBValue::Create* method to produce new SBValue what will 
create a new value object one way or the other. Considering that nobody 
complained about the missing SetName method at the SB API level I don't see a 
big need for exposing the Clone method there. At the same line if SetName/Clone 
isn't part of the SB API then I think we shouldn't document it at the webpage.

(I will upload a fix for the spelling errors later)


https://reviews.llvm.org/D31371



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

[Lldb-commits] [PATCH] D31368: Add support for sythetic operator dereference

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer updated this revision to Diff 93241.
tberghammer added a comment.

Changed StackFrame to use Dereference instead of accessing the $$dereference$$ 
magic field.


https://reviews.llvm.org/D31368

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
  source/Core/ValueObject.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  source/Target/StackFrame.cpp

Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -606,8 +606,10 @@
 // Calculate the next separator index ahead of time
 ValueObjectSP child_valobj_sp;
 const char separator_type = var_expr[0];
+bool expr_is_ptr = false;
 switch (separator_type) {
 case '-':
+  expr_is_ptr = true;
   if (var_expr.size() >= 2 && var_expr[1] != '>')
 return ValueObjectSP();
 
@@ -624,11 +626,32 @@
   return ValueObjectSP();
 }
   }
+
+  // If we have a non pointer type with a sythetic value then lets check if
+  // we have an sythetic dereference specified.
+  if (!valobj_sp->IsPointerType() && valobj_sp->HasSyntheticValue()) {
+Error deref_error;
+if (valobj_sp->GetCompilerType().IsReferenceType()) {
+  valobj_sp = valobj_sp->GetSyntheticValue()->Dereference(deref_error);
+  if (error.Fail()) {
+error.SetErrorStringWithFormatv(
+"Failed to dereference reference type: %s", deref_error);
+return ValueObjectSP();
+  }
+}
+
+valobj_sp = valobj_sp->Dereference(deref_error);
+if (error.Fail()) {
+  error.SetErrorStringWithFormatv(
+  "Failed to dereference sythetic value: %s", deref_error);
+  return ValueObjectSP();
+}
+expr_is_ptr = false;
+  }
+
   var_expr = var_expr.drop_front(); // Remove the '-'
   LLVM_FALLTHROUGH;
 case '.': {
-  const bool expr_is_ptr = var_expr[0] == '>';
-
   var_expr = var_expr.drop_front(); // Remove the '.' or '>'
   separator_idx = var_expr.find_first_of(".-[");
   ConstString child_name(var_expr.substr(0, var_expr.find_first_of(".-[")));
Index: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -114,7 +114,8 @@
 return 0;
   if (name == ConstString("del") || name == ConstString("deleter"))
 return 1;
-  if (name == ConstString("obj") || name == ConstString("object"))
+  if (name == ConstString("obj") || name == ConstString("object") ||
+  name == ConstString("$$dereference$$"))
 return 2;
   return UINT32_MAX;
 }
Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -2889,6 +2889,11 @@
   child_is_base_class, child_is_deref_of_parent, eAddressTypeInvalid,
   language_flags);
 }
+  } else if (HasSyntheticValue()) {
+m_deref_valobj =
+GetSyntheticValue()
+->GetChildMemberWithName(ConstString("$$dereference$$"), true)
+.get();
   }
 
   if (m_deref_valobj) {
Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
===
--- packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
+++ packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
@@ -42,13 +42,17 @@
 self.expect("frame variable sdp", substrs=['sdp = 0x', 'deleter = ', 'a = 3', 'b = 4'])
 
 self.assertEqual(123, frame.GetValueForVariablePath("iup.object").GetValueAsUnsigned())
+self.assertEqual(123, frame.GetValueForVariablePath("*iup").GetValueAsUnsigned())
 self.assertFalse(frame.GetValueForVariablePath("iup.deleter").IsValid())
 
 self.assertEqual('"foobar"', frame.GetValueForVariablePath("sup.object").GetSummary())
+self.assertEqual('"foobar"', frame.GetValueForVariablePath("*sup").GetSummary())
 self.assertFalse(frame.GetValueForVariablePath("sup.deleter").IsValid())
 
 self.assertEqual(456, frame.GetValueForVariablePath("idp.object").GetValueAsUnsigned())
+self.assertEqual(456, frame.GetValueForVariablePath("*idp").GetValueAsUnsigned())
 self.assertEqual('"baz"', frame.GetValueForVariablePath("sdp.object").GetSummary())
+self.assertEqual('"baz"', frame.GetValueForVariablePath("*sdp").GetSummary())
 
 idp_deleter = 

[Lldb-commits] [PATCH] D31366: Do not dereference std::unique_ptr by default

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

My understanding (don't have an OSX machine at hand right now to try out) is 
that OSX ships with the libstdc++ belonging to gcc-4.2.1 and that version of 
libstdc++ is too old to support c++11 features such as std::unique_ptr.

Regarding skipping tests I am not aware of any way to skip a test based on the 
STL library we are using (Pavel is working on it for libc++ at 
https://reviews.llvm.org/D30984) and actually the problem here is that the 
version of libstdc++ shipping on  OSX is too old so I think skipIfDarwin is the 
correct decorator (we do it in several other tests as well). Alternative option 
could be to try to compile the source code and skip the test if compilation 
fails but that seems a bit flaky and might cause false negatives on other 
systems where we expect the test to pass.


https://reviews.llvm.org/D31366



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

[Lldb-commits] [PATCH] D31366: Do not dereference std::unique_ptr by default

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer requested review of this revision.
tberghammer added a comment.

I am trying to compile it with the following command on OSX but I wasn't able 
to get it working:

  clang  -std=c++11  -g -O0 -fno-builtin -arch x86_64   -fno-limit-debug-info 
-I$LLVM_ROOT/lldb/packages/Python/lldbsuite/test/make/../../../../../include 
-include $LLVM_ROOT/lldb/packages/Python/lldbsuite/test/make/test_common.h  
-stdlib=libstdc++ -DLLDB_USING_LIBSTDCPP --driver-mode=g++ -c -o main.o main.cpp

Compile error (first few):

  main.cpp:12:8: error: no member named 'unique_ptr' in namespace 'std'
std::unique_ptr nup;
~^
  main.cpp:12:23: error: expected '(' for function-style cast or type 
construction
std::unique_ptr nup;
^
  main.cpp:12:25: error: use of undeclared identifier 'nup'; did you mean 'dup'?
std::unique_ptr nup;
  ^~~
  dup
  
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/include/unistd.h:438:6:
 note: 'dup' declared here
  int  dup(int);
   ^
  main.cpp:13:8: error: no member named 'unique_ptr' in namespace 'std'
std::unique_ptr iup(new int{123});
~^
  main.cpp:13:22: error: expected '(' for function-style cast or type 
construction
std::unique_ptr iup(new int{123});
~~~^
  main.cpp:13:24: error: use of undeclared identifier 'iup'; did you mean 'dup'?
std::unique_ptr iup(new int{123});
 ^~~
 dup
  
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/include/unistd.h:438:6:
 note: 'dup' declared here
  int  dup(int);

I think the problem is that this is testing libstdc++ what is not available on 
OSX.

Clang version:

  Apple LLVM version 7.3.0 (clang-703.0.31)
  Target: x86_64-apple-darwin16.3.0
  Thread model: posix
  InstalledDir: 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin


https://reviews.llvm.org/D31366



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

[Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer created this revision.

Calling ValueObject::SetName from a sythetic child provider would change
the underying value object used for the non-synthetic child as well what
is clearly unintentional.


https://reviews.llvm.org/D31371

Files:
  include/lldb/Core/ValueObject.h
  source/Core/ValueObject.cpp
  source/DataFormatters/VectorType.cpp
  source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

Index: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -70,19 +70,19 @@
   std::unique_ptr tuple_frontend(
   LibStdcppTupleSyntheticFrontEndCreator(nullptr, tuple_sp));
 
-  m_ptr_obj = tuple_frontend->GetChildAtIndex(0);
-  if (m_ptr_obj)
-m_ptr_obj->SetName(ConstString("pointer"));
+  ValueObjectSP ptr_obj = tuple_frontend->GetChildAtIndex(0);
+  if (ptr_obj)
+m_ptr_obj = ptr_obj->Clone(ConstString("pointer"));
 
-  m_del_obj = tuple_frontend->GetChildAtIndex(1);
-  if (m_del_obj)
-m_del_obj->SetName(ConstString("deleter"));
+  ValueObjectSP del_obj = tuple_frontend->GetChildAtIndex(1);
+  if (del_obj)
+m_del_obj = del_obj->Clone(ConstString("deleter"));
 
   if (m_ptr_obj) {
 Error error;
-m_obj_obj = m_ptr_obj->Dereference(error);
+ValueObjectSP obj_obj = m_ptr_obj->Dereference(error);
 if (error.Success()) {
-  m_obj_obj->SetName(ConstString("object"));
+  m_obj_obj = obj_obj->Clone(ConstString("object"));
 }
   }
 
Index: source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
===
--- source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
@@ -73,9 +73,7 @@
 if (value_sp) {
   StreamString name;
   name.Printf("[%zd]", m_members.size());
-  value_sp->SetName(ConstString(name.GetString()));
-
-  m_members.push_back(value_sp);
+  m_members.push_back(value_sp->Clone(ConstString(name.GetString(;
 }
   }
 }
Index: source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -406,19 +406,18 @@
 case 1: {
   auto child0_sp = potential_child_sp->GetChildAtIndex(0, true);
   if (child0_sp && child0_sp->GetName() == g___cc)
-potential_child_sp = child0_sp;
+potential_child_sp = child0_sp->Clone(ConstString(name.GetString()));
   break;
 }
 case 2: {
   auto child0_sp = potential_child_sp->GetChildAtIndex(0, true);
   auto child1_sp = potential_child_sp->GetChildAtIndex(1, true);
   if (child0_sp && child0_sp->GetName() == g___cc && child1_sp &&
   child1_sp->GetName() == g___nc)
-potential_child_sp = child0_sp;
+potential_child_sp = child0_sp->Clone(ConstString(name.GetString()));
   break;
 }
 }
-potential_child_sp->SetName(ConstString(name.GetString()));
   }
   m_iterators[idx] = iterator;
   return potential_child_sp;
Index: source/DataFormatters/VectorType.cpp
===
--- source/DataFormatters/VectorType.cpp
+++ source/DataFormatters/VectorType.cpp
@@ -204,14 +204,12 @@
 if (idx >= CalculateNumChildren())
   return lldb::ValueObjectSP();
 auto offset = idx * m_child_type.GetByteSize(nullptr);
-ValueObjectSP child_sp(
-m_backend.GetSyntheticChildAtOffset(offset, m_child_type, true));
-if (!child_sp)
-  return child_sp;
-
 StreamString idx_name;
 idx_name.Printf("[%" PRIu64 "]", (uint64_t)idx);
-child_sp->SetName(ConstString(idx_name.GetString()));
+ValueObjectSP child_sp(m_backend.GetSyntheticChildAtOffset(
+offset, m_child_type, true, ConstString(idx_name.GetString(;
+if (!child_sp)
+  return child_sp;
 
 child_sp->SetFormat(m_item_format);
 
Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -2957,6 +2957,10 @@
   return ValueObjectCast::Create(*this, GetName(), compiler_type);
 }
 
+lldb::ValueObjectSP ValueObject::Clone(const ConstString _name) {
+  return ValueObjectCast::Create(*this, new_name, GetCompilerType());
+}
+
 ValueObjectSP ValueObject::CastPointerType(const char *name,
CompilerType _type) {
   ValueObjectSP valobj_sp;
Index: include/lldb/Core/ValueObject.h
===
--- include/lldb/Core/ValueObject.h
+++ include/lldb/Core/ValueObject.h
@@ 

[Lldb-commits] [PATCH] D31368: Add support for sythetic operator dereference

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer created this revision.

After this change a sythetic child provider can generate a special child
named "$$dereference$$" what if present is used when "operator*" or
"operator->" used on a ValueObject. The goal of the change is to make
expressions like "up->foo" work inside the "frame variable" command.


https://reviews.llvm.org/D31368

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
  source/Core/ValueObject.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  source/Target/StackFrame.cpp

Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -606,8 +606,10 @@
 // Calculate the next separator index ahead of time
 ValueObjectSP child_valobj_sp;
 const char separator_type = var_expr[0];
+bool expr_is_ptr = false;
 switch (separator_type) {
 case '-':
+  expr_is_ptr = true;
   if (var_expr.size() >= 2 && var_expr[1] != '>')
 return ValueObjectSP();
 
@@ -624,11 +626,24 @@
   return ValueObjectSP();
 }
   }
+
+  // If we have a non pointer type with a sythetic value then lets check if
+  // we have an sythetic dereference specified.
+  if (!valobj_sp->IsPointerType() && valobj_sp->HasSyntheticValue()) {
+valobj_sp = valobj_sp->GetSyntheticValue()->GetChildMemberWithName(
+ConstString("$$dereference$$"), true);
+if (valobj_sp) {
+  expr_is_ptr = false;
+} else {
+  error.SetErrorString(
+  "Failed to access synthetic dereference member.");
+  return ValueObjectSP();
+}
+  }
+
   var_expr = var_expr.drop_front(); // Remove the '-'
   LLVM_FALLTHROUGH;
 case '.': {
-  const bool expr_is_ptr = var_expr[0] == '>';
-
   var_expr = var_expr.drop_front(); // Remove the '.' or '>'
   separator_idx = var_expr.find_first_of(".-[");
   ConstString child_name(var_expr.substr(0, var_expr.find_first_of(".-[")));
Index: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -114,7 +114,8 @@
 return 0;
   if (name == ConstString("del") || name == ConstString("deleter"))
 return 1;
-  if (name == ConstString("obj") || name == ConstString("object"))
+  if (name == ConstString("obj") || name == ConstString("object") ||
+  name == ConstString("$$dereference$$"))
 return 2;
   return UINT32_MAX;
 }
Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -2889,6 +2889,11 @@
   child_is_base_class, child_is_deref_of_parent, eAddressTypeInvalid,
   language_flags);
 }
+  } else if (HasSyntheticValue()) {
+m_deref_valobj =
+GetSyntheticValue()
+->GetChildMemberWithName(ConstString("$$dereference$$"), true)
+.get();
   }
 
   if (m_deref_valobj) {
Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
===
--- packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
+++ packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
@@ -42,13 +42,17 @@
 self.expect("frame variable sdp", substrs=['sdp = 0x', 'deleter = ', 'a = 3', 'b = 4'])
 
 self.assertEqual(123, frame.GetValueForVariablePath("iup.object").GetValueAsUnsigned())
+self.assertEqual(123, frame.GetValueForVariablePath("*iup").GetValueAsUnsigned())
 self.assertFalse(frame.GetValueForVariablePath("iup.deleter").IsValid())
 
 self.assertEqual('"foobar"', frame.GetValueForVariablePath("sup.object").GetSummary())
+self.assertEqual('"foobar"', frame.GetValueForVariablePath("*sup").GetSummary())
 self.assertFalse(frame.GetValueForVariablePath("sup.deleter").IsValid())
 
 self.assertEqual(456, frame.GetValueForVariablePath("idp.object").GetValueAsUnsigned())
+self.assertEqual(456, frame.GetValueForVariablePath("*idp").GetValueAsUnsigned())
 self.assertEqual('"baz"', frame.GetValueForVariablePath("sdp.object").GetSummary())
+self.assertEqual('"baz"', frame.GetValueForVariablePath("*sdp").GetSummary())
 
 idp_deleter = frame.GetValueForVariablePath("idp.deleter")
 self.assertTrue(idp_deleter.IsValid())
@@ -86,5 +90,7 @@
 frame = self.frame()
 

[Lldb-commits] [PATCH] D30272: Improve data formatter for libstdcpp unique_ptr

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer abandoned this revision.
tberghammer added a comment.

I wasn't able to get this approach working so I propose an alternative solution 
at https://reviews.llvm.org/D31366 for the infinite loop issue and I will 
create a separate CL for dereferencing smart pointers in "frame variable".


https://reviews.llvm.org/D30272



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

[Lldb-commits] [PATCH] D30410: testsuite/android: build test executables with the android ndk directly

[Tamas Berghammer via Phabricator via lldb-commits]

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

LGTM, assuming that it works on all 3 OSes




Comment at: packages/Python/lldbsuite/test/make/Android.rules:1
+NDK_ROOT := $(shell dirname $(CC))/../../../../..
+NDK_ROOT := $(realpath $(NDK_ROOT))

Will this file work on Windows (e.g. dirname, realpath, forward slash)?



Comment at: packages/Python/lldbsuite/test/make/Android.rules:4-13
+ifeq "$(findstring $(ARCH), aarch64 x86_64)" "$(ARCH)"
+   # lowest 64-bit API level
+   API_LEVEL := 21
+else ifeq "$(ARCH)" "i386"
+   # clone(2) declaration is present only since this api level
+   API_LEVEL := 17
+else

Does it make sense to use a different API level for every architecture? I think 
making it somewhat consistent would make it easier to use it


https://reviews.llvm.org/D30410



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

[Lldb-commits] [PATCH] D30272: Improve data formatter for libstdcpp unique_ptr

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

In https://reviews.llvm.org/D30272#686061, @jingham wrote:

> I also thought about having the synthetic child provider mark up the special 
> child value objects when it made them.  It bugs me a little but that you 
> couldn't, for instance, have a synthetic child provider that doesn't display 
> a child that is the result of the dereference in its view, but provides one 
> when you do the dereference.  And it looks like we're creating a system where 
> we have somebody - the SCP - that knows what the dereference means to it, but 
> then we lose the connection to that knowledge when we hand it out.


The current system already supports it to have a synthetic child what doesn't 
displayed by default (can be referenced by name) so we can rely on that 
functionality to have a child what is the result of the dereference while not 
in view. We might have to gave it a special name (e.g. "$dereference") to make 
it easy to access it from the value object but I am not sure if it will be 
necessary (would certainly make the code simpler and more efficient).

I thought quite a bit about adding a Dereference method the the synthetic child 
provider but I am not convinced it is a good idea as it would complicate the 
API for it and I don't see how can we solve the problem I hit with the infinite 
smart pointer loop when I added a synthetic child displayed by default what 
contained the dereference of the pointed object without annotating the children 
itself.

> Explain a bit more why you need to create a copy?  Anything that makes two 
> versions of the same value that we have to keep in sync makes me nervous.  
> For instance, one of the things we want to do in the future is make 
> write-back possible for synthetic children.  That would allow you to change 
> values of std::vector elements in the Locals view of a GUI debugger (which 
> would naturally be based on SBValues) just like you can with the 
> non-synthetic SBValues.  Also, how would the SCP know to update this copy 
> when you've stepped in the debugger and the dereference now points to another 
> pointee?

Take the following (bit strange) example:

  struct Foo {
int x;
int& operator*() { return x; }
  }

If I implement a synthetic child provider for it then it will return the same 
value object as we would return when accessing the "x" member variable. If we 
annotate that value object as dereference of parent then it will effect the way 
we are displaying the "x" member variable even when it is not used through the 
"operator*". Creating a second separate ValueObject solves this problem by 
separating the 2 solving this issue. For updating it we will rely on the same 
logic what is currently used to update other synthetic children after a step 
what is not backed by an actual variable. My current idea is to create a new 
ValueObject with the memory address and compiler type copied from the original 
value object and it will be updated using the Update method of the synthetic 
child provider.


https://reviews.llvm.org/D30272



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

[Lldb-commits] [PATCH] D30272: Improve data formatter for libstdcpp unique_ptr

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a comment.

My original plan was to add it to the synthetic child provider but after some 
thoughts and experimenting I concluded it doesn't really belongs to there 
because it should be responsible for generating new children while here we are 
modifying the way the parent object is printed. Also for this reason the 
implementation of it would be fairly complex because we would have to execute 
the synthetic child providers when the parent is referenced instead of when a 
child of it is queried. Also I think it won't be a good idea to add a new 
method to the synthetic child provider called "IsPointerLike" as it seems like 
introducing a very specific method to an interface what is currently quite 
generic. On the other hand I see the need for making the list of pointer like 
objects easily extensible from python.

One solution I can possibly see (haven't tried it out) is to add the following 
2 new method to the value object:

- IsDeceremntPointerDepth: By default returns true for pointers and references 
and will return true for children generated for smart pointers
- IsDereferenceOfParent: By default returns false and will return true for the 
first child of the smart pointers

If we add these methods then the synthetic child provider can create a child 
where they will be returning true and then rely on them to implement this 
functionality. I think this solution would be a fairly clean design but would 
introduce a lot of new API for a small amount of extra functionality.

The third possible option is to somehow put the IsPointerLikeObject method into 
the summary provider as I think that would be the best place for it but I am 
not sure how to do it as it currently takes a ValueObject and then returns a 
string without too much flexibility. Saying that it should just flip some bit 
on the ValueObject itself seems like a quite big hack and would make the API 
hard to maintain so I don't really like this implementation.

What do you think?




Comment at: source/DataFormatters/ValueObjectPrinter.cpp:515
   const bool is_ptr = IsPtr();
+  const bool is_ref_or_ptr_like = IsRef() || IsPointerLikeObject();
   const bool is_uninit = IsUninitialized();

labath wrote:
> Is there are reason you are bundling the is-pointer-like with the is-ref flag 
> instead of the is-ptr one (which would be more logical)? If there is one it 
> certainly isn't obvious.
Because I want to treat a unique_ptr the same way as a reference so if you just 
write so it is automatically dereferenced at the root level while not at other 
levels (we don't dereference pointers by default at any level). I can split it 
out to a separate bool if you think that helps


https://reviews.llvm.org/D30272



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

[Lldb-commits] [PATCH] D28775: [cmake] Make lldb build with the android ndk toolchain file

[Tamas Berghammer via Phabricator via lldb-commits]

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

LGTM


https://reviews.llvm.org/D28775



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

[Lldb-commits] [PATCH] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests

[Tamas Berghammer via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL291559: Improve Type::GetTypeScopeAndBasenameHelper and add 
unit tests (authored by tberghammer).

Changed prior to commit:
  https://reviews.llvm.org/D28466?vs=83669=83791#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28466

Files:
  lldb/trunk/include/lldb/Symbol/Type.h
  lldb/trunk/source/Core/Module.cpp
  lldb/trunk/source/Symbol/Type.cpp
  lldb/trunk/source/Symbol/TypeList.cpp
  lldb/trunk/source/Symbol/TypeMap.cpp
  lldb/trunk/unittests/Symbol/CMakeLists.txt
  lldb/trunk/unittests/Symbol/TestType.cpp

Index: lldb/trunk/source/Symbol/TypeList.cpp
===
--- lldb/trunk/source/Symbol/TypeList.cpp
+++ lldb/trunk/source/Symbol/TypeList.cpp
@@ -108,13 +108,13 @@
 
 void TypeList::RemoveMismatchedTypes(const char *qualified_typename,
  bool exact_match) {
-  std::string type_scope;
-  std::string type_basename;
+  llvm::StringRef type_scope;
+  llvm::StringRef type_basename;
   TypeClass type_class = eTypeClassAny;
   if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
  type_basename, type_class)) {
 type_basename = qualified_typename;
-type_scope.clear();
+type_scope = "";
   }
   return RemoveMismatchedTypes(type_scope, type_basename, type_class,
exact_match);
@@ -145,8 +145,8 @@
 ConstString match_type_name_const_str(the_type->GetQualifiedName());
 if (match_type_name_const_str) {
   const char *match_type_name = match_type_name_const_str.GetCString();
-  std::string match_type_scope;
-  std::string match_type_basename;
+  llvm::StringRef match_type_scope;
+  llvm::StringRef match_type_basename;
   if (Type::GetTypeScopeAndBasename(match_type_name, match_type_scope,
 match_type_basename,
 match_type_class)) {
Index: lldb/trunk/source/Symbol/TypeMap.cpp
===
--- lldb/trunk/source/Symbol/TypeMap.cpp
+++ lldb/trunk/source/Symbol/TypeMap.cpp
@@ -152,13 +152,13 @@
 
 void TypeMap::RemoveMismatchedTypes(const char *qualified_typename,
 bool exact_match) {
-  std::string type_scope;
-  std::string type_basename;
+  llvm::StringRef type_scope;
+  llvm::StringRef type_basename;
   TypeClass type_class = eTypeClassAny;
   if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
  type_basename, type_class)) {
 type_basename = qualified_typename;
-type_scope.clear();
+type_scope = "";
   }
   return RemoveMismatchedTypes(type_scope, type_basename, type_class,
exact_match);
@@ -189,8 +189,8 @@
 ConstString match_type_name_const_str(the_type->GetQualifiedName());
 if (match_type_name_const_str) {
   const char *match_type_name = match_type_name_const_str.GetCString();
-  std::string match_type_scope;
-  std::string match_type_basename;
+  llvm::StringRef match_type_scope;
+  llvm::StringRef match_type_basename;
   if (Type::GetTypeScopeAndBasename(match_type_name, match_type_scope,
 match_type_basename,
 match_type_class)) {
Index: lldb/trunk/source/Symbol/Type.cpp
===
--- lldb/trunk/source/Symbol/Type.cpp
+++ lldb/trunk/source/Symbol/Type.cpp
@@ -620,50 +620,59 @@
   return GetForwardCompilerType().GetConstTypeName();
 }
 
-bool Type::GetTypeScopeAndBasename(const char *_cstr, std::string ,
-   std::string ,
+bool Type::GetTypeScopeAndBasename(const llvm::StringRef& name,
+   llvm::StringRef ,
+   llvm::StringRef ,
TypeClass _class) {
-  // Protect against null c string.
-
   type_class = eTypeClassAny;
 
-  if (name_cstr && name_cstr[0]) {
-llvm::StringRef name_strref(name_cstr);
-if (name_strref.startswith("struct ")) {
-  name_cstr += 7;
-  type_class = eTypeClassStruct;
-} else if (name_strref.startswith("class ")) {
-  name_cstr += 6;
-  type_class = eTypeClassClass;
-} else if (name_strref.startswith("union ")) {
-  name_cstr += 6;
-  type_class = eTypeClassUnion;
-} else if (name_strref.startswith("enum ")) {
-  name_cstr += 5;
-  type_class = eTypeClassEnumeration;
-} else if (name_strref.startswith("typedef ")) {
-  name_cstr += 8;
-  type_class = eTypeClassTypedef;
-}
-const char *basename_cstr = name_cstr;
-const char *namespace_separator = ::strstr(basename_cstr, "::");
-if (namespace_separator) {
-  const char *template_arg_char = 

[Lldb-commits] [PATCH] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer updated this revision to Diff 83669.
tberghammer marked 9 inline comments as done.

https://reviews.llvm.org/D28466

Files:
  include/lldb/Symbol/Type.h
  source/Core/Module.cpp
  source/Symbol/Type.cpp
  source/Symbol/TypeList.cpp
  source/Symbol/TypeMap.cpp
  unittests/Symbol/CMakeLists.txt
  unittests/Symbol/TestType.cpp

Index: unittests/Symbol/TestType.cpp
===
--- /dev/null
+++ unittests/Symbol/TestType.cpp
@@ -0,0 +1,51 @@
+//===-- TestType.cpp *- C++ -*-===//
+//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Symbol/Type.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+void TestGetTypeScopeAndBasenameHelper(const char *full_type,
+   bool expected_is_scoped,
+   const char *expected_scope,
+   const char *expected_name) {
+  llvm::StringRef scope, name;
+  lldb::TypeClass type_class;
+  bool is_scoped =
+  Type::GetTypeScopeAndBasename(full_type, scope, name, type_class);
+  EXPECT_EQ(is_scoped, expected_is_scoped);
+  if (expected_is_scoped) {
+EXPECT_EQ(scope, expected_scope);
+EXPECT_EQ(name, expected_name);
+  }
+}
+};
+
+TEST(Type, GetTypeScopeAndBasename) {
+  TestGetTypeScopeAndBasenameHelper("int", false, "", "");
+  TestGetTypeScopeAndBasenameHelper("std::string", true, "std::", "string");
+  TestGetTypeScopeAndBasenameHelper("std::set", true, "std::", "set");
+  TestGetTypeScopeAndBasenameHelper("std::set", true,
+"std::", "set");
+  TestGetTypeScopeAndBasenameHelper("std::string::iterator", true,
+"std::string::", "iterator");
+  TestGetTypeScopeAndBasenameHelper("std::set::iterator", true,
+"std::set::", "iterator");
+  TestGetTypeScopeAndBasenameHelper(
+  "std::set::iterator", true,
+  "std::set::", "iterator");
+  TestGetTypeScopeAndBasenameHelper(
+  "std::set::iterator", true,
+  "std::set::", "iterator");
+}
Index: unittests/Symbol/CMakeLists.txt
===
--- unittests/Symbol/CMakeLists.txt
+++ unittests/Symbol/CMakeLists.txt
@@ -1,3 +1,4 @@
 add_lldb_unittest(SymbolTests
   TestClangASTContext.cpp
+  TestType.cpp
   )
Index: source/Symbol/TypeMap.cpp
===
--- source/Symbol/TypeMap.cpp
+++ source/Symbol/TypeMap.cpp
@@ -152,13 +152,13 @@
 
 void TypeMap::RemoveMismatchedTypes(const char *qualified_typename,
 bool exact_match) {
-  std::string type_scope;
-  std::string type_basename;
+  llvm::StringRef type_scope;
+  llvm::StringRef type_basename;
   TypeClass type_class = eTypeClassAny;
   if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
  type_basename, type_class)) {
 type_basename = qualified_typename;
-type_scope.clear();
+type_scope = "";
   }
   return RemoveMismatchedTypes(type_scope, type_basename, type_class,
exact_match);
@@ -189,8 +189,8 @@
 ConstString match_type_name_const_str(the_type->GetQualifiedName());
 if (match_type_name_const_str) {
   const char *match_type_name = match_type_name_const_str.GetCString();
-  std::string match_type_scope;
-  std::string match_type_basename;
+  llvm::StringRef match_type_scope;
+  llvm::StringRef match_type_basename;
   if (Type::GetTypeScopeAndBasename(match_type_name, match_type_scope,
 match_type_basename,
 match_type_class)) {
Index: source/Symbol/TypeList.cpp
===
--- source/Symbol/TypeList.cpp
+++ source/Symbol/TypeList.cpp
@@ -108,13 +108,13 @@
 
 void TypeList::RemoveMismatchedTypes(const char *qualified_typename,
  bool exact_match) {
-  std::string type_scope;
-  std::string type_basename;
+  llvm::StringRef type_scope;
+  llvm::StringRef type_basename;
   TypeClass type_class = eTypeClassAny;
   if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
  type_basename, type_class)) {
 type_basename = qualified_typename;
-type_scope.clear();
+type_scope = "";
   }
   return RemoveMismatchedTypes(type_scope, type_basename, type_class,

[Lldb-commits] [PATCH] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer created this revision.
tberghammer added a reviewer: clayborg.
tberghammer added a subscriber: lldb-commits.
Herald added a subscriber: mgorny.

Improve Type::GetTypeScopeAndBasenameHelper and add unit tests

  

Previously it failed to handle nested types inside templated classes
making it impossible to look up these types using the fully qualified
name.


https://reviews.llvm.org/D28466

Files:
  source/Symbol/Type.cpp
  unittests/Symbol/CMakeLists.txt
  unittests/Symbol/TestType.cpp

Index: unittests/Symbol/TestType.cpp
===
--- /dev/null
+++ unittests/Symbol/TestType.cpp
@@ -0,0 +1,51 @@
+//===-- TestType.cpp *- C++ -*-===//
+//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Symbol/Type.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+void TestGetTypeScopeAndBasenameHelper(const char *full_type,
+   bool expected_is_scoped,
+   const char *expected_scope,
+   const char *expected_name) {
+  std::string scope, name;
+  lldb::TypeClass type_class;
+  bool is_scoped =
+  Type::GetTypeScopeAndBasename(full_type, scope, name, type_class);
+  EXPECT_EQ(is_scoped, expected_is_scoped);
+  if (expected_is_scoped) {
+EXPECT_EQ(scope, expected_scope);
+EXPECT_EQ(name, expected_name);
+  }
+}
+};
+
+TEST(Type, GetTypeScopeAndBasename) {
+  TestGetTypeScopeAndBasenameHelper("int", false, "", "");
+  TestGetTypeScopeAndBasenameHelper("std::string", true, "std::", "string");
+  TestGetTypeScopeAndBasenameHelper("std::set", true, "std::", "set");
+  TestGetTypeScopeAndBasenameHelper("std::set", true,
+"std::", "set");
+  TestGetTypeScopeAndBasenameHelper("std::string::iterator", true,
+"std::string::", "iterator");
+  TestGetTypeScopeAndBasenameHelper("std::set::iterator", true,
+"std::set::", "iterator");
+  TestGetTypeScopeAndBasenameHelper(
+  "std::set::iterator", true,
+  "std::set::", "iterator");
+  TestGetTypeScopeAndBasenameHelper(
+  "std::set::iterator", true,
+  "std::set::", "iterator");
+}
Index: unittests/Symbol/CMakeLists.txt
===
--- unittests/Symbol/CMakeLists.txt
+++ unittests/Symbol/CMakeLists.txt
@@ -1,3 +1,4 @@
 add_lldb_unittest(SymbolTests
   TestClangASTContext.cpp
+  TestType.cpp
   )
Index: source/Symbol/Type.cpp
===
--- source/Symbol/Type.cpp
+++ source/Symbol/Type.cpp
@@ -623,47 +623,62 @@
 bool Type::GetTypeScopeAndBasename(const char *_cstr, std::string ,
std::string ,
TypeClass _class) {
+  type_class = eTypeClassAny;
+
   // Protect against null c string.
+  if (!name_cstr || !name_cstr[0])
+return false;
 
-  type_class = eTypeClassAny;
+  llvm::StringRef name_strref(name_cstr);
+  if (name_strref.startswith("struct ")) {
+name_cstr += 7;
+type_class = eTypeClassStruct;
+  } else if (name_strref.startswith("class ")) {
+name_cstr += 6;
+type_class = eTypeClassClass;
+  } else if (name_strref.startswith("union ")) {
+name_cstr += 6;
+type_class = eTypeClassUnion;
+  } else if (name_strref.startswith("enum ")) {
+name_cstr += 5;
+type_class = eTypeClassEnumeration;
+  } else if (name_strref.startswith("typedef ")) {
+name_cstr += 8;
+type_class = eTypeClassTypedef;
+  }
 
-  if (name_cstr && name_cstr[0]) {
-llvm::StringRef name_strref(name_cstr);
-if (name_strref.startswith("struct ")) {
-  name_cstr += 7;
-  type_class = eTypeClassStruct;
-} else if (name_strref.startswith("class ")) {
-  name_cstr += 6;
-  type_class = eTypeClassClass;
-} else if (name_strref.startswith("union ")) {
-  name_cstr += 6;
-  type_class = eTypeClassUnion;
-} else if (name_strref.startswith("enum ")) {
-  name_cstr += 5;
-  type_class = eTypeClassEnumeration;
-} else if (name_strref.startswith("typedef ")) {
-  name_cstr += 8;
-  type_class = eTypeClassTypedef;
-}
-const char *basename_cstr = name_cstr;
-const char *namespace_separator = ::strstr(basename_cstr, "::");
-if (namespace_separator) {
-  const char *template_arg_char = ::strchr(basename_cstr, '<');
-  while (namespace_separator != nullptr) {
-if (template_arg_char &&
-

[Lldb-commits] [PATCH] D28233: Improve the performance of jModulesInfo in lldb-server

[Tamas Berghammer via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL290895: Improve the performance of jModulesInfo in 
lldb-server (authored by tberghammer).

Changed prior to commit:
  https://reviews.llvm.org/D28233?vs=82884=82897#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28233

Files:
  lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h

Index: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -119,7 +119,7 @@
   ArchSpec m_arch;
 
   LazyBool m_supports_mem_region;
-  std::vector m_mem_region_cache;
+  std::vector> m_mem_region_cache;
 
   lldb::tid_t m_pending_notification_tid;
 
@@ -217,6 +217,8 @@
   void ThreadWasCreated(NativeThreadLinux );
 
   void SigchldHandler();
+
+  Error PopulateMemoryRegionCache();
 };
 
 } // namespace process_linux
Index: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1689,68 +1689,14 @@
   // Assume proc maps entries are in ascending order.
   // FIXME assert if we find differently.
 
-  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
-  Error error;
-
   if (m_supports_mem_region == LazyBool::eLazyBoolNo) {
 // We're done.
-error.SetErrorString("unsupported");
-return error;
+return Error("unsupported");
   }
 
-  // If our cache is empty, pull the latest.  There should always be at least
-  // one memory region
-  // if memory region handling is supported.
-  if (m_mem_region_cache.empty()) {
-error = ProcFileReader::ProcessLineByLine(
-GetID(), "maps", [&](const std::string ) -> bool {
-  MemoryRegionInfo info;
-  const Error parse_error =
-  ParseMemoryRegionInfoFromProcMapsLine(line, info);
-  if (parse_error.Success()) {
-m_mem_region_cache.push_back(info);
-return true;
-  } else {
-if (log)
-  log->Printf("NativeProcessLinux::%s failed to parse proc maps "
-  "line '%s': %s",
-  __FUNCTION__, line.c_str(), error.AsCString());
-return false;
-  }
-});
-
-// If we had an error, we'll mark unsupported.
-if (error.Fail()) {
-  m_supports_mem_region = LazyBool::eLazyBoolNo;
-  return error;
-} else if (m_mem_region_cache.empty()) {
-  // No entries after attempting to read them.  This shouldn't happen if
-  // /proc/{pid}/maps
-  // is supported.  Assume we don't support map entries via procfs.
-  if (log)
-log->Printf("NativeProcessLinux::%s failed to find any procfs maps "
-"entries, assuming no support for memory region metadata "
-"retrieval",
-__FUNCTION__);
-  m_supports_mem_region = LazyBool::eLazyBoolNo;
-  error.SetErrorString("not supported");
-  return error;
-}
-
-if (log)
-  log->Printf("NativeProcessLinux::%s read %" PRIu64
-  " memory region entries from /proc/%" PRIu64 "/maps",
-  __FUNCTION__,
-  static_cast(m_mem_region_cache.size()), GetID());
-
-// We support memory retrieval, remember that.
-m_supports_mem_region = LazyBool::eLazyBoolYes;
-  } else {
-if (log)
-  log->Printf("NativeProcessLinux::%s reusing %" PRIu64
-  " cached memory region entries",
-  __FUNCTION__,
-  static_cast(m_mem_region_cache.size()));
+  Error error = PopulateMemoryRegionCache();
+  if (error.Fail()) {
+return error;
   }
 
   lldb::addr_t prev_base_address = 0;
@@ -1760,7 +1706,7 @@
   // There can be a ton of regions on pthreads apps with lots of threads.
   for (auto it = m_mem_region_cache.begin(); it != m_mem_region_cache.end();
++it) {
-MemoryRegionInfo _entry_info = *it;
+MemoryRegionInfo _entry_info = it->first;
 
 // Sanity check assumption that /proc/{pid}/maps entries are ascending.
 assert((proc_entry_info.GetRange().GetRangeBase() >= prev_base_address) &&
@@ -1803,6 +1749,67 @@
   return error;
 }
 
+Error NativeProcessLinux::PopulateMemoryRegionCache() {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  // If our cache is empty, pull the latest.  There should always be at least
+  // one memory region if memory region handling is supported.
+  if (!m_mem_region_cache.empty()) {
+if (log)
+  log->Printf("NativeProcessLinux::%s reusing %" PRIu64
+  " cached memory region entries",

[Lldb-commits] [PATCH] D28233: Improve the performance of jModulesInfo in lldb-server

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer marked an inline comment as done.
tberghammer added a comment.

On some extremely large cases I seen the response time to fell from ~120s to 
~8s (default packet timeout is 2s).


https://reviews.llvm.org/D28233



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

[Lldb-commits] [PATCH] D27394: Fix expression evaluation inside lambda functions for gcc

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer abandoned this revision.
tberghammer added a comment.

Based on the feedback will try to fix the Clang AS importer instead (don't have 
any ETA for it)


https://reviews.llvm.org/D27394



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

[Lldb-commits] [PATCH] D28233: Improve the performance of jModulesInfo in lldb-server

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer created this revision.
tberghammer added a reviewer: labath.
tberghammer added a subscriber: lldb-commits.

Improve the performance of jModulesInfo in lldb-server

Previously it parsed /proc//maps for every module separately
resulting in a very slow response time. This CL add some caching and
optimizes the implementation to improve the code from O(n*m) to O(n+m)
where n is the number of modules requested and m is the number of
files mapped into memory.


https://reviews.llvm.org/D28233

Files:
  source/Plugins/Process/Linux/NativeProcessLinux.cpp
  source/Plugins/Process/Linux/NativeProcessLinux.h

Index: source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- source/Plugins/Process/Linux/NativeProcessLinux.h
+++ source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -119,7 +119,7 @@
   ArchSpec m_arch;
 
   LazyBool m_supports_mem_region;
-  std::vector m_mem_region_cache;
+  std::vector> m_mem_region_cache;
 
   lldb::tid_t m_pending_notification_tid;
 
@@ -217,6 +217,8 @@
   void ThreadWasCreated(NativeThreadLinux );
 
   void SigchldHandler();
+
+  Error PopulateMemoryRegionCache();
 };
 
 } // namespace process_linux
Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1689,68 +1689,14 @@
   // Assume proc maps entries are in ascending order.
   // FIXME assert if we find differently.
 
-  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
-  Error error;
-
   if (m_supports_mem_region == LazyBool::eLazyBoolNo) {
 // We're done.
-error.SetErrorString("unsupported");
-return error;
+return Error("unsupported");
   }
 
-  // If our cache is empty, pull the latest.  There should always be at least
-  // one memory region
-  // if memory region handling is supported.
-  if (m_mem_region_cache.empty()) {
-error = ProcFileReader::ProcessLineByLine(
-GetID(), "maps", [&](const std::string ) -> bool {
-  MemoryRegionInfo info;
-  const Error parse_error =
-  ParseMemoryRegionInfoFromProcMapsLine(line, info);
-  if (parse_error.Success()) {
-m_mem_region_cache.push_back(info);
-return true;
-  } else {
-if (log)
-  log->Printf("NativeProcessLinux::%s failed to parse proc maps "
-  "line '%s': %s",
-  __FUNCTION__, line.c_str(), error.AsCString());
-return false;
-  }
-});
-
-// If we had an error, we'll mark unsupported.
-if (error.Fail()) {
-  m_supports_mem_region = LazyBool::eLazyBoolNo;
-  return error;
-} else if (m_mem_region_cache.empty()) {
-  // No entries after attempting to read them.  This shouldn't happen if
-  // /proc/{pid}/maps
-  // is supported.  Assume we don't support map entries via procfs.
-  if (log)
-log->Printf("NativeProcessLinux::%s failed to find any procfs maps "
-"entries, assuming no support for memory region metadata "
-"retrieval",
-__FUNCTION__);
-  m_supports_mem_region = LazyBool::eLazyBoolNo;
-  error.SetErrorString("not supported");
-  return error;
-}
-
-if (log)
-  log->Printf("NativeProcessLinux::%s read %" PRIu64
-  " memory region entries from /proc/%" PRIu64 "/maps",
-  __FUNCTION__,
-  static_cast(m_mem_region_cache.size()), GetID());
-
-// We support memory retrieval, remember that.
-m_supports_mem_region = LazyBool::eLazyBoolYes;
-  } else {
-if (log)
-  log->Printf("NativeProcessLinux::%s reusing %" PRIu64
-  " cached memory region entries",
-  __FUNCTION__,
-  static_cast(m_mem_region_cache.size()));
+  Error error = PopulateMemoryRegionCache();
+  if (error.Fail()) {
+return error;
   }
 
   lldb::addr_t prev_base_address = 0;
@@ -1760,7 +1706,7 @@
   // There can be a ton of regions on pthreads apps with lots of threads.
   for (auto it = m_mem_region_cache.begin(); it != m_mem_region_cache.end();
++it) {
-MemoryRegionInfo _entry_info = *it;
+MemoryRegionInfo _entry_info = it->first;
 
 // Sanity check assumption that /proc/{pid}/maps entries are ascending.
 assert((proc_entry_info.GetRange().GetRangeBase() >= prev_base_address) &&
@@ -1803,6 +1749,66 @@
   return error;
 }
 
+Error NativeProcessLinux::PopulateMemoryRegionCache() {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  // If our cache is empty, pull the latest.  There should always be at least
+  // one memory region if memory region handling is supported.
+  if (m_mem_region_cache.empty()) {
+Error error = 

[Lldb-commits] [PATCH] D27394: Fix expression evaluation inside lambda functions for gcc

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added a reviewer: aprantl.
tberghammer added a comment.

+Adrian

Thank you for all the comment. I agree that teaching the AST Importer to be 
able to handle types inside a DeclBlock would be the best long term solution 
but I am not sure about its complexity and feasibility (will take a look).

For the test Jim pasted clang ~ToT generates the following (IMO fairly bad) 
debug info:

   Compilation Unit @ offset 0x0:
Length:0xe5 (32-bit)
Version:   4
Abbrev Offset: 0x0
Pointer Size:  8
  <0>: Abbrev Number: 1 (DW_TAG_compile_unit)
DW_AT_producer: (indirect string, offset: 0x0): clang version 
4.0.0 (http://llvm.org/git/clang.git 12dcbf43701c142e8313d322c14b53a6c2957826) 
(http://llvm.org/git/llvm.git 8ab193a8a5383b6a2ddca138c76cfd43bcff6a09) 
 <10>   DW_AT_language: 4   (C++)
 <12>   DW_AT_name: (indirect string, offset: 0xa5): foo.cpp
 <16>   DW_AT_stmt_list   : 0x0 
 <1a>   DW_AT_comp_dir: (indirect string, offset: 0xad): 
/home/tberghammer/tmp  
 <1e>   DW_AT_low_pc  : 0x400510
 <26>   DW_AT_high_pc : 0x75
  <1><2a>: Abbrev Number: 2 (DW_TAG_subprogram)
 <2b>   DW_AT_low_pc  : 0x400510
 <33>   DW_AT_high_pc : 0x75
 <37>   DW_AT_frame_base  : 1 byte block: 56(DW_OP_reg6 (rbp))
 <39>   DW_AT_name: (indirect string, offset: 0xc3): main   
 <3d>   DW_AT_decl_file   : 1   
 <3e>   DW_AT_decl_line   : 3   
 <3f>   DW_AT_type: <0xc2>  
 <43>   DW_AT_external: 1   
  <2><43>: Abbrev Number: 3 (DW_TAG_formal_parameter)
 <44>   DW_AT_location: 2 byte block: 91 78 (DW_OP_fbreg: -8)
 <47>   DW_AT_name: (indirect string, offset: 0xc8): argc   
 <4b>   DW_AT_decl_file   : 1   
 <4c>   DW_AT_decl_line   : 3   
 <4d>   DW_AT_type: <0xc2>  
  <2><51>: Abbrev Number: 3 (DW_TAG_formal_parameter)
 <52>   DW_AT_location: 2 byte block: 91 70 (DW_OP_fbreg: -16)
 <55>   DW_AT_name: (indirect string, offset: 0xcd): argv   
 <59>   DW_AT_decl_file   : 1   
 <5a>   DW_AT_decl_line   : 3   
 <5b>   DW_AT_type: <0xc9>  
  <2><5f>: Abbrev Number: 4 (DW_TAG_lexical_block)
 <60>   DW_AT_low_pc  : 0x40053a
 <68>   DW_AT_high_pc : 0x1f
  <3><6c>: Abbrev Number: 5 (DW_TAG_variable)
 <6d>   DW_AT_location: 2 byte block: 91 68 (DW_OP_fbreg: -24)
 <70>   DW_AT_name: (indirect string, offset: 0xd7): my_foo 
 <74>   DW_AT_decl_file   : 1   
 <75>   DW_AT_decl_line   : 11  
 <76>   DW_AT_type: <0x97>  
  <3><7a>: Abbrev Number: 0
  <2><7b>: Abbrev Number: 4 (DW_TAG_lexical_block)
 <7c>   DW_AT_low_pc  : 0x400563
 <84>   DW_AT_high_pc : 0x1a
  <3><88>: Abbrev Number: 5 (DW_TAG_variable)
 <89>   DW_AT_location: 2 byte block: 91 60 (DW_OP_fbreg: -32)
 <8c>   DW_AT_name: (indirect string, offset: 0xd7): my_foo 
 <90>   DW_AT_decl_file   : 1   
 <91>   DW_AT_decl_line   : 20  
 <92>   DW_AT_type: <0xac>  
  <3><96>: Abbrev Number: 0
  <2><97>: Abbrev Number: 6 (DW_TAG_structure_type)
 <98>   DW_AT_name: (indirect string, offset: 0xda): foo
 <9c>   DW_AT_byte_size   : 8   
 <9d>   DW_AT_decl_file   : 1   
 <9e>   DW_AT_decl_line   : 7   
  <3><9f>: Abbrev Number: 7 (DW_TAG_member)
DW_AT_name: (indirect string, offset: 0xde): value  
DW_AT_type: <0xda>  
DW_AT_decl_file   : 1   
DW_AT_decl_line   : 9   
DW_AT_data_member_location: 0   
  <3>: Abbrev Number: 0
  <2>: Abbrev Number: 6 (DW_TAG_structure_type)
DW_AT_name: (indirect string, offset: 0xda): foo
DW_AT_byte_size   : 2   
DW_AT_decl_file   : 1   
DW_AT_decl_line   : 16  
  <3>: Abbrev Number: 7 (DW_TAG_member)
DW_AT_name: (indirect string, offset: 0xde): value  
DW_AT_type: <0xe1>  
DW_AT_decl_file   : 1   
DW_AT_decl_line   : 18  
DW_AT_data_member_location: 0   
  <3>: Abbrev Number: 0
  <2>: Abbrev Number: 0
  <1>: Abbrev Number: 8 (DW_TAG_base_type)
DW_AT_name: (indirect string, offset: 0xe9): int
DW_AT_encoding: 5   (signed)
DW_AT_byte_size   : 4   
  <1>: Abbrev Number: 9 (DW_TAG_pointer_type)
DW_AT_type: <0xce>  
  <1>: Abbrev Number: 9 (DW_TAG_pointer_type)
DW_AT_type: <0xd3>  
  <1>: Abbrev Number: 8 (DW_TAG_base_type)
DW_AT_name: (indirect string, offset: 0xd2): char   
DW_AT_encoding: 6   (signed char)
DW_AT_byte_size   : 1   
  <1>: Abbrev Number: 8 (DW_TAG_base_type)
DW_AT_name: (indirect string, offset: 0xe4): long int   

[Lldb-commits] [PATCH] D27394: Fix expression evaluation inside lambda functions for gcc

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer created this revision.
tberghammer added a reviewer: clayborg.
tberghammer added a subscriber: lldb-commits.

Fix expression evaluation inside lambda functions for gcc

gcc emits the artificial struct created for lambda expressions inside a
lexical block what is not understood by clang (it emits them at the
compile unit level). This CL changes the dwarf parsing code to move
types defined inside a lexical block to the first parent what is not a
lexical block to avoid this clang limitation. This will make the
experience between clang and gcc consistent as clang moves these types
out of the lexical blocks before emitting the debug info.


https://reviews.llvm.org/D27394

Files:
  packages/Python/lldbsuite/test/expression_command/lambda/Makefile
  packages/Python/lldbsuite/test/expression_command/lambda/TestLambda.py
  packages/Python/lldbsuite/test/expression_command/lambda/main.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3777,7 +3777,37 @@
 const DWARFDIE , DWARFDIE *decl_ctx_die_copy) {
   SymbolFileDWARF *dwarf = die.GetDWARF();
 
+  bool is_type = false;
+  switch (die.Tag()) {
+case DW_TAG_base_type:
+case DW_TAG_class_type:
+case DW_TAG_const_type:
+case DW_TAG_pointer_type:
+case DW_TAG_reference_type:
+case DW_TAG_restrict_type:
+case DW_TAG_rvalue_reference_type:
+case DW_TAG_structure_type:
+case DW_TAG_typedef:
+case DW_TAG_union_type:
+case DW_TAG_unspecified_type:
+case DW_TAG_volatile_type:
+  is_type = true;
+  break;
+default:
+  break;
+  }
+
   DWARFDIE decl_ctx_die = dwarf->GetDeclContextDIEContainingDIE(die);
+  if (is_type) {
+// The clang AST importer can't handle types declared inside a BlockDecl.
+// Move these declarartions into the parent context of the block. This will
+// make the behavior from gcc (what emits type information inside lexical
+// blocks) consistent with the way clang emits debug info for types defined
+// inside a function (class, namespace or compile unit level).
+while (decl_ctx_die.Tag() == DW_TAG_lexical_block) {
+  decl_ctx_die = dwarf->GetDeclContextDIEContainingDIE(decl_ctx_die);
+}
+  }
 
   if (decl_ctx_die_copy)
 *decl_ctx_die_copy = decl_ctx_die;
Index: packages/Python/lldbsuite/test/expression_command/lambda/main.cpp
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/lambda/main.cpp
@@ -0,0 +1,6 @@
+int main() {
+  auto l = [](int a, int b) {
+return a + b; // Break here 1
+  };
+  return l(123, 456); // Break here 2
+}
Index: packages/Python/lldbsuite/test/expression_command/lambda/TestLambda.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/lambda/TestLambda.py
@@ -0,0 +1,73 @@
+from __future__ import print_function
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class LambdaTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+self.main_source = "main.cpp"
+self.main_source_spec = lldb.SBFileSpec(self.main_source)
+self.exe = os.path.join(os.getcwd(), "a.out")
+
+def test_expression_inside_lambda(self):
+self.build()
+
+target = self.dbg.CreateTarget(self.exe)
+self.assertTrue(target)
+
+breakpoint = target.BreakpointCreateBySourceRegex(
+'// Break here 1', self.main_source_spec)
+self.assertTrue(breakpoint)
+
+# Launch the process, and do not stop at the entry point.
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(process)
+
+threads = lldbutil.get_threads_stopped_at_breakpoint(
+process, breakpoint)
+self.assertEqual(len(threads), 1)
+
+frame = threads[0].GetFrameAtIndex(0)
+
+value = frame.EvaluateExpression("a + b")
+self.assertTrue(value.IsValid())
+self.assertTrue(value.GetError().Success())
+self.assertEqual(value.GetValueAsSigned(0), 579)
+
+@expectedFailureAll(compiler="gcc")
+def test_expression_call_lambda(self):
+self.build()
+
+target = self.dbg.CreateTarget(self.exe)
+self.assertTrue(target)
+
+breakpoint = target.BreakpointCreateBySourceRegex(
+'// Break here 2', self.main_source_spec)
+self.assertTrue(breakpoint)
+
+# Launch the process, and do not stop at the entry point.
+process 

[Lldb-commits] [PATCH] D27305: Replace __ANDROID_NDK__ with simply ANDROID

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added inline comments.



Comment at: include/lldb/Core/RegularExpression.h:34
 #else
-#if __ANDROID_NDK__
+#if ANDROID
 #include 

In most case you use "#ifdef". It would be nice to uniformalize



Comment at: source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp:31
 
-#if __ANDROID_NDK__
+#if ANDROID
 #include 

In most case you use "#ifdef". It would be nice to uniformalize



Comment at: 
source/Plugins/Process/Utility/RegisterContextMacOSXFrameBackchain.cpp:123
 !defined(_MSC_VER) && !defined(__mips__) && !defined(__powerpc__) &&   
\
-!defined(__ANDROID_NDK__)
+!defined(ANDROID)
 case sizeof(long double):

(unrelated): What does an ANDROID specific define do in a MacOSX specific file?


https://reviews.llvm.org/D27305



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