[Lldb-commits] [PATCH] D31367: Expression: add missing linkage to RuntimeDyld component
mgorny added a comment. In https://reviews.llvm.org/D31367#713472, @labath wrote: > We don't depend on the RuntimeDyld component of llvm directy -- we only use > it indirectly through the ExecutionEngine component. Shouldn't that be > reflected as a dependency in the build system somehow, so that the former can > be pulled in directly ? > RuntimeDyld is listed as a dependency of ExecutionEngine in > lib/ExecutionEngine/LLVMBuild.txt, but that does not seem to be reflected in > the cmake? Is that a bug on the llvm side? I think it's not that simple. If that was a plain missing dependency in ExecutionEngine, then linking of shared ExecutionEngine would fail due to missing symbols. Since it doesn't, and it makes LLDB libraries fail, I presume this is the kind of indirect dependency that somehow forces a direct dependency via the headers. If you can confirm it's that, and that we want to implicitly force linking RuntimeDyld to all ExecutionEngine consumers, then I'll look if we can do that properly within LLVM CMake files or extend our support for dependencies for that. However, I think that only makes sense if the dependency is indeed frequently indirectly exposed and not e.g. dependent on use of some uncommon thingy. Repository: rL LLVM https://reviews.llvm.org/D31367 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31485: Verify memory address range validity in GDBRemoteCommunicationClient
labath added a comment. The additional check sounds fine. There's a test for this class in unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp where you can add a test for this behavior. https://reviews.llvm.org/D31485 ___ 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
labath added a comment.
I cant help but feel that this could have been done in a simpler way, but then
again, some of the cases you have dug up are quite tricky.
I think we should do some performance measurements to see whether this needs
more optimising or it's fine as is.
I propose the following benchmark:
bin/lldb bin/clang
- make sure clang is statically linked
breakpoint set --name non_exisiting_function
Clang needs to be statically linked so we can access all its symbols without
running it (which would skew the benchmark) -- this is the reason we cannot
use lldb itself as most of its symbols are in liblldb.
Setting the breakpoint on a nonexistent function avoids us timing the
breakpoint setting machinery, while still getting every symbol in the
executable parsed.
If the performance is ok i am quite happy with this, apart from some stylistic
nits.
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:94
if (!m_parsed && m_full) {
-//ConstString mangled;
-//m_full.GetMangledCounterpart(mangled);
-//printf ("\n parsing = '%s'\n", m_full.GetCString());
-//if (mangled)
-//printf (" mangled = '%s'\n", mangled.GetCString());
-m_parse_error = false;
-m_parsed = true;
-llvm::StringRef full(m_full.GetCString());
-
-size_t arg_start, arg_end;
-llvm::StringRef parens("()", 2);
-if (ReverseFindMatchingChars(full, parens, arg_start, arg_end)) {
- m_arguments = full.substr(arg_start, arg_end - arg_start + 1);
- if (arg_end + 1 < full.size())
-m_qualifiers = full.substr(arg_end + 1);
- if (arg_start > 0) {
-size_t basename_end = arg_start;
-size_t context_start = 0;
-size_t context_end = llvm::StringRef::npos;
-if (basename_end > 0 && full[basename_end - 1] == '>') {
- // TODO: handle template junk...
- // Templated function
- size_t template_start, template_end;
- llvm::StringRef lt_gt("<>", 2);
- if (ReverseFindMatchingChars(full, lt_gt, template_start,
- template_end, basename_end)) {
-// Check for templated functions that include return type like:
-// 'void foo()'
-context_start = full.rfind(' ', template_start);
-if (context_start == llvm::StringRef::npos)
- context_start = 0;
-else
- ++context_start;
-
-context_end = full.rfind(':', template_start);
-if (context_end == llvm::StringRef::npos ||
-context_end < context_start)
- context_end = context_start;
- } else {
-context_end = full.rfind(':', basename_end);
- }
-} else if (context_end == llvm::StringRef::npos) {
- context_end = full.rfind(':', basename_end);
-}
-
-if (context_end == llvm::StringRef::npos)
- m_basename = full.substr(0, basename_end);
-else {
- if (context_start < context_end)
-m_context =
-full.substr(context_start, context_end - 1 - context_start);
- const size_t basename_begin = context_end + 1;
- m_basename =
- full.substr(basename_begin, basename_end - basename_begin);
-}
-m_type = eTypeUnknownMethod;
- } else {
-m_parse_error = true;
-return;
- }
-
- if (!IsValidBasename(m_basename)) {
-// The C++ basename doesn't match our regular expressions so this can't
-// be a valid C++ method, clear everything out and indicate an error
-m_context = llvm::StringRef();
-m_basename = llvm::StringRef();
-m_arguments = llvm::StringRef();
-m_qualifiers = llvm::StringRef();
-m_parse_error = true;
- }
+CPlusPlusNameParser parser(m_full.GetStringRef());
+auto function = parser.ParseAsFunctionDefinition();
How about the following api:
```
if (auto function =
CPlusPlusNameParser::ParseAsFunctionDefinition(m_full.GetStringRef())) {
...
```
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:160
+ auto full_name = parser.ParseAsFullName();
+ if (full_name.hasValue()) {
+identifier = full_name.getValue().m_basename;
Same here
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp:62
+bool CPlusPlusNameParser::HasMoreTokens() {
+ return m_next_token_index < static_cast(m_tokens.size());
+}
Wouldn't it be better to change the type of m_next_token_index to size_t?
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp:188
+ // as arithmetic or shift operators not as template brackets.
+ // Examples: std::enable_if<(10u)<(64), bool>
+ // f>
Is this really the case for the types we are interested in? I
[Lldb-commits] [PATCH] D31367: Expression: add missing linkage to RuntimeDyld component
beanz added a comment. This is definitely not the right fix. Please revert. ExecutionEngine's LLVMBuild.txt file explicitly lists that RuntimeDyld is a required library of ExecutionEngine (as well as a few others). LLVM's CMake should be connecting that as an explicit dependency, and for some reason it isn't. Adding over-specified dependencies in LLDB to workaround bugs in LLVM is not the right approach, and masks problems instead of resolving them. Repository: rL LLVM https://reviews.llvm.org/D31367 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31367: Expression: add missing linkage to RuntimeDyld component
beanz added a comment. Please revert your patch. It is *not* the right solution and is masking underlying problems. ExecutionEngine headers directly reference symbols from RuntimeDyld, so we should enforce a requirement that anyone using ExeuctionEngine also link RuntimeDyld. This works today as expected for static archive builds. It is only broken with `BUILD_SHARED_LIBS`. I suspect the problem is that when built as a DSO ExecutionEngine has no unresolved symbols against RuntimeDyld, and the linker probably isn't including the reference to RuntimeDyld in the produced ExecutionEngine DSO. Regardless of the underlying cause, this is not the correct solution, it merely hides a problem that could occur in other consumers of ExecutionEngine. Repository: rL LLVM https://reviews.llvm.org/D31367 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r299095 - Revert r298776 - Expression: add missing linkage to RuntimeDyld ...
Author: mgorny Date: Thu Mar 30 13:24:07 2017 New Revision: 299095 URL: http://llvm.org/viewvc/llvm-project?rev=299095&view=rev Log: Revert r298776 - Expression: add missing linkage to RuntimeDyld ... This needs to be addressed within LLVM itself. Modified: lldb/trunk/source/Expression/CMakeLists.txt Modified: lldb/trunk/source/Expression/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/CMakeLists.txt?rev=299095&r1=299094&r2=299095&view=diff == --- lldb/trunk/source/Expression/CMakeLists.txt (original) +++ lldb/trunk/source/Expression/CMakeLists.txt Thu Mar 30 13:24:07 2017 @@ -34,6 +34,5 @@ add_lldb_library(lldbExpression LINK_COMPONENTS Core ExecutionEngine -RuntimeDyld Support ) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31367: Expression: add missing linkage to RuntimeDyld component
mgorny added a comment. In https://reviews.llvm.org/D31367#714305, @beanz wrote: > Please revert your patch. It is *not* the right solution and is masking > underlying problems. Reverted in r299095. Now I'd really appreciate some help in figuring out how to fix it properly. > ExecutionEngine headers directly reference symbols from RuntimeDyld, so we > should enforce a requirement that anyone using ExeuctionEngine also link > RuntimeDyld. This works today as expected for static archive builds. It is > only broken with `BUILD_SHARED_LIBS`. I suspect the problem is that when > built as a DSO ExecutionEngine has no unresolved symbols against RuntimeDyld, > and the linker probably isn't including the reference to RuntimeDyld in the > produced ExecutionEngine DSO. The DSO is linking to RuntimeDyld. However, obviously the `PRIVATE` linkage forced in `llvm_add_library` is not the correct solution when the symbols are explicitly used in the headers. It should be `PUBLIC` for that particular component. Any suggestion on how to fix that API? Should I add an additional `PUBLIC_LINK_COMPONENTS` (and `PUBLIC_LINK_LIBS`)? Repository: rL LLVM https://reviews.llvm.org/D31367 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31450: Battery of NetBSD support improvements
krytarowski updated this revision to Diff 93524.
krytarowski added a comment.
Herald added a subscriber: srhines.
Apply changes from review. No visible regressions in "check-lldb".
Repository:
rL LLVM
https://reviews.llvm.org/D31450
Files:
source/Host/common/Host.cpp
source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.cpp
source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
source/Plugins/Process/elf-core/ProcessElfCore.cpp
source/Plugins/Process/elf-core/ThreadElfCore.cpp
Index: source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -21,6 +21,7 @@
#include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
#include "Plugins/Process/Utility/RegisterContextLinux_s390x.h"
#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
+#include "Plugins/Process/Utility/RegisterContextNetBSD_x86_64.h"
#include "Plugins/Process/Utility/RegisterContextOpenBSD_i386.h"
#include "Plugins/Process/Utility/RegisterContextOpenBSD_x86_64.h"
#include "Plugins/Process/Utility/RegisterInfoPOSIX_arm.h"
@@ -112,6 +113,17 @@
break;
}
+case llvm::Triple::NetBSD: {
+ switch (arch.GetMachine()) {
+ case llvm::Triple::x86_64:
+reg_interface = new RegisterContextNetBSD_x86_64(arch);
+break;
+ default:
+break;
+ }
+ break;
+}
+
case llvm::Triple::Linux: {
switch (arch.GetMachine()) {
case llvm::Triple::arm:
@@ -144,8 +156,8 @@
reg_interface = new RegisterInfoPOSIX_arm(arch);
break;
case llvm::Triple::x86:
- reg_interface = new RegisterContextOpenBSD_i386(arch);
- break;
+reg_interface = new RegisterContextOpenBSD_i386(arch);
+break;
case llvm::Triple::x86_64:
reg_interface = new RegisterContextOpenBSD_x86_64(arch);
break;
@@ -260,7 +272,6 @@
pr_cstime.tv_sec = data.GetPointer(&offset);
pr_cstime.tv_usec = data.GetPointer(&offset);
-
return error;
}
@@ -317,9 +328,7 @@
//
// Parse SIGINFO from NOTE entry
//
-ELFLinuxSigInfo::ELFLinuxSigInfo() {
- memset(this, 0, sizeof(ELFLinuxSigInfo));
-}
+ELFLinuxSigInfo::ELFLinuxSigInfo() { memset(this, 0, sizeof(ELFLinuxSigInfo)); }
Error ELFLinuxSigInfo::Parse(DataExtractor &data, const ArchSpec &arch) {
Error error;
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -62,8 +62,8 @@
// to ignore possible presence of the header extension.
const size_t header_size = sizeof(llvm::ELF::Elf64_Ehdr);
-auto data_sp =
-DataBufferLLVM::CreateSliceFromPath(crash_file->GetPath(), header_size, 0);
+auto data_sp = DataBufferLLVM::CreateSliceFromPath(crash_file->GetPath(),
+ header_size, 0);
if (data_sp && data_sp->GetByteSize() == header_size &&
elf::ELFHeader::MagicBytesMatch(data_sp->GetBytes())) {
elf::ELFHeader elf_header;
@@ -223,7 +223,7 @@
bool siginfo_signal_found = false;
bool prstatus_signal_found = false;
// Check we found a signal in a SIGINFO note.
- for (const auto &thread_data: m_thread_data) {
+ for (const auto &thread_data : m_thread_data) {
if (thread_data.signo != 0)
siginfo_signal_found = true;
if (thread_data.prstatus_sig != 0)
@@ -233,7 +233,7 @@
// If we don't have signal from SIGINFO use the signal from each threads
// PRSTATUS note.
if (prstatus_signal_found) {
- for (auto &thread_data: m_thread_data)
+ for (auto &thread_data : m_thread_data)
thread_data.signo = thread_data.prstatus_sig;
} else if (m_thread_data.size() > 0) {
// If all else fails force the first thread to be SIGSTOP
@@ -449,6 +449,11 @@
};
}
+namespace NETBSD {
+
+enum { NT_PROCINFO = 1, NT_AUXV, NT_AMD64_REGS = 33, NT_AMD64_FPREGS = 35 };
+}
+
// Parse a FreeBSD NT_PRSTATUS note - see FreeBSD sys/procfs.h for details.
static void ParseFreeBSDPrStatus(ThreadData &thread_data, DataExtractor &data,
ArchSpec &arch) {
@@ -485,13 +490,23 @@
thread_data.name = data.GetCStr(&offset, 20);
}
-stat
Re: [Lldb-commits] [PATCH] D31367: Expression: add missing linkage to RuntimeDyld component
On Thu, Mar 30, 2017 at 06:00:15PM +, Chris Bieneman via Phabricator wrote: > ExecutionEngine headers directly reference symbols from RuntimeDyld, > so we should enforce a requirement that anyone using ExeuctionEngine > also link RuntimeDyld. This works today as expected for static archive > builds. It is only broken with `BUILD_SHARED_LIBS`. Welcome to the brave new world of newer ELF linkers. Just because a library pulls in a symbol into the namespace doesn't mean you can just use that symbol. They want you to explicitly specify the dependency... This does happen for static archives though. Joerg ___ 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] D31450: Battery of NetBSD support improvements
jingham resigned from this revision. jingham added a comment. Kyril, I haven't been involved in the lldb-server parts of lldb. Greg sketched out those interfaces and mostly folks working on Windows & Linux have fleshed them out. I haven't been following the design discussions for lldb-server, and am not in a position to study them now. So I'm not a good person to review these changes. You might try adding Greg as a reviewer, though I don't know how much time he will have right now, since he's starting a new job. Repository: rL LLVM https://reviews.llvm.org/D31450 ___ 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
jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. One grammar comment in the docs, and then this is good. Comment at: www/varformats.html:1071 [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/) grammar: "what will be" -> "which will be" Though I think it would be clearer to say: If a synthetic child provider supplies a special child named $$dereference$$ it will be... https://reviews.llvm.org/D31368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31450: Battery of NetBSD support improvements
krytarowski added a comment. Thanks! Repository: rL LLVM https://reviews.llvm.org/D31450 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers
It is possible to vend one of the actual backing object as a synthetic child using the SB API. What is not possible from the SB API at the moment (we might want to fix it) is to vend one of the actual backing object with a different name then the underlying object itself. You can still say that object X has a child named "foobar" (so X.foobar will work) but the name of the actual child will be something like "_M_baz" when displayed. On Wed, Mar 29, 2017 at 10:16 AM Jim Ingham wrote: > > > On Mar 29, 2017, at 2:06 AM, Tamas Berghammer via Phabricator < > revi...@reviews.llvm.org> wrote: > > > > 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. > > Seems like vending one of the actual backing objects as a synthetic object > is a reasonable thing to do (it's what you are doing internally). But if > we don't allow a way to do that currently, then there's no reason to add > one. > > Jim > > > > > > (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] 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] D31368: Add support for sythetic operator dereference
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Excellent! https://reviews.llvm.org/D31368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers
I see. Might be worth filing an enhancement request to expose Clone so you can do this in a Python synthetic child provider. But there's no reason that lack should block this change. Jim > On Mar 30, 2017, at 12:51 PM, Tamas Berghammer wrote: > > It is possible to vend one of the actual backing object as a synthetic child > using the SB API. What is not possible from the SB API at the moment (we > might want to fix it) is to vend one of the actual backing object with a > different name then the underlying object itself. You can still say that > object X has a child named "foobar" (so X.foobar will work) but the name of > the actual child will be something like "_M_baz" when displayed. > > On Wed, Mar 29, 2017 at 10:16 AM Jim Ingham wrote: > > > On Mar 29, 2017, at 2:06 AM, Tamas Berghammer via Phabricator > > wrote: > > > > 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. > > Seems like vending one of the actual backing objects as a synthetic object is > a reasonable thing to do (it's what you are doing internally). But if we > don't allow a way to do that currently, then there's no reason to add one. > > Jim > > > > > > (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] D31371: Stop calling ValueObject::SetName from synthetic child providers
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Good. 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] 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
Re: [Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers
Created bug for exposing ValueObject::Clone as SB API: http://bugs.llvm.org/show_bug.cgi?id=32477 On Thu, Mar 30, 2017 at 1:04 PM Jim Ingham via Phabricator < revi...@reviews.llvm.org> wrote: > jingham accepted this revision. > jingham added a comment. > This revision is now accepted and ready to land. > > Good. > > > 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] D31450: Battery of NetBSD support improvements
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Thank you. Keep up the good work. Repository: rL LLVM https://reviews.llvm.org/D31450 ___ 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
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. MacOS hasn't shipped with gcc for a while now. If you were serious about using gcc & its STL implementation you would install your own copies of the tools & libraries. So what's on the system isn't really determinative... But this is a minor point, if there's no way to check the library version we can just leave the test off for Darwin. https://reviews.llvm.org/D31366 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D31371: Stop calling ValueObject::SetName from synthetic child providers
Thanks. Jim > On Mar 30, 2017, at 1:16 PM, Tamas Berghammer wrote: > > Created bug for exposing ValueObject::Clone as SB API: > http://bugs.llvm.org/show_bug.cgi?id=32477 > > On Thu, Mar 30, 2017 at 1:04 PM Jim Ingham via Phabricator > wrote: > jingham accepted this revision. > jingham added a comment. > This revision is now accepted and ready to land. > > Good. > > > 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] [lldb] r299109 - Battery of NetBSD support improvements
Author: kamil
Date: Thu Mar 30 15:25:29 2017
New Revision: 299109
URL: http://llvm.org/viewvc/llvm-project?rev=299109&view=rev
Log:
Battery of NetBSD support improvements
Summary:
Include initial support for:
- single step mode (PT_STEP)
- single step trap handling (TRAP_TRACE)
- exec() trap (TRAP_EXEC)
- add placeholder interfaces for FPR
- initial code for NetBSD core(5) files
- minor tweaks
While there improve style of altered elf-core/ files.
This code raises the number of passing tests on NetBSD to around 50%
(600+/1200+).
The introduced code is subject to improve afterwards for additional features
and bug fixes.
Sponsored by
Reviewers: labath, joerg, emaste, kettenis
Reviewed By: labath
Subscribers: srhines, #lldb
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D31450
Modified:
lldb/trunk/source/Host/common/Host.cpp
lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.cpp
lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
lldb/trunk/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
lldb/trunk/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp
Modified: lldb/trunk/source/Host/common/Host.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Host.cpp?rev=299109&r1=299108&r2=299109&view=diff
==
--- lldb/trunk/source/Host/common/Host.cpp (original)
+++ lldb/trunk/source/Host/common/Host.cpp Thu Mar 30 15:25:29 2017
@@ -679,7 +679,7 @@ Error Host::LaunchProcessPosixSpawn(cons
sigemptyset(&no_signals);
sigfillset(&all_signals);
::posix_spawnattr_setsigmask(&attr, &no_signals);
-#if defined(__linux__) || defined(__FreeBSD__)
+#if defined(__linux__) || defined(__FreeBSD__) || defined(__NetBSD__)
::posix_spawnattr_setsigdefault(&attr, &no_signals);
#else
::posix_spawnattr_setsigdefault(&attr, &all_signals);
Modified: lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp?rev=299109&r1=299108&r2=299109&view=diff
==
--- lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
(original)
+++ lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp Thu
Mar 30 15:25:29 2017
@@ -379,12 +379,13 @@ bool DYLDRendezvous::RemoveSOEntries() {
}
bool DYLDRendezvous::SOEntryIsMainExecutable(const SOEntry &entry) {
- // On Linux the executable is indicated by an empty path in the entry. On
- // FreeBSD and on Android it is the full path to the executable.
+ // On some systes the executable is indicated by an empty path in the entry.
+ // On others it is the full path to the executable.
auto triple = m_process->GetTarget().GetArchitecture().GetTriple();
switch (triple.getOS()) {
case llvm::Triple::FreeBSD:
+ case llvm::Triple::NetBSD:
return entry.file_spec == m_exe_file_spec;
case llvm::Triple::Linux:
if (triple.isAndroid())
Modified: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp?rev=299109&r1=299108&r2=299109&view=diff
==
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp Thu Mar 30
15:25:29 2017
@@ -235,6 +235,24 @@ void NativeProcessNetBSD::MonitorSIGTRAP
}
SetState(StateType::eStateStopped, true);
break;
+case TRAP_TRACE:
+ for (const auto &thread_sp : m_threads) {
+
static_pointer_cast(thread_sp)->SetStoppedByTrace();
+ }
+ SetState(StateType::eStateStopped, true);
+ break;
+case TRAP_EXEC: {
+ Error error = ReinitializeThreads();
+ if (error.Fail()) {
+SetState(StateType::eStateInvalid);
+return;
+ }
+
+ // Let our delegate know we have just exec'd.
+ NotifyDidExec();
+
+ SetState(StateType::eStateStopped, true);
+} break;
}
}
}
@@ -389,11 +407,13 @@ Error NativeProcessNetBSD::Resume(const
return Error();
}
+ Error error;
+
switch (action->state) {
case eStateRunning: {
// Run the thread, possibly feeding it the signal.
-
Re: [Lldb-commits] [PATCH] D31367: Expression: add missing linkage to RuntimeDyld component
On czw, 2017-03-30 at 21:37 +0200, Joerg Sonnenberger wrote: > On Thu, Mar 30, 2017 at 06:00:15PM +, Chris Bieneman via Phabricator > wrote: > > ExecutionEngine headers directly reference symbols from RuntimeDyld, > > so we should enforce a requirement that anyone using ExeuctionEngine > > also link RuntimeDyld. This works today as expected for static archive > > builds. It is only broken with `BUILD_SHARED_LIBS`. > > Welcome to the brave new world of newer ELF linkers. Just because a > library pulls in a symbol into the namespace doesn't mean you can just > use that symbol. They want you to explicitly specify the dependency... > This does happen for static archives though. This is expected and desired. Just because some random library you're using is using zlib does not mean you should skip linking zlib when your program is using it too. -- Best regards, Michał Górny signature.asc Description: This is a digitally signed message part ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D31367: Expression: add missing linkage to RuntimeDyld component
I had a talk with Lang about the ExecutionEngine library structuring, and it
sounds like there are some problems there that need to be worked out.
Luckily for this specific case, I think the solution is actually quite simple:
```
diff --git a/include/llvm/ExecutionEngine/ExecutionEngine.h
b/include/llvm/ExecutionEngine/ExecutionEngine.h
index f68337c..cc99f94 100644
--- a/include/llvm/ExecutionEngine/ExecutionEngine.h
+++ b/include/llvm/ExecutionEngine/ExecutionEngine.h
@@ -15,7 +15,6 @@
#ifndef LLVM_EXECUTIONENGINE_EXECUTIONENGINE_H
#define LLVM_EXECUTIONENGINE_EXECUTIONENGINE_H
-#include "RuntimeDyld.h"
#include "llvm-c/ExecutionEngine.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
@@ -49,6 +48,7 @@ class ObjectCache;
class RTDyldMemoryManager;
class Triple;
class Type;
+class JITSymbolResolver;
namespace object {
class Archive;
```
It seems to me that there is no reason why ExecutionEngine.h needs to include
RuntimeDyld.h. a forward declaration of the JITSymbolResolver class will
suffice.
-Chris
> On Mar 30, 2017, at 11:41 AM, Michał Górny via Phabricator
> wrote:
>
> mgorny added a comment.
>
> In https://reviews.llvm.org/D31367#714305, @beanz wrote:
>
>> Please revert your patch. It is *not* the right solution and is masking
>> underlying problems.
>
>
> Reverted in r299095. Now I'd really appreciate some help in figuring out how
> to fix it properly.
>
>> ExecutionEngine headers directly reference symbols from RuntimeDyld, so we
>> should enforce a requirement that anyone using ExeuctionEngine also link
>> RuntimeDyld. This works today as expected for static archive builds. It is
>> only broken with `BUILD_SHARED_LIBS`. I suspect the problem is that when
>> built as a DSO ExecutionEngine has no unresolved symbols against
>> RuntimeDyld, and the linker probably isn't including the reference to
>> RuntimeDyld in the produced ExecutionEngine DSO.
>
> The DSO is linking to RuntimeDyld. However, obviously the `PRIVATE` linkage
> forced in `llvm_add_library` is not the correct solution when the symbols are
> explicitly used in the headers. It should be `PUBLIC` for that particular
> component. Any suggestion on how to fix that API? Should I add an additional
> `PUBLIC_LINK_COMPONENTS` (and `PUBLIC_LINK_LIBS`)?
>
>
> Repository:
> rL LLVM
>
> https://reviews.llvm.org/D31367
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D31367: Expression: add missing linkage to RuntimeDyld component
On czw, 2017-03-30 at 13:55 -0700, Chris Bieneman wrote:
> I had a talk with Lang about the ExecutionEngine library structuring, and it
> sounds like there are some problems there that need to be worked out.
>
> Luckily for this specific case, I think the solution is actually quite simple:
>
> ```
> diff --git a/include/llvm/ExecutionEngine/ExecutionEngine.h
> b/include/llvm/ExecutionEngine/ExecutionEngine.h
> index f68337c..cc99f94 100644
> --- a/include/llvm/ExecutionEngine/ExecutionEngine.h
> +++ b/include/llvm/ExecutionEngine/ExecutionEngine.h
> @@ -15,7 +15,6 @@
> #ifndef LLVM_EXECUTIONENGINE_EXECUTIONENGINE_H
> #define LLVM_EXECUTIONENGINE_EXECUTIONENGINE_H
>
> -#include "RuntimeDyld.h"
> #include "llvm-c/ExecutionEngine.h"
> #include "llvm/ADT/SmallVector.h"
> #include "llvm/ADT/StringRef.h"
> @@ -49,6 +48,7 @@ class ObjectCache;
> class RTDyldMemoryManager;
> class Triple;
> class Type;
> +class JITSymbolResolver;
>
> namespace object {
>class Archive;
> ```
>
> It seems to me that there is no reason why ExecutionEngine.h needs to include
> RuntimeDyld.h. a forward declaration of the JITSymbolResolver class will
> suffice.
>
Thanks, I'll test this patch and get back to you.
--
Best regards,
Michał Górny
signature.asc
Description: This is a digitally signed message part
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31485: Verify memory address range validity in GDBRemoteCommunicationClient
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Simple logic change so we don't check the range validity more than once.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1506-1510
+ // We got an invalid address range back
+ if (!region_info.GetRange().IsValid()) {
+error.SetErrorString("Server returned invalid range");
+ }
+
Logic is fine, but I would work this into the if statement below. Something
like:
```
if (region_info.GetRange().IsValid()) {
if (!saw_permissions) {
region_info.SetReadable(MemoryRegionInfo::eNo);
region_info.SetWritable(MemoryRegionInfo::eNo);
region_info.SetExecutable(MemoryRegionInfo::eNo);
region_info.SetMapped(MemoryRegionInfo::eNo);
}
} else {
// We got an invalid address range back
error.SetErrorString("Server returned invalid range");
}
```
https://reviews.llvm.org/D31485
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r299116 - add NetBSD files to Xcode project to resolve failure from r299109
Author: penryu
Date: Thu Mar 30 16:27:51 2017
New Revision: 299116
URL: http://llvm.org/viewvc/llvm-project?rev=299116&view=rev
Log:
add NetBSD files to Xcode project to resolve failure from r299109
Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=299116&r1=299115&r2=299116&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Thu Mar 30 16:27:51 2017
@@ -880,6 +880,7 @@
9AC70390117675270086C050 /* SBInstructionList.h in Headers */ =
{isa = PBXBuildFile; fileRef = 9AC7038F117675270086C050 /* SBInstructionList.h
*/; settings = {ATTRIBUTES = (Public, ); }; };
9AC703AF117675410086C050 /* SBInstruction.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 9AC703AE117675410086C050 /* SBInstruction.cpp
*/; };
9AC703B1117675490086C050 /* SBInstructionList.cpp in Sources */
= {isa = PBXBuildFile; fileRef = 9AC703B0117675490086C050 /*
SBInstructionList.cpp */; };
+ 9AD9449F1E8DB26C004796ED /* RegisterContextNetBSD_x86_64.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 9AD9449B1E8DB267004796ED /*
RegisterContextNetBSD_x86_64.cpp */; };
A36FF33C17D8E94600244D40 /* OptionParser.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = A36FF33B17D8E94600244D40 /* OptionParser.cpp */;
};
AE44FB301BB07EB20033EB62 /* GoUserExpression.cpp in Sources */
= {isa = PBXBuildFile; fileRef = AE44FB2C1BB07DD80033EB62 /*
GoUserExpression.cpp */; };
AE44FB311BB07EB80033EB62 /* GoLexer.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = AE44FB2A1BB07DD80033EB62 /* GoLexer.cpp */; };
@@ -2851,6 +2852,8 @@
9AC7038F117675270086C050 /* SBInstructionList.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
SBInstructionList.h; path = include/lldb/API/SBInstructionList.h; sourceTree =
""; };
9AC703AE117675410086C050 /* SBInstruction.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
name = SBInstruction.cpp; path = source/API/SBInstruction.cpp; sourceTree =
""; };
9AC703B0117675490086C050 /* SBInstructionList.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
name = SBInstructionList.cpp; path = source/API/SBInstructionList.cpp;
sourceTree = ""; };
+ 9AD9449B1E8DB267004796ED /* RegisterContextNetBSD_x86_64.cpp */
= {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType =
sourcecode.cpp.cpp; name = RegisterContextNetBSD_x86_64.cpp; path =
Utility/RegisterContextNetBSD_x86_64.cpp; sourceTree = ""; };
+ 9AD9449C1E8DB267004796ED /* RegisterContextNetBSD_x86_64.h */ =
{isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h;
name = RegisterContextNetBSD_x86_64.h; path =
Utility/RegisterContextNetBSD_x86_64.h; sourceTree = ""; };
9AF16A9C11402D5B007A7B3F /* SBBreakpoint.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
name = SBBreakpoint.cpp; path = source/API/SBBreakpoint.cpp; sourceTree =
""; };
9AF16A9E11402D69007A7B3F /* SBBreakpoint.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
SBBreakpoint.h; path = include/lldb/API/SBBreakpoint.h; sourceTree = "";
};
9AF16CC611408686007A7B3F /* SBBreakpointLocation.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
SBBreakpointLocation.h; path = include/lldb/API/SBBreakpointLocation.h;
sourceTree = ""; };
@@ -4517,6 +4520,8 @@
26B4666E11A2080F00CF6220 /* Utility */ = {
isa = PBXGroup;
children = (
+ 9AD9449B1E8DB267004796ED /*
RegisterContextNetBSD_x86_64.cpp */,
+ 9AD9449C1E8DB267004796ED /*
RegisterContextNetBSD_x86_64.h */,
23F403471926C8D50046DC9B /*
NativeRegisterContextRegisterInfo.h */,
23F403481926CC250046DC9B /*
NativeRegisterContextRegisterInfo.cpp */,
B287E63E12EFAE2C00C9BEFE /* ARMDefines.h */,
@@ -7354,6 +7359,7 @@
268900D013353E6F00698AC0 /* Block.cpp in
Sources */,
268900D113353E6F00698AC0 /* ClangASTContext.cpp
in Sources */,
265192C61BA8E905002F08F6 /* CompilerDecl.cpp in
Sources */,
+ 9AD9449F1E8DB26C004796ED /*
RegisterContextNetBSD_x86_64.cpp in Sources */,
268900D213353E6F00698AC0 /* Compil
Re: [Lldb-commits] [PATCH] D31367: Expression: add missing linkage to RuntimeDyld component
On Thu, Mar 30, 2017 at 10:47:39PM +0200, Michał Górny wrote: > On czw, 2017-03-30 at 21:37 +0200, Joerg Sonnenberger wrote: > > On Thu, Mar 30, 2017 at 06:00:15PM +, Chris Bieneman via Phabricator > > wrote: > > > ExecutionEngine headers directly reference symbols from RuntimeDyld, > > > so we should enforce a requirement that anyone using ExeuctionEngine > > > also link RuntimeDyld. This works today as expected for static archive > > > builds. It is only broken with `BUILD_SHARED_LIBS`. > > > > Welcome to the brave new world of newer ELF linkers. Just because a > > library pulls in a symbol into the namespace doesn't mean you can just > > use that symbol. They want you to explicitly specify the dependency... > > This does happen for static archives though. > > This is expected and desired. Just because some random library you're > using is using zlib does not mean you should skip linking zlib when your > program is using it too. That justification never made that much sense. The linker simply does not know whether the dependency is considered part of the ABI contract of the library or not. Hint: the symbol lookup rules for ELF is recursive. Having different rules for link-time vs run-time is a very good sign that something is very fishy. But let's not get distracted. The behavior change of GNU linkers (and anything following them) is exactly why the original patch was correct here. Symbols from RuntimeDyld are directly used -- as such the library should be an explicit declared dependency. If no such reference is created, i.e. due to not using any of the header functions references such symbols, no such dependency is necessary. ExecutionEngine can't tell in advance. Joerg ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31485: Verify memory address range validity in GDBRemoteCommunicationClient
xiaobai updated this revision to Diff 93566.
xiaobai added a comment.
Added unit tests for the method and changed the logic according to clayborg's
suggestion.
https://reviews.llvm.org/D31485
Files:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
Index: unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -13,6 +13,7 @@
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/StructuredData.h"
+#include "lldb/Target/MemoryRegionInfo.h"
#include "lldb/Utility/DataBuffer.h"
#include "llvm/ADT/ArrayRef.h"
@@ -331,3 +332,45 @@
HandlePacket(server, "QPassSignals:", "OK");
EXPECT_TRUE(result.get().Success());
}
+
+TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfo) {
+ TestClient client;
+ MockServer server;
+ Connect(client, server);
+ if (HasFailure())
+return;
+
+ const lldb::addr_t addr = 0xa000;
+ MemoryRegionInfo region_info;
+ std::future result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+ });
+
+ // name is: /foo/bar.so
+ HandlePacket(server,
+ "qMemoryRegionInfo:a000",
+ "start:a000;size:2000;permissions:rx;name:2f666f6f2f6261722e736f;");
+ EXPECT_TRUE(result.get().Success());
+
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfoInvalidResponse) {
+ TestClient client;
+ MockServer server;
+ Connect(client, server);
+ if (HasFailure())
+return;
+
+ const lldb::addr_t addr = 0x4000;
+ MemoryRegionInfo region_info;
+ std::future result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+ });
+
+ HandlePacket(server, "qMemoryRegionInfo:4000", "start:4000;size:;");
+ EXPECT_FALSE(result.get().Success());
+
+ result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+ });
+}
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1503,13 +1503,18 @@
}
}
- // We got a valid address range back but no permissions -- which means
- // this is an unmapped page
- if (region_info.GetRange().IsValid() && saw_permissions == false) {
-region_info.SetReadable(MemoryRegionInfo::eNo);
-region_info.SetWritable(MemoryRegionInfo::eNo);
-region_info.SetExecutable(MemoryRegionInfo::eNo);
-region_info.SetMapped(MemoryRegionInfo::eNo);
+ if (region_info.GetRange().IsValid()) {
+// We got a valid address range back but no permissions -- which means
+// this is an unmapped page
+if (!saw_permissions) {
+ region_info.SetReadable(MemoryRegionInfo::eNo);
+ region_info.SetWritable(MemoryRegionInfo::eNo);
+ region_info.SetExecutable(MemoryRegionInfo::eNo);
+ region_info.SetMapped(MemoryRegionInfo::eNo);
+}
+ } else {
+// We got an invalid address range back
+error.SetErrorString("Server returned invalid range");
}
} else {
m_supports_memory_region_info = eLazyBoolNo;
Index: unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -13,6 +13,7 @@
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/StructuredData.h"
+#include "lldb/Target/MemoryRegionInfo.h"
#include "lldb/Utility/DataBuffer.h"
#include "llvm/ADT/ArrayRef.h"
@@ -331,3 +332,45 @@
HandlePacket(server, "QPassSignals:", "OK");
EXPECT_TRUE(result.get().Success());
}
+
+TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfo) {
+ TestClient client;
+ MockServer server;
+ Connect(client, server);
+ if (HasFailure())
+return;
+
+ const lldb::addr_t addr = 0xa000;
+ MemoryRegionInfo region_info;
+ std::future result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+ });
+
+ // name is: /foo/bar.so
+ HandlePacket(server,
+ "qMemoryRegionInfo:a000",
+ "start:a000;size:2000;permissions:rx;name:2f666f6f2f6261722e736f;");
+ EXPECT_TRUE(result.get().Success());
+
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfoInvalidResponse) {
+ TestClient client;
+ MockServer server;
+ Connect
[Lldb-commits] [PATCH] D31485: Verify memory address range validity in GDBRemoteCommunicationClient
xiaobai marked an inline comment as done.
xiaobai added inline comments.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1506-1510
+ // We got an invalid address range back
+ if (!region_info.GetRange().IsValid()) {
+error.SetErrorString("Server returned invalid range");
+ }
+
clayborg wrote:
> Logic is fine, but I would work this into the if statement below. Something
> like:
>
> ```
> if (region_info.GetRange().IsValid()) {
> if (!saw_permissions) {
> region_info.SetReadable(MemoryRegionInfo::eNo);
> region_info.SetWritable(MemoryRegionInfo::eNo);
> region_info.SetExecutable(MemoryRegionInfo::eNo);
> region_info.SetMapped(MemoryRegionInfo::eNo);
> }
> } else {
> // We got an invalid address range back
> error.SetErrorString("Server returned invalid range");
> }
> ```
Thanks for pointing that out!
https://reviews.llvm.org/D31485
___
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
eugene updated this revision to Diff 93572.
eugene added a comment.
Addressing code-review comments.
Most notable change: MethodName::Parse() tries simple version of name parser,
before invoking full power of CPlusPlusNameParser. It really helps with the
perf.
https://reviews.llvm.org/D31451
Files:
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -6,35 +6,139 @@
// License. See LICENSE.TXT for details.
//
//===--===//
-
#include "gtest/gtest.h"
#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
using namespace lldb_private;
-TEST(CPlusPlusLanguage, MethodName) {
+TEST(CPlusPlusLanguage, MethodNameParsing) {
struct TestCase {
std::string input;
std::string context, basename, arguments, qualifiers, scope_qualified_name;
};
TestCase test_cases[] = {
- {"foo::bar(baz)", "foo", "bar", "(baz)", "", "foo::bar"},
+ {"main(int, char *[]) ", "", "main", "(int, char *[])", "", "main"},
+ {"foo::bar(baz) const", "foo", "bar", "(baz)", "const", "foo::bar"},
+ {"foo::~bar(baz)", "foo", "~bar", "(baz)", "", "foo::~bar"},
+ {"a::b::c::d(e,f)", "a::b::c", "d", "(e,f)", "", "a::b::c::d"},
+ {"void f(int)", "", "f", "(int)", "", "f"},
+
+ // Operators
{"std::basic_ostream >& "
"std::operator<< >"
"(std::basic_ostream >&, char const*)",
"std", "operator<< >",
"(std::basic_ostream >&, char const*)", "",
- "std::operator<< >"}};
+ "std::operator<< >"},
+ {"operator delete[](void*, clang::ASTContext const&, unsigned long)", "",
+ "operator delete[]", "(void*, clang::ASTContext const&, unsigned long)",
+ "", "operator delete[]"},
+ {"llvm::Optional::operator bool() const",
+ "llvm::Optional", "operator bool", "()", "const",
+ "llvm::Optional::operator bool"},
+ {"(anonymous namespace)::FactManager::operator[](unsigned short)",
+ "(anonymous namespace)::FactManager", "operator[]", "(unsigned short)",
+ "", "(anonymous namespace)::FactManager::operator[]"},
+ {"const int& std::map>::operator[](short) const",
+ "std::map>", "operator[]", "(short)", "const",
+ "std::map>::operator[]"},
+ {"CompareInsn::operator()(llvm::StringRef, InsnMatchEntry const&)",
+ "CompareInsn", "operator()", "(llvm::StringRef, InsnMatchEntry const&)",
+ "", "CompareInsn::operator()"},
+ {"llvm::Optional::operator*() const &",
+ "llvm::Optional", "operator*", "()", "const &",
+ "llvm::Optional::operator*"},
+ // Internal classes
+ {"operator<<(Cls, Cls)::Subclass::function()",
+ "operator<<(Cls, Cls)::Subclass", "function", "()", "",
+ "operator<<(Cls, Cls)::Subclass::function"},
+ {"SAEC::checkFunction(context&) const::CallBack::CallBack(int)",
+ "SAEC::checkFunction(context&) const::CallBack", "CallBack", "(int)", "",
+ "SAEC::checkFunction(context&) const::CallBack::CallBack"},
+ // Anonymous namespace
+ {"XX::(anonymous namespace)::anon_class::anon_func() const",
+ "XX::(anonymous namespace)::anon_class", "anon_func", "()", "const",
+ "XX::(anonymous namespace)::anon_class::anon_func"},
+
+ // Function pointers
+ {"string (*f(vector&&))(float)", "", "f", "(vector&&)", "",
+ "f"},
+ {"void (*&std::_Any_data::_M_access())()", "std::_Any_data",
+ "_M_access", "()", "",
+ "std::_Any_data::_M_access"},
+ {"void (*(*(*(*(*(*(*(* const&func1(int))())())())())())())())()", "",
+ "func1", "(int)", "", "func1"},
+
+ // Templates
+ {"void llvm::PM>::"
+ "addPass(llvm::VP)",
+ "llvm::PM>", "addPass",
+ "(llvm::VP)", "",
+ "llvm::PM>::"
+ "addPass"},
+ {"void std::vector >"
+ "::_M_emplace_back_aux(Class const&)",
+ "std::vector >",
+ "_M_emplace_back_aux", "(Class const&)", "",
+ "std::vector >::"
+ "_M_emplace_back_aux"},
+ {"unsigned long llvm::countTrailingOnes"
+ "(unsigned int, llvm::ZeroBehavior)",
+ "llvm", "countTrailingOnes",
+ "(unsigned int, llvm::ZeroBehavior)", "",
+ "llvm::countTrailingOnes"},
+ {"std::enable_if<(10u)<(64), bool>::type llvm::isUInt<10u>(unsigned "
+ "long)",
+ "llvm", "isUInt<10u>", "(unsigned long)", "", "llvm::isUInt<10u>"},
+ {"f, sizeof(B)()", "",
+ "f, sizeof(B)", "()"
[Lldb-commits] [PATCH] D31450: Battery of NetBSD support improvements
krytarowski added a comment. Thanks! I noted that I introduced some bugs.. but I will fix them in future revisions. I will move on to threads now. FPR/watchpoints will be done later, on the cost on adding some code for cores and helping out with base system work. LLDB/NetBSD is now out of the box functional as a debugger for a single thread. Repository: rL LLVM https://reviews.llvm.org/D31450 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31485: Verify memory address range validity in GDBRemoteCommunicationClient
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D31485 ___ 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
eugene marked 4 inline comments as done.
eugene added a comment.
> I think we should do some performance measurements to see whether this needs
> more optimising or it's fine as is.
>
> I propose the following benchmark:
>
> bin/lldb bin/clang
>
> make sure clang is statically linked
> breakpoint set --name non_exisiting_function
>
> Setting the breakpoint on a nonexistent function avoids us timing the
> breakpoint setting machinery, while still getting every symbol in the
> executable parsed.
I did some micro-benchmarking and on average new parser is ~3 time slower than
the old one. (new parser - ~200k string/s, old parser - ~700k string/s)
clang::Lexer appears to be the slowest part of it.
On the clang breakpoint benchmark you proposed, it is hard to notice much of a
difference. clang binary has about 500k functions.
With new parser it takes about 11s to try to set a breakpoint, on the old one
it's about 10s. (release version of LLDB, debug static version of clang)
I decided to use a hybrid approach, when we use old parsing code for simple
cases and call new parser only when it fails.
80% of clang functions are simple enough that we don't really need the new
parser, so it helped to bring clang breakpoint test back to 10s.
I think it's reasonable to assume that similar distribution is true for most
programs, and most of their functions can be parsed with the old code.
I don't think we can really improve performance of a new parser without giving
up on clang::Lexer, and I'm reluctant to do it.
So I propose to keep hybrid approach and call a new parser only for complicated
cases.
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:94
if (!m_parsed && m_full) {
-//ConstString mangled;
-//m_full.GetMangledCounterpart(mangled);
-//printf ("\n parsing = '%s'\n", m_full.GetCString());
-//if (mangled)
-//printf (" mangled = '%s'\n", mangled.GetCString());
-m_parse_error = false;
-m_parsed = true;
-llvm::StringRef full(m_full.GetCString());
-
-size_t arg_start, arg_end;
-llvm::StringRef parens("()", 2);
-if (ReverseFindMatchingChars(full, parens, arg_start, arg_end)) {
- m_arguments = full.substr(arg_start, arg_end - arg_start + 1);
- if (arg_end + 1 < full.size())
-m_qualifiers = full.substr(arg_end + 1);
- if (arg_start > 0) {
-size_t basename_end = arg_start;
-size_t context_start = 0;
-size_t context_end = llvm::StringRef::npos;
-if (basename_end > 0 && full[basename_end - 1] == '>') {
- // TODO: handle template junk...
- // Templated function
- size_t template_start, template_end;
- llvm::StringRef lt_gt("<>", 2);
- if (ReverseFindMatchingChars(full, lt_gt, template_start,
- template_end, basename_end)) {
-// Check for templated functions that include return type like:
-// 'void foo()'
-context_start = full.rfind(' ', template_start);
-if (context_start == llvm::StringRef::npos)
- context_start = 0;
-else
- ++context_start;
-
-context_end = full.rfind(':', template_start);
-if (context_end == llvm::StringRef::npos ||
-context_end < context_start)
- context_end = context_start;
- } else {
-context_end = full.rfind(':', basename_end);
- }
-} else if (context_end == llvm::StringRef::npos) {
- context_end = full.rfind(':', basename_end);
-}
-
-if (context_end == llvm::StringRef::npos)
- m_basename = full.substr(0, basename_end);
-else {
- if (context_start < context_end)
-m_context =
-full.substr(context_start, context_end - 1 - context_start);
- const size_t basename_begin = context_end + 1;
- m_basename =
- full.substr(basename_begin, basename_end - basename_begin);
-}
-m_type = eTypeUnknownMethod;
- } else {
-m_parse_error = true;
-return;
- }
-
- if (!IsValidBasename(m_basename)) {
-// The C++ basename doesn't match our regular expressions so this can't
-// be a valid C++ method, clear everything out and indicate an error
-m_context = llvm::StringRef();
-m_basename = llvm::StringRef();
-m_arguments = llvm::StringRef();
-m_qualifiers = llvm::StringRef();
-m_parse_error = true;
- }
+CPlusPlusNameParser parser(m_full.GetStringRef());
+auto function = parser.ParseAsFunctionDefinition();
labath wrote:
> How about the following api:
> ```
> if (auto function =
> CPlusPlusNameParser::ParseAsFunctionDefinition(m_full.GetStringRef())) {
> ...
> ```
If you don't mind I'll leave it as it is.
I understand that i
[Lldb-commits] [lldb] r299147 - Don't add a newline if the object description already has one.
Author: jingham
Date: Thu Mar 30 20:32:57 2017
New Revision: 299147
URL: http://llvm.org/viewvc/llvm-project?rev=299147&view=rev
Log:
Don't add a newline if the object description already has one.
Modified:
lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp
Modified: lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp?rev=299147&r1=299146&r2=299147&view=diff
==
--- lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp (original)
+++ lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp Thu Mar 30 20:32:57
2017
@@ -457,7 +457,12 @@ bool ValueObjectPrinter::PrintObjectDesc
else
object_desc = GetDescriptionForDisplay();
if (object_desc && *object_desc) {
-m_stream->Printf("%s\n", object_desc);
+// If the description already ends with a \n don't add another one.
+size_t object_end = strlen(object_desc) - 1;
+if (object_desc[object_end] == '\n')
+m_stream->Printf("%s", object_desc);
+else
+m_stream->Printf("%s\n", object_desc);
return true;
} else if (value_printed == false && summary_printed == false)
return true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
