[Lldb-commits] [PATCH] D54057: Remove Go debugger plugin
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] D54059: Remove Java debugger plugin
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] D54060: Remove OCaml debugger plugin
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
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] D56170: RangeMap.h: merge RangeDataArray and RangeDataVector
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] D36046: Improve the posix core file triple detection
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] D39545: Support scoped enums in the DWARF AST parser
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] D39545: Support scoped enums in the DWARF AST parser
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 &decl,
- const CompilerType &integer_clang_type) {
+ const CompilerType &integer_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] ? &ast->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 &decl,
- const CompilerType &integer_qual_type);
+ const CompilerType &integer_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 inf
[Lldb-commits] [PATCH] D39825: [lldb] Fix cu_offset for dwo/dwp used by DWARFCompileUnit::Index
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] D39825: [lldb] Fix cu_offset for dwo/dwp used by DWARFCompileUnit::Index
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
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] D41909: Fix deadlock in dwarf logging
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] D42563: [lldb] attempt to fix DIERef::GetUID
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] D42563: [lldb] attempt to fix DIERef::GetUID
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
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
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] D30272: Improve data formatter for libstdcpp unique_ptr
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] D31368: Add support for sythetic operator dereference
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()
self.assertTrue(frame.Is
[Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers
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 &new_name) {
+ return ValueObjectCast::Create(*this, new_name, GetCompilerType());
+}
+
ValueObjectSP ValueObject::CastPointerType(const char *name,
CompilerType &compiler_type) {
ValueObjectSP valobj_sp;
Index: include/lldb/Core/ValueObject.h
===
--- include/lldb/Core/ValueObject.h
+++ include/lldb/Core/ValueObje
[Lldb-commits] [PATCH] D31366: Do not dereference std::unique_ptr by default
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] D31366: Do not dereference std::unique_ptr by default
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] D31368: Add support for sythetic operator dereference
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 = f
[Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers
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
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: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUn
[Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers
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 &new_name) {
+ return ValueObjectCast::Create(*this, new_name, GetCompilerType());
+}
+
ValueObjectSP ValueObject::CastPointerType(const char *name,
CompilerType &compiler_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
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: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFo
[Lldb-commits] [PATCH] D31366: Do not dereference std::unique_ptr by default
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] D31451: New C++ function name parsing logic
tberghammer added a comment. One note on benchmarking: A did some performance profiling on LLDB using a similar approach to what Pavel suggested and if I remember correctly only ~10% of the time was spent on C++ name parsing (~15% was C++ demangling, ~50% was debug_info parsing, rest of them was fairly well distributed). Because of this I think some targeted micro benchmark will be much more useful to measure the performance of this code then an end-to-end test as an e2e test would have low signal to noise ratio. https://reviews.llvm.org/D31451 ___ 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
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&id=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 &name) {
- 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] D31368: Add support for sythetic operator dereference
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&id=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
[Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers
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&id=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 &new_name) {
+ return ValueObjectCast::Create(*this, new_name, GetCompilerType());
+}
+
ValueObjectSP ValueObject::CastPointerType(const char *name,
CompilerType &compiler_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(
+offse
[Lldb-commits] [Diffusion] rL299714: iwyu fixes for lldbCore.
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] D31882: Fix the libc++ std::vector data formatter
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 &name) 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, &byte, 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 begin_sp(
+ valobj_sp->GetChildMemberWit
[Lldb-commits] [PATCH] D31880: Fix libc++ vector data formatter (bug #32553)
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
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] D32441: Remove the home-grown android toolchain file and all references to it
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] D32441: Remove the home-grown android toolchain file and all references to it
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] D32503: Remove unused code related to CPlusPlusLanguage::FindEquivalentNames
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] D32503: Remove unused code related to CPlusPlusLanguage::FindEquivalentNames
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] D32757: Add TaskMap for iterating a function over a set of integers
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] D32813: ABISysV_arm64: compute return value for large vectors correctly
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] D32813: ABISysV_arm64: compute return value for large vectors correctly
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] D32930: New framework for lldb client-server communication tests.
tberghammer added a comment.
I few high level comments after a quick read through.
Comment at: unittests/tools/lldb-server/.gitignore:1
+thread_inferior
Why do we need this? I would expect cmake to *not* put anything into my source
directory when doing an out-of-source build so I can have multiple build
directory (e.g. for different targets) at the same time.
Comment at: unittests/tools/lldb-server/inferior/thread_inferior.cpp:21
+
+ asm volatile ("int3");
+ delay = false;
krytarowski wrote:
> int3 is specific to x86.
>
> Some instructions to emit software breakpoint in other architectures:
> https://github.com/NetBSD/src/commit/c215d1b7d6c1385720b27fd45189b5dea6d6e4aa
>
> Also there is a support for threads, this is not as portable as it could be.
> The simplest example could be without them, and threads in debuggee could be
> tested in dedicated regression tests. This will help out to sort bugs with
> threads from other features.
I think there should be a compiler intrinsic available as well.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:17
+
+class ProcessInfo {
+public:
Can we somehow reuse the logic in GDBRemoteCommunicationClient for parsing the
packets (possibly after factoring out some of it into new standalone
functions)? I think it would remove code duplication as well as increase the
coverage provided by these tests.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:82-87
+// Common functions for parsing packet data.
+std::unordered_map SplitPairList(const std::string&
s);
+std::vector SplitList(const std::string& s, char delimeter);
+std::pair SplitPair(const std::string& s);
+std::string HexDecode(const std::string& hex_encoded);
+unsigned long SwitchEndian(const std::string& little_endian);
Does these have to be global? Can we make them local to the .cc file (anonymous
namespace or file static variable)?
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);
What if the key isn't unique?
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:86-87
+std::pair SplitPair(const std::string& s);
+std::string HexDecode(const std::string& hex_encoded);
+unsigned long SwitchEndian(const std::string& little_endian);
+}
I would hope we have functions in LLVM/LLDB doing these sort of things (might
have to be changed slightly to make them easily accessible)
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.
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] D32930: New framework for lldb client-server communication tests.
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] D34199: Tweak SysV_arm64 function entry unwind plan
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] D34352: [linux] Change the way we load vdso pseudo-module
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] D34750: [UnwindAssembly/x86] Add support for "lea imm(%ebp), %esp" pattern
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] D34750: [UnwindAssembly/x86] Add support for "lea imm(%ebp), %esp" pattern
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] D35734: Don't allow LLDB to try and parse .debug_types
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] D35734: Don't allow LLDB to try and parse .debug_types
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
tberghammer added a comment. I tried to compile the code snippet you mentioned but it doesn't put anything into the debug_types section. If I move the enum definition outside of the class then it produces debug_type section but LLDB fails back gracefully without any crash or error message. I also tried a few other code snippets and the debugging experience usually wasn't great (types of local variables were messed up) but I haven't manage to get LLDB crashing. Repository: rL LLVM 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] D11465: Fix "process load/unload" on android
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] D36046: Improve the posix core file triple detection
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] D36046: Improve the posix core file triple detection
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] D36068: Add support for base address entries in the .debug_ranges section
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 &debug_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 &debug_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] D36068: Add support for base address entries in the .debug_ranges section
tberghammer added a comment. Submitted as https://reviews.llvm.org/rL309554 to get the buildbot using ToT clang green again but if you have any comment then let me know and I will address it in a followup CL. https://reviews.llvm.org/D36068 ___ 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
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] D36046: Improve the posix core file triple detection
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] D36046: Improve the posix core file triple detection
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] D37930: Use ThreadLauncher to launch TaskPool threads
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] D37930: Use ThreadLauncher to launch TaskPool threads
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] D38492: [lldb] Fix initialization of m_debug_cu_index_map
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] D38568: [lldb] Enable using out-of-tree dwps
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] D38568: [lldb] Enable using out-of-tree dwps
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] D39283: [lldb-dev] Update LLDB test cases for 'inlineStepping'
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] D27161: Fix floating point register reads x86_64 linux on targets with no AVX support
tberghammer accepted this revision. tberghammer added a comment. LGTM Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:246-247 + + NativeRegisterContextLinux * (nit): Please revert https://reviews.llvm.org/D27161 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27305: Replace __ANDROID_NDK__ with simply ANDROID
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
[Lldb-commits] [PATCH] D27394: Fix expression evaluation inside lambda functions for gcc
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 &die, 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.
+proce
[Lldb-commits] [PATCH] D27394: Fix expression evaluation inside lambda functions for gcc
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 DW
[Lldb-commits] [PATCH] D28233: Improve the performance of jModulesInfo in lldb-server
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 &thread);
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 &line) -> 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 &proc_entry_info = *it;
+MemoryRegionInfo &proc_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 = ProcFileReader::ProcessLineBy
[Lldb-commits] [PATCH] D27394: Fix expression evaluation inside lambda functions for gcc
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
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] D28233: Improve the performance of jModulesInfo in lldb-server
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&id=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 &thread);
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 &line) -> 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 &proc_entry_info = *it;
+MemoryRegionInfo &proc_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] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests
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 *&name_cstr, std::string &scope,
std::string &basename,
TypeClass &type_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 &&
-namespace_separator > template_arg_char) // but namespace
[Lldb-commits] [PATCH] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests
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,
exact_match);
@@ -145,8 +145,8 @@
ConstString match_type_name_const_str(the_type->GetQual
[Lldb-commits] [PATCH] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests
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&id=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 *&name_cstr, std::string &scope,
- std::string &basename,
+bool Type::GetTypeScopeAndBasename(const llvm::StringRef& name,
+ llvm::StringRef &scope,
+ llvm::StringRef &basename,
TypeClass &type_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) {
[Lldb-commits] [PATCH] D28775: [cmake] Make lldb build with the android ndk toolchain file
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] D30251: Map ELF files as writable so we can update the relocations in them
tberghammer created this revision.
Map ELF files as writable so we can update the relocations in them
After this change ELF files will be mapped as private writable pages so
we can cheaply update the relocations inside them without have to read
them into memory and any modification we do in the memory will not be
reflected in the on disk files.
https://reviews.llvm.org/D30251
Files:
include/lldb/Host/FileSpec.h
source/Host/common/FileSpec.cpp
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -386,7 +386,7 @@
lldb::offset_t file_offset,
lldb::offset_t length) {
if (!data_sp) {
-data_sp = file->MemoryMapFileContentsIfLocal(file_offset, length);
+data_sp = file->MemoryMapFileContentsIfLocal(file_offset, length, true);
data_offset = 0;
}
@@ -396,7 +396,7 @@
if (ELFHeader::MagicBytesMatch(magic)) {
// Update the data to contain the entire file if it doesn't already
if (data_sp->GetByteSize() < length) {
-data_sp = file->MemoryMapFileContentsIfLocal(file_offset, length);
+data_sp = file->MemoryMapFileContentsIfLocal(file_offset, length, true);
data_offset = 0;
magic = data_sp->GetBytes();
}
@@ -654,7 +654,8 @@
if (header.HasHeaderExtension() &&
section_header_end > data_sp->GetByteSize()) {
data_sp = file.MemoryMapFileContentsIfLocal (file_offset,
- section_header_end);
+ section_header_end,
+ true);
data.SetData(data_sp);
lldb::offset_t header_offset = data_offset;
header.Parse(data, &header_offset);
@@ -667,7 +668,8 @@
header.e_shoff + header.e_shnum * header.e_shentsize;
if (section_header_end > data_sp->GetByteSize()) {
data_sp = file.MemoryMapFileContentsIfLocal(file_offset,
-section_header_end);
+section_header_end,
+true);
data.SetData(data_sp);
}
@@ -712,7 +714,7 @@
header.e_phoff + header.e_phnum * header.e_phentsize;
if (program_headers_end > data_sp->GetByteSize()) {
data_sp = file.MemoryMapFileContentsIfLocal(
- file_offset, program_headers_end);
+ file_offset, program_headers_end, true);
data.SetData(data_sp);
}
ProgramHeaderColl program_headers;
@@ -727,16 +729,19 @@
if (segment_data_end > data_sp->GetByteSize()) {
data_sp = file.MemoryMapFileContentsIfLocal(file_offset,
- segment_data_end);
+ segment_data_end,
+ true);
data.SetData(data_sp);
}
core_notes_crc =
CalculateELFNotesSegmentsCRC32(program_headers, data);
} else {
// Need to map entire file into memory to calculate the crc.
data_sp =
-file.MemoryMapFileContentsIfLocal(file_offset, SIZE_MAX);
+file.MemoryMapFileContentsIfLocal(file_offset,
+ SIZE_MAX,
+ true);
data.SetData(data_sp);
gnu_debuglink_crc = calc_gnu_debuglink_crc32(
data.GetDataStart(), data.GetByteSize());
Index: source/Host/common/FileSpec.cpp
===
--- source/Host/common/FileSpec.cpp
+++ source/Host/common/FileSpec.cpp
@@ -837,23 +837,28 @@
// verified using the DataBuffer::GetByteSize() function.
//--
DataBufferSP FileSpec::MemoryMapFileContents(off_t file_offset,
- size_t file_size) const {
+ size_t file_size,
+ bool writeable) const {
DataBufferSP data_sp;
std::unique_ptr mmap_data(new DataBufferMemoryMap());
if (mmap_data.get()) {
const size_t mapped_length =
-mmap_data->MemoryMapFromFileSpec(this, file_offset, file_size);
+mmap_data
[Lldb-commits] [PATCH] D30272: Improve data formatter for libstdcpp unique_ptr
tberghammer created this revision.
Improve data formatter for libstdcpp unique_ptr
- Fix infinite loop when there is a reference loop created from smart pointers
- Respect pointer depth argument in frame variable command
- Support dereferencing unique_ptr in the frame variable command
- Support operator-> for unique_ptr in the frame variable command
Note: After submitting this change I plan to add the same functionality for the
other smart pointers as well (shared_ptr/weak_ptr, libcpp/libstdcpp)
https://reviews.llvm.org/D30272
Files:
include/lldb/Core/ValueObject.h
include/lldb/DataFormatters/ValueObjectPrinter.h
include/lldb/Target/Language.h
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/main.cpp
source/Core/ValueObject.cpp
source/DataFormatters/ValueObjectPrinter.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
source/Target/Language.cpp
source/Target/StackFrame.cpp
Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -604,8 +604,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();
@@ -622,11 +624,26 @@
return ValueObjectSP();
}
}
+
+ // If we have operator-> and it is a pointer like value object then we
+ // dereference it and then handle it as operator.
+ if (valobj_sp->IsPointerLikeObject()) {
+Error deref_error;
+ValueObjectSP deref_sp = valobj_sp->Dereference(deref_error);
+if (deref_sp && error.Success()) {
+ valobj_sp = deref_sp;
+ expr_is_ptr = false;
+} else {
+ error.SetErrorStringWithFormat(
+ "Failed to dereference a pointer like object: %s",
+ deref_error.AsCString());
+ 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/Target/Language.cpp
===
--- source/Target/Language.cpp
+++ source/Target/Language.cpp
@@ -413,6 +413,9 @@
s.Printf("Exception breakpoint (catch: %s throw: %s)",
catch_on ? "on" : "off", throw_on ? "on" : "off");
}
+
+bool Language::IsPointerLikeObject(ValueObject &valobj) { return false; }
+
//--
// Constructor
//--
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -101,6 +101,8 @@
HardcodedFormatters::HardcodedSyntheticFinder
GetHardcodedSynthetics() override;
+ bool IsPointerLikeObject(ValueObject &valobj) override;
+
//--
// Static Functions
//--
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -1164,3 +1164,12 @@
return g_formatters;
}
+
+bool CPlusPlusLanguage::IsPointerLikeObject(ValueObject &valobj) {
+ static RegularExpression unique_ptr_regex("^std::unique_ptr<.+>(( )?&)?$");
+
+ llvm::StringRef name(valobj.GetCompilerType().GetTypeName().GetStringRef());
+ if (unique_ptr_regex.Execute(name))
+return true;
+ return false;
+}
Index: source/DataFormatters/ValueObjectPrinter.cpp
===
--- source/DataFormatters/ValueObjectPrinter.cpp
+++ source/DataFormatters/ValueObjectPrinter.cpp
@@ -63,6 +63,7 @@
m_is_ptr = eLazyBoolCalculate;
m_is_ref = eLazyBoolCalculate;
m_is_aggregate = eLazyBoolCalculate;
+ m_is_pointer_like = eLazyBoolCalculate;
m_is_instance_ptr = eLazyBoolCalculate;
m_summary_formatter = {nullptr, false};
m_value.assign("");
@@ -208,6 +209,
[Lldb-commits] [PATCH] D30272: Improve data formatter for libstdcpp unique_ptr
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] D30272: Improve data formatter for libstdcpp unique_ptr
tberghammer added a comment. Thank you the comments. Based on them I have the following proposal: - Add a new property to value object with name "m_is_dereference_of_parent" - Add a new static method named "CreateCopy(name, valobj, is_dereference_of_parent)" to ValueObject and to SBValue what will create a new value object with the same type and data as the original value object with m_is_dereference_of_parent is set (I am open to suggestions for a better name). The reason I need to create a copy of the value object instead of modifying it in place is that if the dereferenced object is referenced without using a synthetic child provider then this flag shouldn't be set and the cleanest way to achieve this is to have 2 separate value object instances. - From the synthetic child providers when we want to create a child what is the dereference of the parent (e.g. object for unique_ptr) then we will use the above CreateCopy method - We will decrement the pointer depth during display every time we go through an object marked as m_is_dereference_of_parent - In "frame variable" when the user uses "operator*" or "operator->" we return the first child with m_is_dereference_of_parent (possibly return an error if multiple child has it set) I think this approach satisfies all the requirements we had (extensibility, dereference support, loop detection) and makes the API fairly clean. What do you think? 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
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] D30410: testsuite/android: build test executables with the android ndk directly
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] D30624: Remove all of LLDB's custom filesystem statting code.
tberghammer added a comment. Can you make the usage of "using namespace llvm::sys::fs;" a bit more consistent? Sometime you write fully qualified name, sometime you add it to the top of the file while sometime only to the function where it is used. Comment at: llvm/include/llvm/Support/FileSystem.h:482-486 /// @brief Does status represent a directory? /// /// @param status A file_status previously returned from status. /// @returns status.type() == file_type::directory_file. +file_type get_file_type(const Twine &Path); Please fix the comment https://reviews.llvm.org/D30624 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
