[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
labath added a comment. Which version is this patch based on? The line numbers don't seem to match what I see on master. Could you rebase the patch to master, and upload the patch with a full diff (e.g. via `git diff -U`, see https://llvm.org/docs/Phabricator.html#id3). Comment at: include/lldb/Core/ModuleList.h:545 +bool always_create = false, +const char* sysroot = nullptr); static bool RemoveSharedModule(lldb::ModuleSP &module_sp); Please make this an `llvm::StringRef` (and then change `AsCString` to `GetStringRef` below). Repository: rL LLVM https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
labath added a comment.
The patch makes sense to me. It's nice to get a performance boost, while
reducing the number of demanglers at the same time.
Comment at: source/Core/Mangled.cpp:310
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
erik.pilkington wrote:
> sgraenitz wrote:
> > erik.pilkington wrote:
> > > sgraenitz wrote:
> > > > sgraenitz wrote:
> > > > > sgraenitz wrote:
> > > > > > erik.pilkington wrote:
> > > > > > > I think this is going to really tank performance:
> > > > > > > ItaniumPartialDemangler dynamically allocates a pretty big buffer
> > > > > > > on construction that it uses to store the AST (and reuse for
> > > > > > > subsequent calls to partialDemangle). Is there somewhere that you
> > > > > > > can put IPD it so that it stays alive between demangles?
> > > > > > >
> > > > > > > An alternative, if its more convenient, would be to just put the
> > > > > > > buffer inline into ItaniumPartialDemangler, and `= delete` the
> > > > > > > move operations.
> > > > > > Thanks for the remark, I didn't dig deep enough to see what's going
> > > > > > on in the `BumpPointerAllocator` class. I guess there is a reason
> > > > > > for having `ASTAllocator` in the `Db` struct as an instance and
> > > > > > thus allocating upfront, instead of having a pointer there and
> > > > > > postponing the instantiation to `Db::reset()`?
> > > > > >
> > > > > > I am not familiar enough with the code yet to know how it will look
> > > > > > exactly, but having a heavy demangler in every `Mangled` per se
> > > > > > sounds unreasonable. There's just too many of them that don't
> > > > > > require demangling at all. For each successfully initiated
> > > > > > `partialDemangle()` I will need to keep one of course.
> > > > > >
> > > > > > I will have a closer look on Monday. So far thanks for mentioning
> > > > > > that!
> > > > > Well, right the pointer to `BumpPointerAllocator` won't solve
> > > > > anything. Ok will have a look.
> > > > > ItaniumPartialDemangler dynamically allocates a pretty big buffer on
> > > > > construction
> > > >
> > > > I think in the end each `Mangled` instance will have a pointer to IPD
> > > > field for lazy access to rich demangling info. This piece of code may
> > > > become something like:
> > > > ```
> > > > m_IPD = new ItaniumPartialDemangler();
> > > > if (bool err = m_IPD->partialDemangle(mangled_name)) {
> > > > delete m_IPD; m_IPD = nullptr;
> > > > }
> > > > ```
> > > >
> > > > In order to avoid unnecessary instantiations, we can consider to filter
> > > > symbols upfront that we know can't be demangled. E.g. we could
> > > > duplicate the simple checks from `Db::parse()` before creating a
> > > > `ItaniumPartialDemangler` instance.
> > > >
> > > > Even the simple switch with the above code in place shows performance
> > > > improvements. So for now I would like to leave it this way and review
> > > > the issue after having the bigger patch, which will actually make use
> > > > of the rich demangle info.
> > > >
> > > > What do you think?
> > > Sure, if this is a performance win then I can't think of any reason not
> > > to do it.
> > >
> > > In the future though, I don't think that having copies of IPD in each
> > > Mangled is a good idea, even if they are lazily initialized. The
> > > partially demangled state held in IPD is significantly larger than the
> > > demangled string, so I think it would lead to a lot more memory usage. Do
> > > you think it is possible to have just one instance of IPD that you could
> > > use to demangle all the symbols to their chopped-up state, and just store
> > > that instead?
> > Yes if that will be a bit more work, but also a possibility. I did a small
> > experiment making the IPD in line 288 `static`, to reuse a single instance.
> > That didn't affect runtimes much. I tried it several times and got the same
> > results again, but have no explanation.
> >
> > You would expect a speedup from this right? Is there maybe cleanup work
> > that eats up time when reusing an instance? Maybe I have to revisit that.
> Weird, I would have expected a speedup for making it static. My only guess is
> that `malloc` is just quickly handing back the buffer it used for the last
> IPD or something. I think the cleanup work IPD does should be about the same
> as the cost of construction.
If reusing the same IPD object can bring significant benefit, I think the right
approach would be to change/extend/add the API (not in this patch) to make it
possible to use it naturally. There aren't many places that do batch demangling
of a large amount of symbols (`Symtab::InitNameIndexes` is one I recall), so it
shouldn't be too hard to modify them to do something smarter.
https://reviews.llvm.org/D49612
___
lldb-commits mailing list
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
JDevlieghere added inline comments. Comment at: source/Core/Mangled.cpp:30 #include "llvm/ADT/StringRef.h"// for StringRef +#include "llvm/Demangle/Demangle.h" While you're here I'd remove these redundant comments so this block looks more consistent. Comment at: source/Core/Mangled.cpp:292 + // Default buffer and size (realloc is used in case it's too small). + size_t default_size = 80; + demangled_name = static_cast(::malloc(default_size)); Nit: I'd call this `demangled_size` because finishDemangle will update the value. https://reviews.llvm.org/D49612 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337819 - Reimplement EventDataBytes::Dump to avoid DumpDataExtractor
Author: labath
Date: Tue Jul 24 03:49:14 2018
New Revision: 337819
URL: http://llvm.org/viewvc/llvm-project?rev=337819&view=rev
Log:
Reimplement EventDataBytes::Dump to avoid DumpDataExtractor
This is the only external non-trivial dependency of the Event classes.
Added:
lldb/trunk/unittests/Core/EventTest.cpp
Modified:
lldb/trunk/source/Core/Event.cpp
lldb/trunk/unittests/Core/CMakeLists.txt
Modified: lldb/trunk/source/Core/Event.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Event.cpp?rev=337819&r1=337818&r2=337819&view=diff
==
--- lldb/trunk/source/Core/Event.cpp (original)
+++ lldb/trunk/source/Core/Event.cpp Tue Jul 24 03:49:14 2018
@@ -10,7 +10,6 @@
#include "lldb/Core/Event.h"
#include "lldb/Core/Broadcaster.h"
-#include "lldb/Core/DumpDataExtractor.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/Endian.h"
#include "lldb/Utility/Stream.h"
@@ -134,14 +133,13 @@ const ConstString &EventDataBytes::GetFl
void EventDataBytes::Dump(Stream *s) const {
size_t num_printable_chars =
std::count_if(m_bytes.begin(), m_bytes.end(), isprint);
- if (num_printable_chars == m_bytes.size()) {
-s->Printf("\"%s\"", m_bytes.c_str());
- } else if (!m_bytes.empty()) {
-DataExtractor data;
-data.SetData(m_bytes.data(), m_bytes.size(), endian::InlHostByteOrder());
-DumpDataExtractor(data, s, 0, eFormatBytes, 1, m_bytes.size(), 32,
- LLDB_INVALID_ADDRESS, 0, 0);
- }
+ if (num_printable_chars == m_bytes.size())
+s->Format("\"{0}\"", m_bytes);
+ else
+s->Format("{0:$[ ]@[x-2]}", llvm::make_range(
+ reinterpret_cast(m_bytes.data()),
+ reinterpret_cast(m_bytes.data() +
+ m_bytes.size(;
}
const void *EventDataBytes::GetBytes() const {
Modified: lldb/trunk/unittests/Core/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Core/CMakeLists.txt?rev=337819&r1=337818&r2=337819&view=diff
==
--- lldb/trunk/unittests/Core/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Core/CMakeLists.txt Tue Jul 24 03:49:14 2018
@@ -1,6 +1,7 @@
add_lldb_unittest(LLDBCoreTests
BroadcasterTest.cpp
DataExtractorTest.cpp
+ EventTest.cpp
ListenerTest.cpp
ScalarTest.cpp
StateTest.cpp
Added: lldb/trunk/unittests/Core/EventTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Core/EventTest.cpp?rev=337819&view=auto
==
--- lldb/trunk/unittests/Core/EventTest.cpp (added)
+++ lldb/trunk/unittests/Core/EventTest.cpp Tue Jul 24 03:49:14 2018
@@ -0,0 +1,25 @@
+//===-- EventTest.cpp ---*- C++
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Core/Event.h"
+#include "lldb/Utility/StreamString.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+static std::string to_string(const EventDataBytes &E) {
+ StreamString S;
+ E.Dump(&S);
+ return S.GetString();
+}
+
+TEST(EventTest, DumpEventDataBytes) {
+ EXPECT_EQ(R"("foo")", to_string(EventDataBytes("foo")));
+ EXPECT_EQ("01 02 03", to_string(EventDataBytes("\x01\x02\x03")));
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
sgraenitz updated this revision to Diff 157001.
sgraenitz added a comment.
Minor improvements on naming and comments
https://reviews.llvm.org/D49612
Files:
cmake/modules/LLDBConfig.cmake
lldb.xcodeproj/project.pbxproj
source/Core/Mangled.cpp
unittests/Core/CMakeLists.txt
unittests/Core/MangledTest.cpp
Index: unittests/Core/MangledTest.cpp
===
--- /dev/null
+++ unittests/Core/MangledTest.cpp
@@ -0,0 +1,38 @@
+//===-- MangledTest.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/Core/Mangled.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(MangledTest, ResultForValidName) {
+ ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+ bool IsMangled = true;
+
+ Mangled TheMangled(MangledName, IsMangled);
+ const ConstString &TheDemangled =
+ TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+ ConstString ExpectedResult("void a::b::c(unsigned long)");
+ EXPECT_EQ(ExpectedResult, TheDemangled);
+}
+
+TEST(MangledTest, EmptyForInvalidName) {
+ ConstString MangledName("_ZN1a1b1cmxktpEEvm");
+ bool IsMangled = true;
+
+ Mangled TheMangled(MangledName, IsMangled);
+ const ConstString &TheDemangled =
+ TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+ EXPECT_EQ(ConstString(""), TheDemangled);
+}
Index: unittests/Core/CMakeLists.txt
===
--- unittests/Core/CMakeLists.txt
+++ unittests/Core/CMakeLists.txt
@@ -1,6 +1,7 @@
add_lldb_unittest(LLDBCoreTests
BroadcasterTest.cpp
DataExtractorTest.cpp
+ MangledTest.cpp
ListenerTest.cpp
ScalarTest.cpp
StateTest.cpp
Index: source/Core/Mangled.cpp
===
--- source/Core/Mangled.cpp
+++ source/Core/Mangled.cpp
@@ -16,34 +16,24 @@
#pragma comment(lib, "dbghelp.lib")
#endif
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-// Provide a fast-path demangler implemented in FastDemangle.cpp until it can
-// replace the existing C++ demangler with a complete implementation
-#include "lldb/Utility/FastDemangle.h"
-#include "llvm/Demangle/Demangle.h"
-#else
-// FreeBSD9-STABLE requires this to know about size_t in cxxabi.
-#include
-#include
-#endif
-
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Logging.h"
#include "lldb/Utility/RegularExpression.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/Timer.h"
-#include "lldb/lldb-enumerations.h" // for LanguageType
+#include "lldb/lldb-enumerations.h"
#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
#include "Plugins/Language/ObjC/ObjCLanguage.h"
-#include "llvm/ADT/StringRef.h"// for StringRef
-#include "llvm/Support/Compiler.h" // for LLVM_PRETT...
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Demangle/Demangle.h"
+#include "llvm/Support/Compiler.h"
-#include// for mutex, loc...
-#include // for string
-#include // for pair
+#include
+#include
+#include
#include
#include
@@ -295,18 +285,15 @@
break;
}
case eManglingSchemeItanium: {
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-if (log)
- log->Printf("demangle itanium: %s", mangled_name);
-// Try to use the fast-path demangler first for the performance win,
-// falling back to the full demangler only when necessary
-demangled_name = FastDemangle(mangled_name, m_mangled.GetLength());
-if (!demangled_name)
- demangled_name =
- llvm::itaniumDemangle(mangled_name, NULL, NULL, NULL);
-#else
-demangled_name = abi::__cxa_demangle(mangled_name, NULL, NULL, NULL);
-#endif
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
+if (!demangle_err) {
+ // Default buffer and size (realloc is used in case it's too small).
+ size_t demangled_size = 80;
+ demangled_name = static_cast(::malloc(demangled_size));
+ demangled_name = IPD.finishDemangle(demangled_name, &demangled_size);
+}
+
if (log) {
if (demangled_name)
log->Printf("demangled itanium: %s -> \"%s\"", mangled_name,
Index: lldb.xcodeproj/project.pbxproj
===
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/project.pbxproj
@@ -483,6 +483,7 @@
9A20573A1F3B8E7E00F6C293 /* MainLoopTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A2057301F3B8E7600F6C293 /* MainLoopTest.cpp */; };
8C3BD9961EF45DA50016C343 /* MainThreadCheckerRuntime.cpp in Sources */ = {isa = PBXBuildFile; fileRef
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
sgraenitz marked 2 inline comments as done.
sgraenitz added inline comments.
Comment at: source/Core/Mangled.cpp:310
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
labath wrote:
> erik.pilkington wrote:
> > sgraenitz wrote:
> > > erik.pilkington wrote:
> > > > sgraenitz wrote:
> > > > > sgraenitz wrote:
> > > > > > sgraenitz wrote:
> > > > > > > erik.pilkington wrote:
> > > > > > > > I think this is going to really tank performance:
> > > > > > > > ItaniumPartialDemangler dynamically allocates a pretty big
> > > > > > > > buffer on construction that it uses to store the AST (and reuse
> > > > > > > > for subsequent calls to partialDemangle). Is there somewhere
> > > > > > > > that you can put IPD it so that it stays alive between
> > > > > > > > demangles?
> > > > > > > >
> > > > > > > > An alternative, if its more convenient, would be to just put
> > > > > > > > the buffer inline into ItaniumPartialDemangler, and `= delete`
> > > > > > > > the move operations.
> > > > > > > Thanks for the remark, I didn't dig deep enough to see what's
> > > > > > > going on in the `BumpPointerAllocator` class. I guess there is a
> > > > > > > reason for having `ASTAllocator` in the `Db` struct as an
> > > > > > > instance and thus allocating upfront, instead of having a pointer
> > > > > > > there and postponing the instantiation to `Db::reset()`?
> > > > > > >
> > > > > > > I am not familiar enough with the code yet to know how it will
> > > > > > > look exactly, but having a heavy demangler in every `Mangled` per
> > > > > > > se sounds unreasonable. There's just too many of them that don't
> > > > > > > require demangling at all. For each successfully initiated
> > > > > > > `partialDemangle()` I will need to keep one of course.
> > > > > > >
> > > > > > > I will have a closer look on Monday. So far thanks for mentioning
> > > > > > > that!
> > > > > > Well, right the pointer to `BumpPointerAllocator` won't solve
> > > > > > anything. Ok will have a look.
> > > > > > ItaniumPartialDemangler dynamically allocates a pretty big buffer
> > > > > > on construction
> > > > >
> > > > > I think in the end each `Mangled` instance will have a pointer to IPD
> > > > > field for lazy access to rich demangling info. This piece of code may
> > > > > become something like:
> > > > > ```
> > > > > m_IPD = new ItaniumPartialDemangler();
> > > > > if (bool err = m_IPD->partialDemangle(mangled_name)) {
> > > > > delete m_IPD; m_IPD = nullptr;
> > > > > }
> > > > > ```
> > > > >
> > > > > In order to avoid unnecessary instantiations, we can consider to
> > > > > filter symbols upfront that we know can't be demangled. E.g. we could
> > > > > duplicate the simple checks from `Db::parse()` before creating a
> > > > > `ItaniumPartialDemangler` instance.
> > > > >
> > > > > Even the simple switch with the above code in place shows performance
> > > > > improvements. So for now I would like to leave it this way and review
> > > > > the issue after having the bigger patch, which will actually make use
> > > > > of the rich demangle info.
> > > > >
> > > > > What do you think?
> > > > Sure, if this is a performance win then I can't think of any reason not
> > > > to do it.
> > > >
> > > > In the future though, I don't think that having copies of IPD in each
> > > > Mangled is a good idea, even if they are lazily initialized. The
> > > > partially demangled state held in IPD is significantly larger than the
> > > > demangled string, so I think it would lead to a lot more memory usage.
> > > > Do you think it is possible to have just one instance of IPD that you
> > > > could use to demangle all the symbols to their chopped-up state, and
> > > > just store that instead?
> > > Yes if that will be a bit more work, but also a possibility. I did a
> > > small experiment making the IPD in line 288 `static`, to reuse a single
> > > instance. That didn't affect runtimes much. I tried it several times and
> > > got the same results again, but have no explanation.
> > >
> > > You would expect a speedup from this right? Is there maybe cleanup work
> > > that eats up time when reusing an instance? Maybe I have to revisit that.
> > Weird, I would have expected a speedup for making it static. My only guess
> > is that `malloc` is just quickly handing back the buffer it used for the
> > last IPD or something. I think the cleanup work IPD does should be about
> > the same as the cost of construction.
> If reusing the same IPD object can bring significant benefit, I think the
> right approach would be to change/extend/add the API (not in this patch) to
> make it possible to use it naturally. There aren't many places that do batch
> demangling of a large amount of symbols (`Symtab::InitNameIndexes` is one I
> recall), so it shouldn't be too hard to modify them to do something
[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet
ramana-nvr added a comment. Sorry, I am not helpful to you in providing a unit test case for this patch. I am still learning about test suite framework and/or trying to get the lldb test suite up and running. Regarding how I got to this, it is not that I have run into some issue due to the original code. For my private port, I had to create a thread ID list similar to m_thread_ids and during code browsing for that I happened to see the code in concern and observed that clearing m_thread_pcs is wrong there. After the process gets stopped, while trying to set the thread stop info, we also update the thread list (ProcessGDBRemote::SetThreadStopInfo() ---> Process::UpdateThreadListIfNeeded() ---> ProcessGDBRemote::UpdateThreadList()) and here we can reach UpdateThreadIDList() if m_thread_ids.empty() is true i.e. if we somehow failed to populate m_thread_ids while parsing the stop info packet and need to populate the m_thread_ids now. Repository: rL LLVM https://reviews.llvm.org/D48868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
I think doing this in the module list is not the right place. Why? Some
platforms might have multiple sysroot to check. iOS for instance has a
directory for each device that Xcode has connected to which can be checked. I
am fine with adding this ability to lldb_private::Platform, but I would just do
it in there. Try GetRemoteSharedModule with the spec, if it fails, try again
after modifying the spec to prepend the sysroot path. Possible even just check
the sysroot path + path first if m_sdk_sysroot is filled in. I don't see the
need to change ModuleList itself.
Comment at: include/lldb/Core/ModuleList.h:544-545
bool *did_create_ptr,
-bool always_create = false);
+bool always_create = false,
+const char* sysroot = nullptr);
static bool RemoveSharedModule(lldb::ModuleSP &module_sp);
Revert this? See my main comment
Comment at: source/Core/ModuleList.cpp:710-714
+ bool *did_create_ptr, bool always_create,
+ const char* sysroot) {
+ // Make sure no one else can try and get or create a module while this
+ // function is actively working on it by doing an extra lock on the
+ // global mutex list.
Revert
Comment at: source/Core/ModuleList.cpp:766-770
+ if (sysroot != nullptr)
+resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot);
+
+ module_sp.reset(new Module(resolved_module_spec));
// Make sure there are a module and an object file since we can specify
Revert
Comment at: source/Target/Platform.cpp:228
module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
-did_create_ptr, false);
+did_create_ptr, false, m_sdk_sysroot.AsCString());
return GetRemoteSharedModule(module_spec, process, module_sp,
Revert
Comment at: source/Target/Platform.cpp:230
return GetRemoteSharedModule(module_spec, process, module_sp,
[&](const ModuleSpec &spec) {
Status error = ModuleList::GetSharedModule(
Here just make a module spec that has m_sdk_sysroot prepended if m_sdk_sysroot
is set, and check for that file first. If that succeeds, return that, else do
the same code as before.
Repository:
rL LLVM
https://reviews.llvm.org/D49685
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337832 - Move dumping code out of RegisterValue class
Author: labath
Date: Tue Jul 24 08:48:13 2018
New Revision: 337832
URL: http://llvm.org/viewvc/llvm-project?rev=337832&view=rev
Log:
Move dumping code out of RegisterValue class
Summary:
The dump function was the only part of this class which depended on
high-level functionality. This was due to the DumpDataExtractor
function, which uses info from a running target to control dump format
(although, RegisterValue doesn't really use the high-level part of
DumpDataExtractor).
This patch follows the same approach done for the DataExtractor class,
and extracts the dumping code into a separate function/file. This file
can stay in the higher level code, while the RegisterValue class and
anything that does not depend in dumping can stay go to lower layers.
The XCode project will need to be updated after this patch.
Reviewers: zturner, jingham, clayborg
Subscribers: lldb-commits, mgorny
Differential Revision: https://reviews.llvm.org/D48351
Added:
lldb/trunk/include/lldb/Core/DumpRegisterValue.h
lldb/trunk/source/Core/DumpRegisterValue.cpp
Modified:
lldb/trunk/include/lldb/Core/RegisterValue.h
lldb/trunk/source/Commands/CommandObjectRegister.cpp
lldb/trunk/source/Core/CMakeLists.txt
lldb/trunk/source/Core/EmulateInstruction.cpp
lldb/trunk/source/Core/FormatEntity.cpp
lldb/trunk/source/Core/RegisterValue.cpp
lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
lldb/trunk/source/Target/ThreadPlanCallFunction.cpp
lldb/trunk/source/Target/ThreadPlanTracer.cpp
Added: lldb/trunk/include/lldb/Core/DumpRegisterValue.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/DumpRegisterValue.h?rev=337832&view=auto
==
--- lldb/trunk/include/lldb/Core/DumpRegisterValue.h (added)
+++ lldb/trunk/include/lldb/Core/DumpRegisterValue.h Tue Jul 24 08:48:13 2018
@@ -0,0 +1,31 @@
+//===-- DumpRegisterValue.h -*- C++
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLDB_CORE_DUMPREGISTERVALUE_H
+#define LLDB_CORE_DUMPREGISTERVALUE_H
+
+#include "lldb/lldb-enumerations.h"
+#include
+
+namespace lldb_private {
+
+class RegisterValue;
+struct RegisterInfo;
+class Stream;
+
+// The default value of 0 for reg_name_right_align_at means no alignment at
+// all.
+bool DumpRegisterValue(const RegisterValue ®_val, Stream *s,
+ const RegisterInfo *reg_info, bool prefix_with_name,
+ bool prefix_with_alt_name, lldb::Format format,
+ uint32_t reg_name_right_align_at = 0);
+
+} // namespace lldb_private
+
+#endif // LLDB_CORE_DUMPREGISTERVALUE_H
Modified: lldb/trunk/include/lldb/Core/RegisterValue.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/RegisterValue.h?rev=337832&r1=337831&r2=337832&view=diff
==
--- lldb/trunk/include/lldb/Core/RegisterValue.h (original)
+++ lldb/trunk/include/lldb/Core/RegisterValue.h Tue Jul 24 08:48:13 2018
@@ -248,12 +248,6 @@ public:
Status SetValueFromData(const RegisterInfo *reg_info, DataExtractor &data,
lldb::offset_t offset, bool partial_data_ok);
- // The default value of 0 for reg_name_right_align_at means no alignment at
- // all.
- bool Dump(Stream *s, const RegisterInfo *reg_info, bool prefix_with_name,
-bool prefix_with_alt_name, lldb::Format format,
-uint32_t reg_name_right_align_at = 0) const;
-
const void *GetBytes() const;
lldb::ByteOrder GetByteOrder() const {
Modified: lldb/trunk/source/Commands/CommandObjectRegister.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectRegister.cpp?rev=337832&r1=337831&r2=337832&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectRegister.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectRegister.cpp Tue Jul 24 08:48:13
2018
@@ -9,6 +9,7 @@
#include "CommandObjectRegister.h"
#include "lldb/Core/Debugger.h"
+#include "lldb/Core/DumpRegisterValue.h"
#include "lldb/Core/RegisterValue.h"
#include "lldb/Core/Scalar.h"
#include "lldb/Host/OptionParser.h"
@@ -92,8 +93,8 @@ public:
bool prefix_with_altname = (bool)m_command_options.alternate_name;
bool prefix_with_name = !prefix_with_altname;
-reg_value.Dump(&strm, reg_info, prefix_with_name, prefix_with_altname,
- m_format_options.GetFormat(), 8);
+DumpRegisterValue(reg_value, &strm, reg_info, prefix_with_name,
+ prefix_with_altname, m_form
[Lldb-commits] [PATCH] D48351: Move dumping code out of RegisterValue class
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337832: Move dumping code out of RegisterValue class
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D48351
Files:
lldb/trunk/include/lldb/Core/DumpRegisterValue.h
lldb/trunk/include/lldb/Core/RegisterValue.h
lldb/trunk/source/Commands/CommandObjectRegister.cpp
lldb/trunk/source/Core/CMakeLists.txt
lldb/trunk/source/Core/DumpRegisterValue.cpp
lldb/trunk/source/Core/EmulateInstruction.cpp
lldb/trunk/source/Core/FormatEntity.cpp
lldb/trunk/source/Core/RegisterValue.cpp
lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
lldb/trunk/source/Target/ThreadPlanCallFunction.cpp
lldb/trunk/source/Target/ThreadPlanTracer.cpp
Index: lldb/trunk/source/Core/RegisterValue.cpp
===
--- lldb/trunk/source/Core/RegisterValue.cpp
+++ lldb/trunk/source/Core/RegisterValue.cpp
@@ -9,7 +9,6 @@
#include "lldb/Core/RegisterValue.h"
-#include "lldb/Core/DumpDataExtractor.h"
#include "lldb/Core/Scalar.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/DataExtractor.h"
@@ -34,67 +33,6 @@
using namespace lldb;
using namespace lldb_private;
-bool RegisterValue::Dump(Stream *s, const RegisterInfo *reg_info,
- bool prefix_with_name, bool prefix_with_alt_name,
- Format format,
- uint32_t reg_name_right_align_at) const {
- DataExtractor data;
- if (GetData(data)) {
-bool name_printed = false;
-// For simplicity, alignment of the register name printing applies only in
-// the most common case where:
-//
-// prefix_with_name^prefix_with_alt_name is true
-//
-StreamString format_string;
-if (reg_name_right_align_at && (prefix_with_name ^ prefix_with_alt_name))
- format_string.Printf("%%%us", reg_name_right_align_at);
-else
- format_string.Printf("%%s");
-std::string fmt = format_string.GetString();
-if (prefix_with_name) {
- if (reg_info->name) {
-s->Printf(fmt.c_str(), reg_info->name);
-name_printed = true;
- } else if (reg_info->alt_name) {
-s->Printf(fmt.c_str(), reg_info->alt_name);
-prefix_with_alt_name = false;
-name_printed = true;
- }
-}
-if (prefix_with_alt_name) {
- if (name_printed)
-s->PutChar('/');
- if (reg_info->alt_name) {
-s->Printf(fmt.c_str(), reg_info->alt_name);
-name_printed = true;
- } else if (!name_printed) {
-// No alternate name but we were asked to display a name, so show the
-// main name
-s->Printf(fmt.c_str(), reg_info->name);
-name_printed = true;
- }
-}
-if (name_printed)
- s->PutCString(" = ");
-
-if (format == eFormatDefault)
- format = reg_info->format;
-
-DumpDataExtractor(data, s,
- 0,// Offset in "data"
- format, // Format to use when dumping
- reg_info->byte_size, // item_byte_size
- 1,// item_count
- UINT32_MAX, // num_per_line
- LLDB_INVALID_ADDRESS, // base_addr
- 0,// item_bit_size
- 0); // item_bit_offset
-return true;
- }
- return false;
-}
-
bool RegisterValue::GetData(DataExtractor &data) const {
return data.SetData(GetBytes(), GetByteSize(), GetByteOrder()) > 0;
}
Index: lldb/trunk/source/Core/DumpRegisterValue.cpp
===
--- lldb/trunk/source/Core/DumpRegisterValue.cpp
+++ lldb/trunk/source/Core/DumpRegisterValue.cpp
@@ -0,0 +1,79 @@
+//===-- DumpRegisterValue.cpp ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Core/DumpRegisterValue.h"
+#include "lldb/Core/DumpDataExtractor.h"
+#include "lldb/Core/RegisterValue.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/StreamString.h"
+#include "lldb/lldb-private-types.h"
+
+using namespace lldb;
+
+bool lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream *s,
+ const RegisterInfo *reg_info,
+ bool prefix_with_name,
+ bool prefix_with_alt_name, Format format,
+ uint32_t reg_name_right_align_at) {
+ DataExtractor data;
+ if (reg_val.GetData(data)) {
+bool na
[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.
apolyakov created this revision.
apolyakov added reviewers: aprantl, clayborg, labath.
Herald added a subscriber: ki.stfu.
In this patch I suggest a way to deal with the problem started in
https://reviews.llvm.org/D47302 and use it to re-implement MI target-select
command. You are welcome to comment this patch, especially the new methods of
Target and SBTarget classes and the bash script(do we need to find a random
port or should we hardcode it?).
https://reviews.llvm.org/D49739
Files:
include/lldb/API/SBTarget.h
include/lldb/Target/Target.h
lit/lit.cfg
lit/tools/lldb-mi/target/inputs/main.c
lit/tools/lldb-mi/target/inputs/target-select-so-path.sh
lit/tools/lldb-mi/target/lit.local.cfg
lit/tools/lldb-mi/target/target-select-so-path.test
source/API/SBTarget.cpp
source/Target/Target.cpp
tools/lldb-mi/MICmdCmdTarget.cpp
Index: tools/lldb-mi/MICmdCmdTarget.cpp
===
--- tools/lldb-mi/MICmdCmdTarget.cpp
+++ tools/lldb-mi/MICmdCmdTarget.cpp
@@ -10,9 +10,8 @@
// Overview:CMICmdCmdTargetSelect implementation.
// Third Party Headers:
-#include "lldb/API/SBCommandInterpreter.h"
-#include "lldb/API/SBCommandReturnObject.h"
#include "lldb/API/SBStream.h"
+#include "lldb/API/SBError.h"
// In-house headers:
#include "MICmdArgValNumber.h"
@@ -52,7 +51,7 @@
// Return: None.
// Throws: None.
//--
-CMICmdCmdTargetSelect::~CMICmdCmdTargetSelect() {}
+CMICmdCmdTargetSelect::~CMICmdCmdTargetSelect() = default;
//++
//
@@ -93,51 +92,44 @@
CMICmnLLDBDebugSessionInfo &rSessionInfo(
CMICmnLLDBDebugSessionInfo::Instance());
+ lldb::SBTarget target = rSessionInfo.GetTarget();
- // Check we have a valid target
- // Note: target created via 'file-exec-and-symbols' command
- if (!rSessionInfo.GetTarget().IsValid()) {
+ // Check we have a valid target.
+ // Note: target created via 'file-exec-and-symbols' command.
+ if (!target.IsValid()) {
SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_TARGET_CURRENT),
m_cmdData.strMiCmd.c_str()));
return MIstatus::failure;
}
- // Verify that we are executing remotely
+ // Verify that we are executing remotely.
const CMIUtilString &rRemoteType(pArgType->GetValue());
if (rRemoteType != "remote") {
SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_TARGET_TYPE),
m_cmdData.strMiCmd.c_str(),
rRemoteType.c_str()));
return MIstatus::failure;
}
- // Create a URL pointing to the remote gdb stub
+ // Create a URL pointing to the remote gdb stub.
const CMIUtilString strUrl =
CMIUtilString::Format("connect://%s", pArgParameters->GetValue().c_str());
- // Ask LLDB to connect to the target port
- const char *pPlugin("gdb-remote");
lldb::SBError error;
- lldb::SBProcess process = rSessionInfo.GetTarget().ConnectRemote(
+ // Ask LLDB to connect to the target port.
+ const char *pPlugin("gdb-remote");
+ lldb::SBProcess process = target.ConnectRemote(
rSessionInfo.GetListener(), strUrl.c_str(), pPlugin, error);
- // Verify that we have managed to connect successfully
- lldb::SBStream errMsg;
- error.GetDescription(errMsg);
+ // Verify that we have managed to connect successfully.
if (!process.IsValid()) {
SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_TARGET_PLUGIN),
m_cmdData.strMiCmd.c_str(),
- errMsg.GetData()));
-return MIstatus::failure;
- }
- if (error.Fail()) {
-SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_CONNECT_TO_TARGET),
- m_cmdData.strMiCmd.c_str(),
- errMsg.GetData()));
+ error.GetCString()));
return MIstatus::failure;
}
- // Set the environment path if we were given one
+ // Set the environment path if we were given one.
CMIUtilString strWkDir;
if (rSessionInfo.SharedDataRetrieve(
rSessionInfo.m_constStrSharedDataKeyWkDir, strWkDir)) {
@@ -150,28 +142,13 @@
}
}
- // Set the shared object path if we were given one
+ // Set the shared object path if we were given one.
CMIUtilString strSolibPath;
if (rSessionInfo.SharedDataRetrieve(
- rSessionInfo.m_constStrSharedDataSolibPath, strSolibPath)) {
-lldb::SBDebugger &rDbgr = rSessionInfo.GetDebugger();
-lldb::SBCommandInterpreter cmdIterpreter = rDbgr.GetCommandInterpreter();
-
-CMIUtilString strCmdString = CMIUtilString::Format(
-"target modules search-paths add . %s", strSolibPath.c_str());
-
-lldb::SBCommandReturnObject retObj;
-cmdIterpreter.HandleCommand(strCmdString.c_str(), retObj, false);
+ rSessionInfo.m_constStrSharedDataSolibPath, strSol
[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.
clayborg added inline comments.
Comment at: include/lldb/API/SBTarget.h:275-276
+ void AppendImageSearchPath(const char *from, const char *to);
+
+ void AppendImageSearchPath(const char *from, const char *to,
Remove this first one? Other functions were first created and gave no error
feedback and then another version was added that had an error. We should start
with one that has an error and not ever add this version.
Comment at: include/lldb/Target/Target.h:1003-1005
+ void AppendImageSearchPath(const ConstString &from,
+ const ConstString &to);
+
Remove and just use "PathMappingList &Target::GetImageSearchPathList();"
Comment at: lit/tools/lldb-mi/target/inputs/target-select-so-path.sh:3-11
+function get_random_unused_port {
+local port=$(shuf -i 2000-65000 -n 1)
+netstat -lat | grep $port > /dev/null
+if [[ $? == 1 ]] ; then
+echo $port
+else
+get_random_unused_port
This seems like it could be racy and cause problems on buildbots.
Comment at: source/API/SBTarget.cpp:1460-1464
+void SBTarget::AppendImageSearchPath(const char *from, const char *to) {
+ lldb::SBError error; // Ignored
+ AppendImageSearchPath(from, to, error);
+}
+
Remove this version that takes no error
Comment at: source/Target/Target.cpp:2055-2059
+void Target::AppendImageSearchPath(const ConstString &from,
+ const ConstString &to) {
+ m_image_search_paths.Append(from, to, true);
+}
+
You don't need this, just call "target. GetImageSearchPathList().Append(...)
https://reviews.llvm.org/D49739
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
lemo added subscribers: clayborg, EugeneBi, labath, lemo. lemo added a comment. > The problem is that shared libraries differ on these machines and > LLDB either fails to load some libraries *or loads wrong ones*. Not finding the modules is not surprising but the latter (loading the wrong modules) is a bit concerning. Do you know why the module build-id is not used when searching for the local binary? Repository: rL LLVM https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
>
> The problem is that shared libraries differ on these machines and
> LLDB either fails to load some libraries *or loads wrong ones*.
>
Not finding the modules is not surprising but the latter (loading the wrong
modules) is a bit concerning. Do you know why the module build-id is not
used when searching for the local binary?
On Tue, Jul 24, 2018 at 7:19 AM, Greg Clayton via Phabricator via
lldb-commits wrote:
> clayborg requested changes to this revision.
> clayborg added a comment.
> This revision now requires changes to proceed.
>
> I think doing this in the module list is not the right place. Why? Some
> platforms might have multiple sysroot to check. iOS for instance has a
> directory for each device that Xcode has connected to which can be checked.
> I am fine with adding this ability to lldb_private::Platform, but I would
> just do it in there. Try GetRemoteSharedModule with the spec, if it fails,
> try again after modifying the spec to prepend the sysroot path. Possible
> even just check the sysroot path + path first if m_sdk_sysroot is filled
> in. I don't see the need to change ModuleList itself.
>
>
>
>
> Comment at: include/lldb/Core/ModuleList.h:544-545
> bool *did_create_ptr,
> -bool always_create = false);
> +bool always_create = false,
> +const char* sysroot = nullptr);
>static bool RemoveSharedModule(lldb::ModuleSP &module_sp);
>
> Revert this? See my main comment
>
>
>
> Comment at: source/Core/ModuleList.cpp:710-714
> + bool *did_create_ptr, bool
> always_create,
> + const char* sysroot) {
> + // Make sure no one else can try and get or create a module while this
> + // function is actively working on it by doing an extra lock on the
> + // global mutex list.
>
> Revert
>
>
>
> Comment at: source/Core/ModuleList.cpp:766-770
> + if (sysroot != nullptr)
> +resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot);
> +
> + module_sp.reset(new Module(resolved_module_spec));
>// Make sure there are a module and an object file since we can specify
>
> Revert
>
>
>
> Comment at: source/Target/Platform.cpp:228
> module_spec, module_sp, module_search_paths_ptr,
> old_module_sp_ptr,
> -did_create_ptr, false);
> +did_create_ptr, false, m_sdk_sysroot.AsCString());
>return GetRemoteSharedModule(module_spec, process, module_sp,
>
> Revert
>
>
>
> Comment at: source/Target/Platform.cpp:230
>return GetRemoteSharedModule(module_spec, process, module_sp,
> [&](const ModuleSpec &spec) {
> Status error =
> ModuleList::GetSharedModule(
>
> Here just make a module spec that has m_sdk_sysroot prepended if
> m_sdk_sysroot is set, and check for that file first. If that succeeds,
> return that, else do the same code as before.
>
>
> Repository:
> rL LLVM
>
> https://reviews.llvm.org/D49685
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
clayborg added a comment. We should never be loading the wrong shared libraries. The module spec we fill out must contain the UUID of the file are looking for. If it doesn't we have no chance of every really loading the right stuff. Repository: rL LLVM https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.
labath added a comment. I am sure that particular test is worth trying to implement in lit. That script of yours is full of operating system specifics, and it's going to be super flaky. I'd suggest keeping this as a python test, as there you can abstract the platform specifics much easier, and we also have a lot of code we can (re)use to setup socket connections safely. (e.g. lldb-server has a mode where *it* selects a port to listen on (race-free), and then you can just fetch that port and connect to it.) Doing that from a shell script is going to be painful. I suppose if you really want to make this a lit test, you should at least ditch the bash script and replace that bit with python. https://reviews.llvm.org/D49739 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
On Tue, 24 Jul 2018 at 17:17, Leonard Mosescu wrote: >> >> The problem is that shared libraries differ on these machines and >> LLDB either fails to load some libraries or loads wrong ones. > > > Not finding the modules is not surprising but the latter (loading the wrong > modules) is a bit concerning. Do you know why the module build-id is not used > when searching for the local binary? > > Core files don't contain a build-id, just a file path. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support
teemperor updated this revision to Diff 157054.
teemperor added a comment.
[Updated patch to address Pavel's comments, thanks!]
@zturner So I looked into the Windows support:
Windows requires us to directly flush/signal/write/flush to the console output
stream. However lldb's output is buffered by design into a StreamString first
to prevent overlapping with the process output. So we can't just add Windows
support to the coloring backend as we don't have direct access to the output
stream.
If we want to fix this in lldb, then we could write an ANSI color code
interpreter and let that run over our final output while we print it. That
wouldn't be too complex and would fix all existing coloring output in lldb. It
would also fix that lldb configs that enable color support on Windows are
broken (because people just add color codes there).
However, it seems Windows starting with https://reviews.llvm.org/W10 anyway
supports ANSI color codes (or at least you can enable them in the settings). So
that interpreter is only really necessary until everyone moved to a version
that supports color codes:
https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences#span-idtextformattingspanspan-idtextformattingspanspan-idtextformattingspantext-formatting
So I would suggest we maybe just hack in the color interpreter and drop it when
https://reviews.llvm.org/W7 reaches EoL (?). I can make another patch for that
if it sounds good.
https://reviews.llvm.org/D49334
Files:
include/lldb/Core/Debugger.h
include/lldb/Core/Highlighter.h
include/lldb/Target/Language.h
packages/Python/lldbsuite/test/source-manager/TestSourceManager.py
source/Core/CMakeLists.txt
source/Core/Debugger.cpp
source/Core/Highlighter.cpp
source/Core/SourceManager.cpp
source/Plugins/Language/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
source/Plugins/Language/ClangCommon/CMakeLists.txt
source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
source/Plugins/Language/ClangCommon/ClangHighlighter.h
source/Plugins/Language/Go/GoLanguage.cpp
source/Plugins/Language/Go/GoLanguage.h
source/Plugins/Language/Java/JavaLanguage.cpp
source/Plugins/Language/Java/JavaLanguage.h
source/Plugins/Language/OCaml/OCamlLanguage.cpp
source/Plugins/Language/OCaml/OCamlLanguage.h
source/Plugins/Language/ObjC/CMakeLists.txt
source/Plugins/Language/ObjC/ObjCLanguage.cpp
source/Plugins/Language/ObjC/ObjCLanguage.h
source/Plugins/Language/ObjCPlusPlus/CMakeLists.txt
source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp
source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
source/Target/Language.cpp
unittests/Language/CMakeLists.txt
unittests/Language/Highlighting/CMakeLists.txt
unittests/Language/Highlighting/HighlighterTest.cpp
Index: unittests/Language/Highlighting/HighlighterTest.cpp
===
--- /dev/null
+++ unittests/Language/Highlighting/HighlighterTest.cpp
@@ -0,0 +1,228 @@
+//===-- HighlighterTest.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/Core/Highlighter.h"
+
+#include "llvm/Support/Threading.h"
+
+#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/Language/Go/GoLanguage.h"
+#include "Plugins/Language/Java/JavaLanguage.h"
+#include "Plugins/Language/OCaml/OCamlLanguage.h"
+#include "Plugins/Language/ObjC/ObjCLanguage.h"
+#include "Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h"
+
+using namespace lldb_private;
+
+static void InitLanguages() {
+ static llvm::once_flag g_once_flag;
+ llvm::call_once(g_once_flag, []() {
+// The HighlighterManager uses the language plugins under the hood, so we
+// have to initialize them here for our test process.
+CPlusPlusLanguage::Initialize();
+GoLanguage::Initialize();
+JavaLanguage::Initialize();
+ObjCLanguage::Initialize();
+ObjCPlusPlusLanguage::Initialize();
+OCamlLanguage::Initialize();
+ });
+}
+
+static std::string getName(lldb::LanguageType type) {
+ HighlighterManager mgr;
+ return mgr.getHighlighterFor(type, "").GetName().str();
+}
+
+static std::string getName(llvm::StringRef path) {
+ HighlighterManager mgr;
+ return mgr.getHighlighterFor(lldb::eLanguageTypeUnknown, path)
+ .GetName()
+ .str();
+}
+
+TEST(HighlighterTest, HighlighterSelectionType) {
+ InitLanguages();
+ EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus), "clang");
+ EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_03), "clang");
+ EXPECT_EQ(getName(lldb::eL
[Lldb-commits] [PATCH] D49740: Move RegisterValue, Scalar, State from Core to Utility
teemperor added a comment. Did you test that with lldb's C++ modules? If not I can test it locally. https://reviews.llvm.org/D49740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
sgraenitz updated this revision to Diff 157061.
sgraenitz added a comment.
Change EXPECTED_EQ to EXPECTED_STREQ, because ConstString::operator==()
compares identity of pointers, not equality of strings. While "they must come
from the same pool in order to be equal" (as stated in the description), this
is verifying string equality not the internals of ConstString.
https://reviews.llvm.org/D49612
Files:
cmake/modules/LLDBConfig.cmake
lldb.xcodeproj/project.pbxproj
source/Core/Mangled.cpp
unittests/Core/CMakeLists.txt
unittests/Core/MangledTest.cpp
Index: unittests/Core/MangledTest.cpp
===
--- /dev/null
+++ unittests/Core/MangledTest.cpp
@@ -0,0 +1,38 @@
+//===-- MangledTest.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/Core/Mangled.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(MangledTest, ResultForValidName) {
+ ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+ bool IsMangled = true;
+
+ Mangled TheMangled(MangledName, IsMangled);
+ const ConstString &TheDemangled =
+ TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+ ConstString ExpectedResult("void a::b::c(unsigned long)");
+ EXPECT_STREQ(ExpectedResult.GetCString(), TheDemangled.GetCString());
+}
+
+TEST(MangledTest, EmptyForInvalidName) {
+ ConstString MangledName("_ZN1a1b1cmxktpEEvm");
+ bool IsMangled = true;
+
+ Mangled TheMangled(MangledName, IsMangled);
+ const ConstString &TheDemangled =
+ TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+ EXPECT_STREQ("", TheDemangled.GetCString());
+}
Index: unittests/Core/CMakeLists.txt
===
--- unittests/Core/CMakeLists.txt
+++ unittests/Core/CMakeLists.txt
@@ -1,6 +1,7 @@
add_lldb_unittest(LLDBCoreTests
BroadcasterTest.cpp
DataExtractorTest.cpp
+ MangledTest.cpp
ListenerTest.cpp
ScalarTest.cpp
StateTest.cpp
Index: source/Core/Mangled.cpp
===
--- source/Core/Mangled.cpp
+++ source/Core/Mangled.cpp
@@ -16,34 +16,24 @@
#pragma comment(lib, "dbghelp.lib")
#endif
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-// Provide a fast-path demangler implemented in FastDemangle.cpp until it can
-// replace the existing C++ demangler with a complete implementation
-#include "lldb/Utility/FastDemangle.h"
-#include "llvm/Demangle/Demangle.h"
-#else
-// FreeBSD9-STABLE requires this to know about size_t in cxxabi.
-#include
-#include
-#endif
-
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Logging.h"
#include "lldb/Utility/RegularExpression.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/Timer.h"
-#include "lldb/lldb-enumerations.h" // for LanguageType
+#include "lldb/lldb-enumerations.h"
#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
#include "Plugins/Language/ObjC/ObjCLanguage.h"
-#include "llvm/ADT/StringRef.h"// for StringRef
-#include "llvm/Support/Compiler.h" // for LLVM_PRETT...
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Demangle/Demangle.h"
+#include "llvm/Support/Compiler.h"
-#include// for mutex, loc...
-#include // for string
-#include // for pair
+#include
+#include
+#include
#include
#include
@@ -295,18 +285,15 @@
break;
}
case eManglingSchemeItanium: {
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-if (log)
- log->Printf("demangle itanium: %s", mangled_name);
-// Try to use the fast-path demangler first for the performance win,
-// falling back to the full demangler only when necessary
-demangled_name = FastDemangle(mangled_name, m_mangled.GetLength());
-if (!demangled_name)
- demangled_name =
- llvm::itaniumDemangle(mangled_name, NULL, NULL, NULL);
-#else
-demangled_name = abi::__cxa_demangle(mangled_name, NULL, NULL, NULL);
-#endif
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
+if (!demangle_err) {
+ // Default buffer and size (realloc is used in case it's too small).
+ size_t demangled_size = 80;
+ demangled_name = static_cast(::malloc(demangled_size));
+ demangled_name = IPD.finishDemangle(demangled_name, &demangled_size);
+}
+
if (log) {
if (demangled_name)
log->Printf("demangled itanium: %s -> \"%s\"", mangled_name,
Index: lldb.xcodeproj/project.pbxproj
===
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/pro
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
teemperor added inline comments. Comment at: unittests/Core/CMakeLists.txt:4 DataExtractorTest.cpp + MangledTest.cpp ListenerTest.cpp This should be after ListenerTest.cpp to keep this file in alphabetical order. https://reviews.llvm.org/D49612 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
sgraenitz updated this revision to Diff 157062.
sgraenitz added a comment.
Keep alphabetical order of files in CMakeLists.txt
https://reviews.llvm.org/D49612
Files:
cmake/modules/LLDBConfig.cmake
lldb.xcodeproj/project.pbxproj
source/Core/Mangled.cpp
unittests/Core/CMakeLists.txt
unittests/Core/MangledTest.cpp
Index: unittests/Core/MangledTest.cpp
===
--- /dev/null
+++ unittests/Core/MangledTest.cpp
@@ -0,0 +1,38 @@
+//===-- MangledTest.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/Core/Mangled.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(MangledTest, ResultForValidName) {
+ ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+ bool IsMangled = true;
+
+ Mangled TheMangled(MangledName, IsMangled);
+ const ConstString &TheDemangled =
+ TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+ ConstString ExpectedResult("void a::b::c(unsigned long)");
+ EXPECT_STREQ(ExpectedResult.GetCString(), TheDemangled.GetCString());
+}
+
+TEST(MangledTest, EmptyForInvalidName) {
+ ConstString MangledName("_ZN1a1b1cmxktpEEvm");
+ bool IsMangled = true;
+
+ Mangled TheMangled(MangledName, IsMangled);
+ const ConstString &TheDemangled =
+ TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+ EXPECT_STREQ("", TheDemangled.GetCString());
+}
Index: unittests/Core/CMakeLists.txt
===
--- unittests/Core/CMakeLists.txt
+++ unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
BroadcasterTest.cpp
DataExtractorTest.cpp
ListenerTest.cpp
+ MangledTest.cpp
ScalarTest.cpp
StateTest.cpp
StreamCallbackTest.cpp
Index: source/Core/Mangled.cpp
===
--- source/Core/Mangled.cpp
+++ source/Core/Mangled.cpp
@@ -16,34 +16,24 @@
#pragma comment(lib, "dbghelp.lib")
#endif
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-// Provide a fast-path demangler implemented in FastDemangle.cpp until it can
-// replace the existing C++ demangler with a complete implementation
-#include "lldb/Utility/FastDemangle.h"
-#include "llvm/Demangle/Demangle.h"
-#else
-// FreeBSD9-STABLE requires this to know about size_t in cxxabi.
-#include
-#include
-#endif
-
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Logging.h"
#include "lldb/Utility/RegularExpression.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/Timer.h"
-#include "lldb/lldb-enumerations.h" // for LanguageType
+#include "lldb/lldb-enumerations.h"
#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
#include "Plugins/Language/ObjC/ObjCLanguage.h"
-#include "llvm/ADT/StringRef.h"// for StringRef
-#include "llvm/Support/Compiler.h" // for LLVM_PRETT...
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Demangle/Demangle.h"
+#include "llvm/Support/Compiler.h"
-#include// for mutex, loc...
-#include // for string
-#include // for pair
+#include
+#include
+#include
#include
#include
@@ -295,18 +285,15 @@
break;
}
case eManglingSchemeItanium: {
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-if (log)
- log->Printf("demangle itanium: %s", mangled_name);
-// Try to use the fast-path demangler first for the performance win,
-// falling back to the full demangler only when necessary
-demangled_name = FastDemangle(mangled_name, m_mangled.GetLength());
-if (!demangled_name)
- demangled_name =
- llvm::itaniumDemangle(mangled_name, NULL, NULL, NULL);
-#else
-demangled_name = abi::__cxa_demangle(mangled_name, NULL, NULL, NULL);
-#endif
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
+if (!demangle_err) {
+ // Default buffer and size (realloc is used in case it's too small).
+ size_t demangled_size = 80;
+ demangled_name = static_cast(::malloc(demangled_size));
+ demangled_name = IPD.finishDemangle(demangled_name, &demangled_size);
+}
+
if (log) {
if (demangled_name)
log->Printf("demangled itanium: %s -> \"%s\"", mangled_name,
Index: lldb.xcodeproj/project.pbxproj
===
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/project.pbxproj
@@ -483,6 +483,7 @@
9A20573A1F3B8E7E00F6C293 /* MainLoopTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A2057301F3B8E7600F6C293 /* MainLoopTest.cpp */; };
8C3BD9961EF45DA50016C343 /* MainThreadCheckerRuntime.cpp in Sources
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
sgraenitz marked an inline comment as done. sgraenitz added inline comments. Comment at: unittests/Core/CMakeLists.txt:4 DataExtractorTest.cpp + MangledTest.cpp ListenerTest.cpp teemperor wrote: > This should be after ListenerTest.cpp to keep this file in alphabetical order. Hehe good catch! https://reviews.llvm.org/D49612 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
EugeneBi added a comment. In https://reviews.llvm.org/D49685#1173640, @clayborg wrote: > We should never be loading the wrong shared libraries. The module spec we > fill out must contain the UUID of the file are looking for. If it doesn't we > have no chance of every really loading the right stuff. Well, that's what it does on my machine. So prepending sysroot *after* trying to load module without it will cause problems to me. Also, I assume that if you specified sysroot for a platform, we should not try to load solibs without it - these paths are just not a part of the platform. > I think doing this in the module list is not the right place. Why? Some > platforms might have multiple sysroot to check. iOS for instance has a > directory for each device that Xcode has connected to which can be checked. I > am fine with adding this ability to> lldb_private::Platform, but I would > just do it in there. Try GetRemoteSharedModule with the spec, if it fails, > try again after modifying the spec to prepend the sysroot path. Possible even > just check the sysroot path + path first if m_sdk_sysroot is filled in. I > don't see the need to change ModuleList itself. I do not see how this prepend sysroot could be done in the platform... Essentially, my fix is doing what you suggest: the platform supplies sysroot to the module list, and two different platforms (for two XCode devices, etc.) would supply different sysroots. What do I miss? Repository: rL LLVM https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49713: Replace StreamTee's recursive_mutex with a normal mutex.
teemperor added a reviewer: clayborg. teemperor added a comment. Adding Greg because he seem to be the one who added the lock. I think it's also worth investigating why this lock is there in the first place (as other Stream implementations are not thread safe, but this one is?) https://reviews.llvm.org/D49713 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
clayborg added a comment. In https://reviews.llvm.org/D49685#1173720, @EugeneBi wrote: > In https://reviews.llvm.org/D49685#1173640, @clayborg wrote: > > > We should never be loading the wrong shared libraries. The module spec we > > fill out must contain the UUID of the file are looking for. If it doesn't > > we have no chance of every really loading the right stuff. > > > Well, that's what it does on my machine. So prepending sysroot *after* trying > to load module without it will cause problems to me. Also, I assume that if > you specified sysroot for a platform, we should not try to load solibs > without it - these paths are just not a part of the platform. > > > I think doing this in the module list is not the right place. Why? Some > > platforms might have multiple sysroot to check. iOS for instance has a > > directory for each device that Xcode has connected to which can be checked. > > I am fine with adding this ability to> lldb_private::Platform, but I would > > just do it in there. Try GetRemoteSharedModule with the spec, if it fails, > > try again after modifying the spec to prepend the sysroot path. Possible > > even just check the sysroot path + path first if m_sdk_sysroot is filled > > in. I don't see the need to change ModuleList itself. > > I do not see how this prepend sysroot could be done in the platform... > Essentially, my fix is doing what you suggest: the platform supplies sysroot > to the module list, and two different platforms (for two XCode devices, etc.) > would supply different sysroots. What do I miss? You would just move: auto resolved_module_spec(module_spec); if (sysroot != nullptr) resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot); into the code in Platform.cpp and do it all there. Repository: rL LLVM https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
EugeneBi updated this revision to Diff 157088.
EugeneBi added a comment.
Rebased to recent master.
Included the whole file in diff.
https://reviews.llvm.org/D49685
Files:
include/lldb/Core/ModuleList.h
source/Core/ModuleList.cpp
source/Target/Platform.cpp
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -226,13 +226,14 @@
if (IsHost())
return ModuleList::GetSharedModule(
module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
-did_create_ptr, false);
+did_create_ptr, false, m_sdk_sysroot.AsCString());
return GetRemoteSharedModule(module_spec, process, module_sp,
[&](const ModuleSpec &spec) {
Status error = ModuleList::GetSharedModule(
spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
+ old_module_sp_ptr, did_create_ptr, false,
+ m_sdk_sysroot.AsCString());
if (error.Success() && module_sp)
module_sp->SetPlatformFileSpec(
spec.GetFileSpec());
Index: source/Core/ModuleList.cpp
===
--- source/Core/ModuleList.cpp
+++ source/Core/ModuleList.cpp
@@ -770,7 +770,11 @@
ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr,
ModuleSP *old_module_sp_ptr,
- bool *did_create_ptr, bool always_create) {
+ bool *did_create_ptr, bool always_create,
+ const char* sysroot) {
+ // Make sure no one else can try and get or create a module while this
+ // function is actively working on it by doing an extra lock on the
+ // global mutex list.
ModuleList &shared_module_list = GetSharedModuleList();
std::lock_guard guard(
shared_module_list.m_modules_mutex);
@@ -789,9 +793,6 @@
const FileSpec &module_file_spec = module_spec.GetFileSpec();
const ArchSpec &arch = module_spec.GetArchitecture();
- // Make sure no one else can try and get or create a module while this
- // function is actively working on it by doing an extra lock on the global
- // mutex list.
if (!always_create) {
ModuleList matching_module_list;
const size_t num_matching_modules =
@@ -825,7 +826,11 @@
if (module_sp)
return error;
- module_sp.reset(new Module(module_spec));
+ auto resolved_module_spec(module_spec);
+ if (sysroot != nullptr)
+resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot);
+
+ module_sp.reset(new Module(resolved_module_spec));
// Make sure there are a module and an object file since we can specify a
// valid file path with an architecture that might not be in that file. By
// getting the object file we can guarantee that the architecture matches
Index: include/lldb/Core/ModuleList.h
===
--- include/lldb/Core/ModuleList.h
+++ include/lldb/Core/ModuleList.h
@@ -537,7 +537,8 @@
const FileSpecList *module_search_paths_ptr,
lldb::ModuleSP *old_module_sp_ptr,
bool *did_create_ptr,
-bool always_create = false);
+bool always_create = false,
+const char* sysroot = nullptr);
static bool RemoveSharedModule(lldb::ModuleSP &module_sp);
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -226,13 +226,14 @@
if (IsHost())
return ModuleList::GetSharedModule(
module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
-did_create_ptr, false);
+did_create_ptr, false, m_sdk_sysroot.AsCString());
return GetRemoteSharedModule(module_spec, process, module_sp,
[&](const ModuleSpec &spec) {
Status error = ModuleList::GetSharedModule(
spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
+ old_module_sp_ptr, did_create_ptr, false,
+ m_sdk_sysroot.AsCString());
if (error.Success() && module_sp)
module_sp->SetPlatformFileSpec(
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
EugeneBi added a comment. I need to convert char* to StringRef yet. https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
EugeneBi added a comment. > You would just move: > > auto resolved_module_spec(module_spec); > if (sysroot != nullptr) > resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot); > > > into the code in Platform.cpp and do it all there. Ah, I see. Will do, thanks. https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.
clayborg created this revision.
clayborg added reviewers: labath, zturner, markmentovai.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.
In this patch I add support for ARM and ARM64 break pad files. There are two
flavors of ARM: Apple where FP is https://reviews.llvm.org/source/openmp/, and
non Apple where FP is https://reviews.llvm.org/source/libunwind/. Added minimal
tests that load up ARM64 and the two flavors or ARM core files with a single
thread and known register values in each register. Each register is checked for
the exact value.
https://reviews.llvm.org/D49750
Files:
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm-linux.dmp
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm-macos.dmp
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm64-macos.dmp
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/ProcessMinidump.cpp
source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h
source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
source/Plugins/Process/minidump/ThreadMinidump.cpp
Index: source/Plugins/Process/minidump/ThreadMinidump.cpp
===
--- source/Plugins/Process/minidump/ThreadMinidump.cpp
+++ source/Plugins/Process/minidump/ThreadMinidump.cpp
@@ -13,6 +13,8 @@
#include "RegisterContextMinidump_x86_32.h"
#include "RegisterContextMinidump_x86_64.h"
+#include "RegisterContextMinidump_ARM.h"
+#include "RegisterContextMinidump_ARM64.h"
// Other libraries and framework includes
#include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
@@ -88,6 +90,20 @@
*this, reg_interface, gpregset, {}));
break;
}
+case llvm::Triple::aarch64: {
+ DataExtractor data(m_gpregset_data.data(), m_gpregset_data.size(),
+ lldb::eByteOrderLittle, 8);
+ m_thread_reg_ctx_sp.reset(new RegisterContextMinidump_ARM64(*this, data));
+ break;
+}
+case llvm::Triple::arm: {
+ DataExtractor data(m_gpregset_data.data(), m_gpregset_data.size(),
+ lldb::eByteOrderLittle, 8);
+ const bool apple = arch.GetTriple().getVendor() == llvm::Triple::Apple;
+ m_thread_reg_ctx_sp.reset(new RegisterContextMinidump_ARM(*this, data,
+apple));
+ break;
+}
default:
break;
}
Index: source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
===
--- source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
+++ source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
@@ -0,0 +1,86 @@
+//===-- RegisterContextMinidump_ARM64.h -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef liblldb_RegisterContextMinidump_ARM64_h_
+#define liblldb_RegisterContextMinidump_ARM64_h_
+
+// Project includes
+#include "MinidumpTypes.h"
+
+// Other libraries and framework includes
+#include "Plugins/Process/Utility/RegisterInfoInterface.h"
+#include "lldb/Target/RegisterContext.h"
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/BitmaskEnum.h"
+
+// C includes
+// C++ includes
+
+namespace lldb_private {
+
+namespace minidump {
+
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
+class RegisterContextMinidump_ARM64: public lldb_private::RegisterContext {
+public:
+ RegisterContextMinidump_ARM64(lldb_private::Thread &thread,
+ const DataExtractor &data);
+
+ ~RegisterContextMinidump_ARM64() override {}
+
+ void InvalidateAllRegisters() override {
+// Do nothing... registers are always valid...
+ }
+
+ size_t GetRegisterCount() override;
+
+ const lldb_private::RegisterInfo *GetRegisterInfoAtIndex(size_t reg) override;
+
+ size_t GetRegisterSetCount() override;
+
+ const lldb_private::RegisterSet *GetRegisterSet(size_t set) override;
+
+ const char *GetRegisterName(unsigned reg);
+
+ bool ReadRegister(const RegisterInfo *reg_info,
+RegisterValue ®_value) override;
+
+ bool WriteRegister(const RegisterInfo *reg_info,
+ const RegisterValue ®_value) override;
+
+
+ uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
+
[Lldb-commits] [PATCH] D49755: Remove unused History class
teemperor created this revision.
Herald added a subscriber: mgorny.
This class doesn't seem to be used anywhere, so we might as well remove the
code.
https://reviews.llvm.org/D49755
Files:
include/lldb/Utility/History.h
lldb.xcodeproj/project.pbxproj
source/Utility/CMakeLists.txt
source/Utility/History.cpp
Index: source/Utility/History.cpp
===
--- source/Utility/History.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-//===-- History.cpp -*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "lldb/Utility/History.h"
-
-// C Includes
-#include
-// C++ Includes
-// Other libraries and framework includes
-// Project includes
-#include "lldb/Utility/Stream.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-void HistorySourceUInt::DumpHistoryEvent(Stream &strm, HistoryEvent event) {
- strm.Printf("%s %" PRIu64, m_name.c_str(), (uint64_t)((uintptr_t)event));
-}
Index: source/Utility/CMakeLists.txt
===
--- source/Utility/CMakeLists.txt
+++ source/Utility/CMakeLists.txt
@@ -53,7 +53,6 @@
Environment.cpp
FastDemangle.cpp
FileSpec.cpp
- History.cpp
IOObject.cpp
JSON.cpp
LLDBAssert.cpp
Index: lldb.xcodeproj/project.pbxproj
===
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/project.pbxproj
@@ -1880,8 +1880,6 @@
26A0DA4D140F721D006DA411 /* HashedNameToDIE.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = HashedNameToDIE.h; sourceTree = ""; };
2666ADC31B3CB675001FAFD3 /* HexagonDYLDRendezvous.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HexagonDYLDRendezvous.cpp; sourceTree = ""; };
2666ADC41B3CB675001FAFD3 /* HexagonDYLDRendezvous.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HexagonDYLDRendezvous.h; sourceTree = ""; };
- AFC2DCF21E6E30CF00283714 /* History.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = History.cpp; path = source/Utility/History.cpp; sourceTree = ""; };
- AFC2DCF41E6E30D800283714 /* History.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = History.h; path = include/lldb/Utility/History.h; sourceTree = ""; };
AF1729D4182C907200E0AB97 /* HistoryThread.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = HistoryThread.cpp; path = Utility/HistoryThread.cpp; sourceTree = ""; };
AF061F89182C98B6A19C /* HistoryThread.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = HistoryThread.h; path = Utility/HistoryThread.h; sourceTree = ""; };
AF1729D5182C907200E0AB97 /* HistoryUnwind.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = HistoryUnwind.cpp; path = Utility/HistoryUnwind.cpp; sourceTree = ""; };
Index: include/lldb/Utility/History.h
===
--- include/lldb/Utility/History.h
+++ /dev/null
@@ -1,136 +0,0 @@
-//===-- History.h ---*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef lldb_History_h_
-#define lldb_History_h_
-
-#include "lldb/lldb-defines.h" // for DISALLOW_COPY_AND_ASSIGN
-
-// C++ Includes
-#include
-#include
-#include
-
-#include // for size_t
-#include
-
-namespace lldb_private {
-class Stream;
-}
-
-namespace lldb_private {
-
-//--
-/// @class HistorySource History.h "lldb/Core/History.h"
-/// A class that defines history events.
-//--
-
-class HistorySource {
-public:
- typedef const void *HistoryEvent;
-
- HistorySource() : m_mutex(), m_events() {}
-
- virtual ~HistorySource() {}
-
- // Create a new history event. Subclasses should use any data or members in
- // the subclass of this class to produce a history event and push it onto the
- // end of the history stack.
-
- virtual HistoryEvent CreateHistoryEvent() = 0;
-
- virtual void DeleteHistoryEvent(HistoryEvent event) = 0;
-
- virtual void DumpHistoryEvent(Stream &strm, HistoryEvent event) = 0;
-
- virtual size_t GetHistoryEventCount() = 0;
-
- virtual HistoryEvent GetHistoryEventAtIn
[Lldb-commits] [PATCH] D49755: Remove unused History class
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Yes, please. https://reviews.llvm.org/D49755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337855 - Remove unused History class
Author: teemperor
Date: Tue Jul 24 14:09:17 2018
New Revision: 337855
URL: http://llvm.org/viewvc/llvm-project?rev=337855&view=rev
Log:
Remove unused History class
Summary: This class doesn't seem to be used anywhere, so we might as well
remove the code.
Reviewers: labath
Reviewed By: labath
Subscribers: labath, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D49755
Removed:
lldb/trunk/include/lldb/Utility/History.h
lldb/trunk/source/Utility/History.cpp
Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/source/Utility/CMakeLists.txt
Removed: lldb/trunk/include/lldb/Utility/History.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/History.h?rev=337854&view=auto
==
--- lldb/trunk/include/lldb/Utility/History.h (original)
+++ lldb/trunk/include/lldb/Utility/History.h (removed)
@@ -1,136 +0,0 @@
-//===-- History.h ---*- C++
-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef lldb_History_h_
-#define lldb_History_h_
-
-#include "lldb/lldb-defines.h" // for DISALLOW_COPY_AND_ASSIGN
-
-// C++ Includes
-#include
-#include
-#include
-
-#include // for size_t
-#include
-
-namespace lldb_private {
-class Stream;
-}
-
-namespace lldb_private {
-
-//--
-/// @class HistorySource History.h "lldb/Core/History.h"
-/// A class that defines history events.
-//--
-
-class HistorySource {
-public:
- typedef const void *HistoryEvent;
-
- HistorySource() : m_mutex(), m_events() {}
-
- virtual ~HistorySource() {}
-
- // Create a new history event. Subclasses should use any data or members in
- // the subclass of this class to produce a history event and push it onto the
- // end of the history stack.
-
- virtual HistoryEvent CreateHistoryEvent() = 0;
-
- virtual void DeleteHistoryEvent(HistoryEvent event) = 0;
-
- virtual void DumpHistoryEvent(Stream &strm, HistoryEvent event) = 0;
-
- virtual size_t GetHistoryEventCount() = 0;
-
- virtual HistoryEvent GetHistoryEventAtIndex(uint32_t idx) = 0;
-
- virtual HistoryEvent GetCurrentHistoryEvent() = 0;
-
- // Return 0 when lhs == rhs, 1 if lhs > rhs, or -1 if lhs < rhs.
- virtual int CompareHistoryEvents(const HistoryEvent lhs,
- const HistoryEvent rhs) = 0;
-
- virtual bool IsCurrentHistoryEvent(const HistoryEvent event) = 0;
-
-private:
- typedef std::stack collection;
-
- std::recursive_mutex m_mutex;
- collection m_events;
-
- DISALLOW_COPY_AND_ASSIGN(HistorySource);
-};
-
-//--
-/// @class HistorySourceUInt History.h "lldb/Core/History.h"
-/// A class that defines history events that are represented by
-/// unsigned integers.
-///
-/// Any history event that is defined by a unique monotonically increasing
-/// unsigned integer
-//--
-
-class HistorySourceUInt : public HistorySource {
- HistorySourceUInt(const char *id_name, uintptr_t start_value = 0u)
- : HistorySource(), m_name(id_name), m_curr_id(start_value) {}
-
- ~HistorySourceUInt() override {}
-
- // Create a new history event. Subclasses should use any data or members in
- // the subclass of this class to produce a history event and push it onto the
- // end of the history stack.
-
- HistoryEvent CreateHistoryEvent() override {
-++m_curr_id;
-return (HistoryEvent)m_curr_id;
- }
-
- void DeleteHistoryEvent(HistoryEvent event) override {
-// Nothing to delete, the event contains the integer
- }
-
- void DumpHistoryEvent(Stream &strm, HistoryEvent event) override;
-
- size_t GetHistoryEventCount() override { return m_curr_id; }
-
- HistoryEvent GetHistoryEventAtIndex(uint32_t idx) override {
-return (HistoryEvent)((uintptr_t)idx);
- }
-
- HistoryEvent GetCurrentHistoryEvent() override {
-return (HistoryEvent)m_curr_id;
- }
-
- // Return 0 when lhs == rhs, 1 if lhs > rhs, or -1 if lhs < rhs.
- int CompareHistoryEvents(const HistoryEvent lhs,
- const HistoryEvent rhs) override {
-uintptr_t lhs_uint = (uintptr_t)lhs;
-uintptr_t rhs_uint = (uintptr_t)rhs;
-if (lhs_uint < rhs_uint)
- return -1;
-if (lhs_uint > rhs_uint)
- return +1;
-return 0;
- }
-
- bool IsCurrentHistoryEvent(const HistoryEvent event) override {
-return (uintptr_t)event == m_curr_id;
- }
-
-protected:
- std::string m_name; // The name of the history unsigned integer
- uintptr_t m_curr_id; // The curr
[Lldb-commits] [PATCH] D49755: Remove unused History class
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337855: Remove unused History class (authored by teemperor,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49755?vs=157075&id=157128#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49755
Files:
lldb/trunk/include/lldb/Utility/History.h
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/source/Utility/CMakeLists.txt
lldb/trunk/source/Utility/History.cpp
Index: lldb/trunk/lldb.xcodeproj/project.pbxproj
===
--- lldb/trunk/lldb.xcodeproj/project.pbxproj
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj
@@ -1880,8 +1880,6 @@
26A0DA4D140F721D006DA411 /* HashedNameToDIE.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = HashedNameToDIE.h; sourceTree = ""; };
2666ADC31B3CB675001FAFD3 /* HexagonDYLDRendezvous.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HexagonDYLDRendezvous.cpp; sourceTree = ""; };
2666ADC41B3CB675001FAFD3 /* HexagonDYLDRendezvous.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HexagonDYLDRendezvous.h; sourceTree = ""; };
- AFC2DCF21E6E30CF00283714 /* History.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = History.cpp; path = source/Utility/History.cpp; sourceTree = ""; };
- AFC2DCF41E6E30D800283714 /* History.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = History.h; path = include/lldb/Utility/History.h; sourceTree = ""; };
AF1729D4182C907200E0AB97 /* HistoryThread.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = HistoryThread.cpp; path = Utility/HistoryThread.cpp; sourceTree = ""; };
AF061F89182C98B6A19C /* HistoryThread.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = HistoryThread.h; path = Utility/HistoryThread.h; sourceTree = ""; };
AF1729D5182C907200E0AB97 /* HistoryUnwind.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = HistoryUnwind.cpp; path = Utility/HistoryUnwind.cpp; sourceTree = ""; };
Index: lldb/trunk/source/Utility/CMakeLists.txt
===
--- lldb/trunk/source/Utility/CMakeLists.txt
+++ lldb/trunk/source/Utility/CMakeLists.txt
@@ -53,7 +53,6 @@
Environment.cpp
FastDemangle.cpp
FileSpec.cpp
- History.cpp
IOObject.cpp
JSON.cpp
LLDBAssert.cpp
Index: lldb/trunk/source/Utility/History.cpp
===
--- lldb/trunk/source/Utility/History.cpp
+++ lldb/trunk/source/Utility/History.cpp
@@ -1,24 +0,0 @@
-//===-- History.cpp -*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "lldb/Utility/History.h"
-
-// C Includes
-#include
-// C++ Includes
-// Other libraries and framework includes
-// Project includes
-#include "lldb/Utility/Stream.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-void HistorySourceUInt::DumpHistoryEvent(Stream &strm, HistoryEvent event) {
- strm.Printf("%s %" PRIu64, m_name.c_str(), (uint64_t)((uintptr_t)event));
-}
Index: lldb/trunk/include/lldb/Utility/History.h
===
--- lldb/trunk/include/lldb/Utility/History.h
+++ lldb/trunk/include/lldb/Utility/History.h
@@ -1,136 +0,0 @@
-//===-- History.h ---*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef lldb_History_h_
-#define lldb_History_h_
-
-#include "lldb/lldb-defines.h" // for DISALLOW_COPY_AND_ASSIGN
-
-// C++ Includes
-#include
-#include
-#include
-
-#include // for size_t
-#include
-
-namespace lldb_private {
-class Stream;
-}
-
-namespace lldb_private {
-
-//--
-/// @class HistorySource History.h "lldb/Core/History.h"
-/// A class that defines history events.
-//--
-
-class HistorySource {
-public:
- typedef const void *HistoryEvent;
-
- HistorySource() : m_mutex(), m_events() {}
-
- virtual ~HistorySource() {}
-
- // Create a new history event. Subclasses should use any data or members in
- // the subcl
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
EugeneBi updated this revision to Diff 157135.
EugeneBi marked 6 inline comments as done.
EugeneBi added a comment.
Code review followup:
- Restricted change to Platform.cpp
- Restricted change only to remote platforms.
https://reviews.llvm.org/D49685
Files:
source/Target/Platform.cpp
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -228,17 +228,33 @@
module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
did_create_ptr, false);
+ // Module resolver lambda.
+ auto resolver = [&](const ModuleSpec &spec) {
+Status error(eErrorTypeGeneric);
+// Check if we have sysroot set.
+if (m_sdk_sysroot) {
+ // Prepend sysroot to module spec.
+ auto resolved_spec(spec);
+ resolved_spec.GetFileSpec().PrependPathComponent(
+m_sdk_sysroot.GetStringRef());
+ // Try to get shared module with resolved spec.
+ error = ModuleList::GetSharedModule(
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
+did_create_ptr, false);
+}
+// If we don't have sysroot or it didn't work then
+// try original module spec.
+if (!error.Success())
+ error = ModuleList::GetSharedModule(
+spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
+did_create_ptr, false);
+if (error.Success() && module_sp)
+ module_sp->SetPlatformFileSpec(spec.GetFileSpec());
+return error;
+ };
+
return GetRemoteSharedModule(module_spec, process, module_sp,
- [&](const ModuleSpec &spec) {
- Status error = ModuleList::GetSharedModule(
- spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
- if (error.Success() && module_sp)
- module_sp->SetPlatformFileSpec(
- spec.GetFileSpec());
- return error;
- },
- did_create_ptr);
+ resolver, did_create_ptr);
}
bool Platform::GetModuleSpec(const FileSpec &module_file_spec,
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -228,17 +228,33 @@
module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
did_create_ptr, false);
+ // Module resolver lambda.
+ auto resolver = [&](const ModuleSpec &spec) {
+Status error(eErrorTypeGeneric);
+// Check if we have sysroot set.
+if (m_sdk_sysroot) {
+ // Prepend sysroot to module spec.
+ auto resolved_spec(spec);
+ resolved_spec.GetFileSpec().PrependPathComponent(
+m_sdk_sysroot.GetStringRef());
+ // Try to get shared module with resolved spec.
+ error = ModuleList::GetSharedModule(
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
+did_create_ptr, false);
+}
+// If we don't have sysroot or it didn't work then
+// try original module spec.
+if (!error.Success())
+ error = ModuleList::GetSharedModule(
+spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
+did_create_ptr, false);
+if (error.Success() && module_sp)
+ module_sp->SetPlatformFileSpec(spec.GetFileSpec());
+return error;
+ };
+
return GetRemoteSharedModule(module_spec, process, module_sp,
- [&](const ModuleSpec &spec) {
- Status error = ModuleList::GetSharedModule(
- spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
- if (error.Success() && module_sp)
- module_sp->SetPlatformFileSpec(
- spec.GetFileSpec());
- return error;
- },
- did_create_ptr);
+ resolver, did_create_ptr);
}
bool Platform::GetModuleSpec(const FileSpec &module_file_spec,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support
teemperor planned changes to this revision. teemperor added a comment. Actually, we also have to tear down the plugins, so let me fix that first. https://reviews.llvm.org/D49334 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. This will help, but not fix us loading incorrect versions of the shared library. I wonder if there is anything in the core file that could help use get the build ID/UUID of each binary. I doubt it, but might be worth checking. Linux core files are really useless unless they can be processed with the exact binaries and that is a shame. The windows minidump files that breakpad can create are much better than standard core files since they do contain the information we need (path + version + UUID) and they are very similar to core files but just don't always save the same amount of memory (just the thread stacks, not all mapped memory), though that can easily be fixed. https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.
lemo added a comment.
Looks really good, a few comments inline.
This may not big a big deal, but the part about FP (and Apple vs. non-Apple) is
confusing: the FP is a pretty weak convention, and in some ABIs is not actually
"fixed" (ex. FP can be either https://reviews.llvm.org/source/openmp/ or
https://reviews.llvm.org/source/libunwind/, which in turn can be used as GPRs).
If Apple sticks to a strict usage it makes sense to name it but then maybe we
should just not name any FP for non-Apple?
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:149-150
ArchSpec MinidumpParser::GetArchitecture() {
- ArchSpec arch_spec;
+ if (m_arch.IsValid())
+return m_arch;
const MinidumpSystemInfo *system_info = GetSystemInfo();
instead of useing m_arch as a cache I'd explicitly initialize it in
MinidumpParser::Initialize()
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:212-216
+ std::string csd_version;
+ if (auto s = GetMinidumpString(system_info->csd_version_rva))
+csd_version = *s;
+ if (csd_version.find("Linux") != std::string::npos)
+triple.setOS(llvm::Triple::OSType::Linux);
why is this needed when we have MinidumpOSPlatform::Linux ?
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:179
+auto platform_sp = target.GetPlatform();
+if (platform_sp && !platform_sp->IsCompatibleArchitecture(arch, false,
nullptr)) {
+ ArchSpec platform_arch;
shouldn't this be a check resulting in an error? why do we need to make this
implicit adjustment here?
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:192
+
+static size_t k_num_reg_infos = llvm::array_lengthof(g_reg_infos);
+
constexpr?
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:195
+// ARM general purpose registers.
+const uint32_t g_gpr_regnums[] = {
+ reg_r0, reg_r1, reg_r2, reg_r3, reg_r4, reg_r5, reg_r6, reg_r7,
use std::array for these kind of static arrays? (debug bounds checks, easy
access to the static size, ...)
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:225
+ m_regs.context_flags = data.GetU32(&offset);
+ for (unsigned i=0; i<16; ++i)
+m_regs.r[i] = data.GetU32(&offset);
symbolic constants or use the array size?
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:231
+m_regs.d[i] = data.GetU64(&offset);
+ assert(k_num_regs == k_num_reg_infos_apple);
+ assert(k_num_regs == k_num_reg_infos);
lldb_assert ?
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:242
+}
+size_t RegisterContextMinidump_ARM::GetRegisterCount() {
+ return k_num_regs;
not a big deal, but this kind of accessors for constants can be constexpr
themselves
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:248
+RegisterContextMinidump_ARM::GetRegisterInfoAtIndex(size_t reg) {
+ if (reg < k_num_reg_infos)
+return &m_reg_infos[reg];
failfast if out of bounds? who'd pass an invalid index and expect a meaninful
result?
(btw, std::array would provide the debug checks if that's all that we want)
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:281
+bool RegisterContextMinidump_ARM::WriteRegister(
+const RegisterInfo *reg_info, const RegisterValue ®_value) {
+ return false;
remove unused parameter name or [[maybe_unused]]
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h:82
+Context m_regs;
+RegisterInfo *m_reg_infos;
+size_t m_num_reg_infos;
= nullptr;
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h:83
+RegisterInfo *m_reg_infos;
+size_t m_num_reg_infos;
+};
= 0;
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h:37
+
+ ~RegisterContextMinidump_ARM64() override {}
+
=default instead of {} ?
https://reviews.llvm.org/D49750
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
EugeneBi added inline comments. Comment at: source/Target/Platform.cpp:252 +if (error.Success() && module_sp) + module_sp->SetPlatformFileSpec(spec.GetFileSpec()); +return error; A bug here. must be resolved_spec if it succeeds. https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.
clayborg added a comment.
I will make update the patch with many of the suggested inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:150
+ if (m_arch.IsValid())
+return m_arch;
const MinidumpSystemInfo *system_info = GetSystemInfo();
Can do
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:216
+ if (csd_version.find("Linux") != std::string::npos)
+triple.setOS(llvm::Triple::OSType::Linux);
+ break;
I have run into some minidump files that don't have linux set corrreclty as the
OS, but they do have "linux" in the description. So this is to handle those
cases. I have told the people that are generating these about the issue and
they will fix it, but we have minidump files out in the wild that don't set
linux as the OS correctly.
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:179
+auto platform_sp = target.GetPlatform();
+if (platform_sp && !platform_sp->IsCompatibleArchitecture(arch, false,
nullptr)) {
+ ArchSpec platform_arch;
lemo wrote:
> shouldn't this be a check resulting in an error? why do we need to make this
> implicit adjustment here?
By default the "host" platform is selected and it was incorrectly being used
along with _any_ triple. So we need to adjust the platform if the host platform
isn't compatible. The platform being set correctly helps with many things
during a debug session like finding symbols and much more.
https://reviews.llvm.org/D49750
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
EugeneBi updated this revision to Diff 157146.
EugeneBi added a comment.
Fix a bug with resolved_spec path.
https://reviews.llvm.org/D49685
Files:
source/Target/Platform.cpp
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -228,17 +228,36 @@
module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
did_create_ptr, false);
+ // Module resolver lambda.
+ auto resolver = [&](const ModuleSpec &spec) {
+Status error(eErrorTypeGeneric);
+ModuleSpec resolved_spec;
+// Check if we have sysroot set.
+if (m_sdk_sysroot) {
+ // Prepend sysroot to module spec.
+ resolved_spec = spec;
+ resolved_spec.GetFileSpec().PrependPathComponent(
+m_sdk_sysroot.GetStringRef());
+ // Try to get shared module with resolved spec.
+ error = ModuleList::GetSharedModule(
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
+did_create_ptr, false);
+}
+// If we don't have sysroot or it didn't work then
+// try original module spec.
+if (!error.Success()) {
+ resolved_spec = spec;
+ error = ModuleList::GetSharedModule(
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
+did_create_ptr, false);
+}
+if (error.Success() && module_sp)
+ module_sp->SetPlatformFileSpec(resolved_spec.GetFileSpec());
+return error;
+ };
+
return GetRemoteSharedModule(module_spec, process, module_sp,
- [&](const ModuleSpec &spec) {
- Status error = ModuleList::GetSharedModule(
- spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
- if (error.Success() && module_sp)
- module_sp->SetPlatformFileSpec(
- spec.GetFileSpec());
- return error;
- },
- did_create_ptr);
+ resolver, did_create_ptr);
}
bool Platform::GetModuleSpec(const FileSpec &module_file_spec,
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -228,17 +228,36 @@
module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
did_create_ptr, false);
+ // Module resolver lambda.
+ auto resolver = [&](const ModuleSpec &spec) {
+Status error(eErrorTypeGeneric);
+ModuleSpec resolved_spec;
+// Check if we have sysroot set.
+if (m_sdk_sysroot) {
+ // Prepend sysroot to module spec.
+ resolved_spec = spec;
+ resolved_spec.GetFileSpec().PrependPathComponent(
+m_sdk_sysroot.GetStringRef());
+ // Try to get shared module with resolved spec.
+ error = ModuleList::GetSharedModule(
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
+did_create_ptr, false);
+}
+// If we don't have sysroot or it didn't work then
+// try original module spec.
+if (!error.Success()) {
+ resolved_spec = spec;
+ error = ModuleList::GetSharedModule(
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
+did_create_ptr, false);
+}
+if (error.Success() && module_sp)
+ module_sp->SetPlatformFileSpec(resolved_spec.GetFileSpec());
+return error;
+ };
+
return GetRemoteSharedModule(module_spec, process, module_sp,
- [&](const ModuleSpec &spec) {
- Status error = ModuleList::GetSharedModule(
- spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
- if (error.Success() && module_sp)
- module_sp->SetPlatformFileSpec(
- spec.GetFileSpec());
- return error;
- },
- did_create_ptr);
+ resolver, did_create_ptr);
}
bool Platform::GetModuleSpec(const FileSpec &module_file_spec,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux
EugeneBi added inline comments. Comment at: source/Target/Platform.cpp:252 +if (error.Success() && module_sp) + module_sp->SetPlatformFileSpec(spec.GetFileSpec()); +return error; EugeneBi wrote: > A bug here. must be resolved_spec if it succeeds. Now I have my doubts: what "platform file spec" really means? I mean, which code is correct: current or previous one? https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.
lemo added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:216
+ if (csd_version.find("Linux") != std::string::npos)
+triple.setOS(llvm::Triple::OSType::Linux);
+ break;
clayborg wrote:
> I have run into some minidump files that don't have linux set corrreclty as
> the OS, but they do have "linux" in the description. So this is to handle
> those cases. I have told the people that are generating these about the issue
> and they will fix it, but we have minidump files out in the wild that don't
> set linux as the OS correctly.
1. any idea where this minidumps originate from?
2. should we raise some kind of signal when we go down this path? print an
warning or at least verbose logging?
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:179
+auto platform_sp = target.GetPlatform();
+if (platform_sp && !platform_sp->IsCompatibleArchitecture(arch, false,
nullptr)) {
+ ArchSpec platform_arch;
clayborg wrote:
> lemo wrote:
> > shouldn't this be a check resulting in an error? why do we need to make
> > this implicit adjustment here?
> By default the "host" platform is selected and it was incorrectly being used
> along with _any_ triple. So we need to adjust the platform if the host
> platform isn't compatible. The platform being set correctly helps with many
> things during a debug session like finding symbols and much more.
Nice catch/fix in that case!
Just curious: It still seems a bit surprising to me to have the target mutated
by the process object - is that how it's normally done in other cases?
https://reviews.llvm.org/D49750
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337865 - Add DumpRegisterValue.cpp.
Author: jmolenda
Date: Tue Jul 24 16:19:56 2018
New Revision: 337865
URL: http://llvm.org/viewvc/llvm-project?rev=337865&view=rev
Log:
Add DumpRegisterValue.cpp.
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=337865&r1=337864&r2=337865&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Jul 24 16:19:56 2018
@@ -264,6 +264,7 @@
2579065F1BD0488D00178368 /* DomainSocket.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 2579065E1BD0488D00178368 /* DomainSocket.cpp */;
};
26F5C27710F3D9E4009D5894 /* Driver.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = 26F5C27310F3D9E4009D5894 /* Driver.cpp */; };
4C4EB7811E6A4DCC002035C0 /* DumpDataExtractor.cpp in Sources */
= {isa = PBXBuildFile; fileRef = 4C4EB77F1E6A4DB8002035C0 /*
DumpDataExtractor.cpp */; };
+ AFA585D02107EB7400D7689A /* DumpRegisterValue.cpp in Sources */
= {isa = PBXBuildFile; fileRef = AFA585CF2107EB7300D7689A /*
DumpRegisterValue.cpp */; };
9447DE431BD5963300E67212 /* DumpValueObjectOptions.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 9447DE421BD5963300E67212 /*
DumpValueObjectOptions.cpp */; };
268900EA13353E6F00698AC0 /* DynamicLoader.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 26BC7E7710F1B85900F91463 /* DynamicLoader.cpp
*/; };
AF27AD551D3603EA00CF2833 /* DynamicLoaderDarwin.cpp in Sources
*/ = {isa = PBXBuildFile; fileRef = AF27AD531D3603EA00CF2833 /*
DynamicLoaderDarwin.cpp */; };
@@ -335,7 +336,6 @@
AE44FB301BB07EB20033EB62 /* GoUserExpression.cpp in Sources */
= {isa = PBXBuildFile; fileRef = AE44FB2C1BB07DD80033EB62 /*
GoUserExpression.cpp */; };
6D95DC011B9DC057000E318A /* HashedNameToDIE.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 6D95DBFE1B9DC057000E318A /* HashedNameToDIE.cpp
*/; };
2666ADC81B3CB675001FAFD3 /* HexagonDYLDRendezvous.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 2666ADC31B3CB675001FAFD3 /*
HexagonDYLDRendezvous.cpp */; };
- AFC2DCF31E6E30CF00283714 /* History.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = AFC2DCF21E6E30CF00283714 /* History.cpp */; };
AF1729D6182C907200E0AB97 /* HistoryThread.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = AF1729D4182C907200E0AB97 /* HistoryThread.cpp
*/; };
AF1729D7182C907200E0AB97 /* HistoryUnwind.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = AF1729D5182C907200E0AB97 /* HistoryUnwind.cpp
*/; };
2689007113353E1A00698AC0 /* Host.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = 69A01E1C1236C5D400C660B5 /* Host.cpp */; };
@@ -1735,6 +1735,7 @@
26F5C27410F3D9E4009D5894 /* Driver.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
Driver.h; path = tools/driver/Driver.h; sourceTree = ""; };
4C4EB77F1E6A4DB8002035C0 /* DumpDataExtractor.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
name = DumpDataExtractor.cpp; path = source/Core/DumpDataExtractor.cpp;
sourceTree = ""; };
4C4EB7821E6A4DE7002035C0 /* DumpDataExtractor.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
DumpDataExtractor.h; path = include/lldb/Core/DumpDataExtractor.h; sourceTree =
""; };
+ AFA585CF2107EB7300D7689A /* DumpRegisterValue.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
name = DumpRegisterValue.cpp; path = source/Core/DumpRegisterValue.cpp;
sourceTree = ""; };
9447DE421BD5963300E67212 /* DumpValueObjectOptions.cpp */ =
{isa = PBXFileReference; fileEncoding = 4; lastKnownFileType =
sourcecode.cpp.cpp; name = DumpValueObjectOptions.cpp; path =
source/DataFormatters/DumpValueObjectOptions.cpp; sourceTree = ""; };
9447DE411BD5962900E67212 /* DumpValueObjectOptions.h */ = {isa
= PBXFileReference; lastKnownFileType = sourcecode.c.h; name =
DumpValueObjectOptions.h; path =
include/lldb/DataFormatters/DumpValueObjectOptions.h; sourceTree = ""; };
26BC7E7710F1B85900F91463 /* DynamicLoader.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
name = DynamicLoader.cpp; path = source/Core/DynamicLoader.cpp; sourceTree =
""; };
@@ -4484,8 +4485,6 @@
AFC2DCE61E6E2ED000283714 /* FastDemangle.cpp */,
AFC2DCED1E6E2F9800283714 /* FastDemangle.h */,
4CBFF0471F579A36004AFA92 /* Flags.h */,
- AFC2DCF21E6E30C
[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support
teemperor updated this revision to Diff 157169.
teemperor added a reviewer: davide.
teemperor added a comment.
- Removed some leftover code from the refactoring.
- Correctly SetUp/TearDown the test case.
https://reviews.llvm.org/D49334
Files:
include/lldb/Core/Debugger.h
include/lldb/Core/Highlighter.h
include/lldb/Target/Language.h
packages/Python/lldbsuite/test/source-manager/TestSourceManager.py
source/Core/CMakeLists.txt
source/Core/Debugger.cpp
source/Core/Highlighter.cpp
source/Core/SourceManager.cpp
source/Plugins/Language/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
source/Plugins/Language/ClangCommon/CMakeLists.txt
source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
source/Plugins/Language/ClangCommon/ClangHighlighter.h
source/Plugins/Language/Go/GoLanguage.cpp
source/Plugins/Language/Go/GoLanguage.h
source/Plugins/Language/Java/JavaLanguage.cpp
source/Plugins/Language/Java/JavaLanguage.h
source/Plugins/Language/OCaml/OCamlLanguage.cpp
source/Plugins/Language/OCaml/OCamlLanguage.h
source/Plugins/Language/ObjC/CMakeLists.txt
source/Plugins/Language/ObjC/ObjCLanguage.cpp
source/Plugins/Language/ObjC/ObjCLanguage.h
source/Plugins/Language/ObjCPlusPlus/CMakeLists.txt
source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp
source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
source/Target/Language.cpp
unittests/Language/CMakeLists.txt
unittests/Language/Highlighting/CMakeLists.txt
unittests/Language/Highlighting/HighlighterTest.cpp
Index: unittests/Language/Highlighting/HighlighterTest.cpp
===
--- /dev/null
+++ unittests/Language/Highlighting/HighlighterTest.cpp
@@ -0,0 +1,233 @@
+//===-- HighlighterTest.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/Core/Highlighter.h"
+
+#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/Language/Go/GoLanguage.h"
+#include "Plugins/Language/Java/JavaLanguage.h"
+#include "Plugins/Language/OCaml/OCamlLanguage.h"
+#include "Plugins/Language/ObjC/ObjCLanguage.h"
+#include "Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h"
+
+using namespace lldb_private;
+
+namespace {
+class HighlighterTest : public testing::Test {
+public:
+ static void SetUpTestCase();
+ static void TearDownTestCase();
+};
+} // namespace
+
+void HighlighterTest::SetUpTestCase() {
+ // The HighlighterManager uses the language plugins under the hood, so we
+ // have to initialize them here for our test process.
+ CPlusPlusLanguage::Initialize();
+ GoLanguage::Initialize();
+ JavaLanguage::Initialize();
+ ObjCLanguage::Initialize();
+ ObjCPlusPlusLanguage::Initialize();
+ OCamlLanguage::Initialize();
+}
+
+void HighlighterTest::TearDownTestCase() {
+ CPlusPlusLanguage::Terminate();
+ GoLanguage::Terminate();
+ JavaLanguage::Terminate();
+ ObjCLanguage::Terminate();
+ ObjCPlusPlusLanguage::Terminate();
+ OCamlLanguage::Terminate();
+}
+
+static std::string getName(lldb::LanguageType type) {
+ HighlighterManager m;
+ return m.getHighlighterFor(type, "").GetName().str();
+}
+
+static std::string getName(llvm::StringRef path) {
+ HighlighterManager m;
+ return m.getHighlighterFor(lldb::eLanguageTypeUnknown, path).GetName().str();
+}
+
+TEST_F(HighlighterTest, HighlighterSelectionType) {
+ EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus), "clang");
+ EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_03), "clang");
+ EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_11), "clang");
+ EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_14), "clang");
+ EXPECT_EQ(getName(lldb::eLanguageTypeObjC), "clang");
+ EXPECT_EQ(getName(lldb::eLanguageTypeObjC_plus_plus), "clang");
+
+ EXPECT_EQ(getName(lldb::eLanguageTypeUnknown), "none");
+ EXPECT_EQ(getName(lldb::eLanguageTypeJulia), "none");
+ EXPECT_EQ(getName(lldb::eLanguageTypeJava), "none");
+ EXPECT_EQ(getName(lldb::eLanguageTypeHaskell), "none");
+}
+
+TEST_F(HighlighterTest, HighlighterSelectionPath) {
+ EXPECT_EQ(getName("myfile.cc"), "clang");
+ EXPECT_EQ(getName("moo.cpp"), "clang");
+ EXPECT_EQ(getName("mar.cxx"), "clang");
+ EXPECT_EQ(getName("foo.C"), "clang");
+ EXPECT_EQ(getName("bar.CC"), "clang");
+ EXPECT_EQ(getName("a/dir.CC"), "clang");
+ EXPECT_EQ(getName("/a/dir.hpp"), "clang");
+ EXPECT_EQ(getName("header.h"), "clang");
+
+ EXPECT_EQ(getName(""), "none");
+ EXPECT_EQ(getName("/dev/null"), "none");
+ EXPECT_EQ(getName("Factory.java"), "none");
+ EXPECT_EQ(getName("poll.py"), "none");
+ EXP
[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange
teemperor updated this revision to Diff 157172.
teemperor added a comment.
- Applied Greg's requested changes (thank you very much).
https://reviews.llvm.org/D49415
Files:
unittests/Utility/CMakeLists.txt
unittests/Utility/VMRangeTest.cpp
Index: unittests/Utility/VMRangeTest.cpp
===
--- /dev/null
+++ unittests/Utility/VMRangeTest.cpp
@@ -0,0 +1,152 @@
+//===-- VMRangeTest.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
+
+#include "lldb/Utility/VMRange.h"
+
+using namespace lldb_private;
+
+namespace lldb_private {
+void PrintTo(const VMRange &v, std::ostream *os) {
+ (*os) << "VMRange(" << v.GetBaseAddress() << ", " << v.GetEndAddress() << ")";
+}
+} // namespace lldb_private
+
+TEST(VMRange, IsValid) {
+ VMRange range;
+ EXPECT_FALSE(range.IsValid());
+
+ range.Reset(0x1, 0x100);
+ EXPECT_TRUE(range.IsValid());
+
+ range.Reset(0x1, 0x1);
+ EXPECT_FALSE(range.IsValid());
+}
+
+TEST(VMRange, Clear) {
+ VMRange range(0x100, 0x200);
+ EXPECT_NE(VMRange(), range);
+ range.Clear();
+ EXPECT_EQ(VMRange(), range);
+}
+
+TEST(VMRange, Comparison) {
+ VMRange range1(0x100, 0x200);
+ VMRange range2(0x100, 0x200);
+ EXPECT_EQ(range1, range2);
+
+ EXPECT_NE(VMRange(0x100, 0x1ff), range1);
+ EXPECT_NE(VMRange(0x100, 0x201), range1);
+ EXPECT_NE(VMRange(0x0ff, 0x200), range1);
+ EXPECT_NE(VMRange(0x101, 0x200), range1);
+
+ range2.Clear();
+ EXPECT_NE(range1, range2);
+}
+
+TEST(VMRange, Reset) {
+ VMRange range(0x100, 0x200);
+ EXPECT_FALSE(VMRange(0x200, 0x200) == range);
+ range.Reset(0x200, 0x200);
+ EXPECT_TRUE(VMRange(0x200, 0x200) == range);
+}
+
+TEST(VMRange, SetEndAddress) {
+ VMRange range(0x100, 0x200);
+
+ range.SetEndAddress(0xFF);
+ EXPECT_EQ(0U, range.GetByteSize());
+ EXPECT_FALSE(range.IsValid());
+
+ range.SetEndAddress(0x101);
+ EXPECT_EQ(1U, range.GetByteSize());
+ EXPECT_TRUE(range.IsValid());
+}
+
+TEST(VMRange, ContainsAddr) {
+ VMRange range(0x100, 0x200);
+
+ EXPECT_FALSE(range.Contains(0x00));
+ EXPECT_FALSE(range.Contains(0xFF));
+ EXPECT_TRUE(range.Contains(0x100));
+ EXPECT_TRUE(range.Contains(0x101));
+ EXPECT_TRUE(range.Contains(0x1FF));
+ EXPECT_FALSE(range.Contains(0x200));
+ EXPECT_FALSE(range.Contains(0x201));
+ EXPECT_FALSE(range.Contains(0xFFF));
+ EXPECT_FALSE(range.Contains(std::numeric_limits::max()));
+}
+
+TEST(VMRange, ContainsRange) {
+ VMRange range(0x100, 0x200);
+
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x100)));
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x101)));
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x105)));
+ EXPECT_TRUE(range.Contains(VMRange(0x101, 0x105)));
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x1FF)));
+ EXPECT_TRUE(range.Contains(VMRange(0x105, 0x200)));
+ EXPECT_FALSE(range.Contains(VMRange(0x105, 0x201)));
+ EXPECT_FALSE(range.Contains(VMRange(0x200, 0x201)));
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x200)));
+ EXPECT_FALSE(
+ range.Contains(VMRange(0x105, std::numeric_limits::max(;
+
+ // Empty range.
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x100)));
+
+ range.Clear();
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+}
+
+TEST(VMRange, Ordering) {
+ VMRange range1(0x44, 0x200);
+ VMRange range2(0x100, 0x1FF);
+ VMRange range3(0x100, 0x200);
+
+ EXPECT_LE(range1, range1);
+ EXPECT_GE(range1, range1);
+
+ EXPECT_LT(range1, range2);
+ EXPECT_LT(range2, range3);
+
+ EXPECT_GT(range2, range1);
+ EXPECT_GT(range3, range2);
+
+ // Ensure that < and > are always false when comparing ranges with themselves.
+ EXPECT_FALSE(range1 < range1);
+ EXPECT_FALSE(range2 < range2);
+ EXPECT_FALSE(range3 < range3);
+
+ EXPECT_FALSE(range1 > range1);
+ EXPECT_FALSE(range2 > range2);
+ EXPECT_FALSE(range3 > range3);
+}
+
+TEST(VMRange, CollectionContains) {
+ VMRange::collection collection = {VMRange(0x100, 0x105),
+VMRange(0x108, 0x110)};
+
+ EXPECT_FALSE(VMRange::ContainsValue(collection, 0xFF));
+ EXPECT_TRUE(VMRange::ContainsValue(collection, 0x100));
+ EXPECT_FALSE(VMRange::ContainsValue(collection, 0x105));
+ EXPECT_TRUE(VMRange::ContainsValue(collection, 0x109));
+
+ EXPECT_TRUE(VMRange::ContainsRange(collection, VMRange(0x100, 0x104)));
+ EXPECT_TRUE(VMRange::ContainsRange(collection, VMRange(0x108, 0x100)));
+ EXPECT_FALSE(VMRange::ContainsRange(collection, VMRange(0xFF, 0x100)));
+
+ // TODO: Implement and test ContainsRange with values that span multiple
+ // ranges in the collection.
+}
Index: unittests/Utility/CMakeLists.txt
=
[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange
teemperor added a comment. Merging this in as Davide suggested. https://reviews.llvm.org/D49415 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337873: Add unit tests for VMRange (authored by teemperor,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49415?vs=157172&id=157173#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49415
Files:
lldb/trunk/unittests/Utility/CMakeLists.txt
lldb/trunk/unittests/Utility/VMRangeTest.cpp
Index: lldb/trunk/unittests/Utility/VMRangeTest.cpp
===
--- lldb/trunk/unittests/Utility/VMRangeTest.cpp
+++ lldb/trunk/unittests/Utility/VMRangeTest.cpp
@@ -0,0 +1,152 @@
+//===-- VMRangeTest.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
+
+#include "lldb/Utility/VMRange.h"
+
+using namespace lldb_private;
+
+namespace lldb_private {
+void PrintTo(const VMRange &v, std::ostream *os) {
+ (*os) << "VMRange(" << v.GetBaseAddress() << ", " << v.GetEndAddress() << ")";
+}
+} // namespace lldb_private
+
+TEST(VMRange, IsValid) {
+ VMRange range;
+ EXPECT_FALSE(range.IsValid());
+
+ range.Reset(0x1, 0x100);
+ EXPECT_TRUE(range.IsValid());
+
+ range.Reset(0x1, 0x1);
+ EXPECT_FALSE(range.IsValid());
+}
+
+TEST(VMRange, Clear) {
+ VMRange range(0x100, 0x200);
+ EXPECT_NE(VMRange(), range);
+ range.Clear();
+ EXPECT_EQ(VMRange(), range);
+}
+
+TEST(VMRange, Comparison) {
+ VMRange range1(0x100, 0x200);
+ VMRange range2(0x100, 0x200);
+ EXPECT_EQ(range1, range2);
+
+ EXPECT_NE(VMRange(0x100, 0x1ff), range1);
+ EXPECT_NE(VMRange(0x100, 0x201), range1);
+ EXPECT_NE(VMRange(0x0ff, 0x200), range1);
+ EXPECT_NE(VMRange(0x101, 0x200), range1);
+
+ range2.Clear();
+ EXPECT_NE(range1, range2);
+}
+
+TEST(VMRange, Reset) {
+ VMRange range(0x100, 0x200);
+ EXPECT_FALSE(VMRange(0x200, 0x200) == range);
+ range.Reset(0x200, 0x200);
+ EXPECT_TRUE(VMRange(0x200, 0x200) == range);
+}
+
+TEST(VMRange, SetEndAddress) {
+ VMRange range(0x100, 0x200);
+
+ range.SetEndAddress(0xFF);
+ EXPECT_EQ(0U, range.GetByteSize());
+ EXPECT_FALSE(range.IsValid());
+
+ range.SetEndAddress(0x101);
+ EXPECT_EQ(1U, range.GetByteSize());
+ EXPECT_TRUE(range.IsValid());
+}
+
+TEST(VMRange, ContainsAddr) {
+ VMRange range(0x100, 0x200);
+
+ EXPECT_FALSE(range.Contains(0x00));
+ EXPECT_FALSE(range.Contains(0xFF));
+ EXPECT_TRUE(range.Contains(0x100));
+ EXPECT_TRUE(range.Contains(0x101));
+ EXPECT_TRUE(range.Contains(0x1FF));
+ EXPECT_FALSE(range.Contains(0x200));
+ EXPECT_FALSE(range.Contains(0x201));
+ EXPECT_FALSE(range.Contains(0xFFF));
+ EXPECT_FALSE(range.Contains(std::numeric_limits::max()));
+}
+
+TEST(VMRange, ContainsRange) {
+ VMRange range(0x100, 0x200);
+
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x100)));
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x101)));
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x105)));
+ EXPECT_TRUE(range.Contains(VMRange(0x101, 0x105)));
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x1FF)));
+ EXPECT_TRUE(range.Contains(VMRange(0x105, 0x200)));
+ EXPECT_FALSE(range.Contains(VMRange(0x105, 0x201)));
+ EXPECT_FALSE(range.Contains(VMRange(0x200, 0x201)));
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x200)));
+ EXPECT_FALSE(
+ range.Contains(VMRange(0x105, std::numeric_limits::max(;
+
+ // Empty range.
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x100)));
+
+ range.Clear();
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+}
+
+TEST(VMRange, Ordering) {
+ VMRange range1(0x44, 0x200);
+ VMRange range2(0x100, 0x1FF);
+ VMRange range3(0x100, 0x200);
+
+ EXPECT_LE(range1, range1);
+ EXPECT_GE(range1, range1);
+
+ EXPECT_LT(range1, range2);
+ EXPECT_LT(range2, range3);
+
+ EXPECT_GT(range2, range1);
+ EXPECT_GT(range3, range2);
+
+ // Ensure that < and > are always false when comparing ranges with themselves.
+ EXPECT_FALSE(range1 < range1);
+ EXPECT_FALSE(range2 < range2);
+ EXPECT_FALSE(range3 < range3);
+
+ EXPECT_FALSE(range1 > range1);
+ EXPECT_FALSE(range2 > range2);
+ EXPECT_FALSE(range3 > range3);
+}
+
+TEST(VMRange, CollectionContains) {
+ VMRange::collection collection = {VMRange(0x100, 0x105),
+VMRange(0x108, 0x110)};
+
+ EXPECT_FALSE(VMRange::ContainsValue(collection, 0xFF));
+ EXPECT_TRUE(VMRange::ContainsValue(collection, 0x100));
+ EXPECT_FALSE(VMRange::ContainsValue(collection, 0x105));
+ EXPECT_TRUE(VMRange::ContainsValue(collection, 0x109));
+
+ EXPECT_TRUE(VMRange::ContainsRange(collection, VMRange(0x100, 0x104)));
+ EXPECT_TR
[Lldb-commits] [lldb] r337873 - Add unit tests for VMRange
Author: teemperor
Date: Tue Jul 24 16:52:39 2018
New Revision: 337873
URL: http://llvm.org/viewvc/llvm-project?rev=337873&view=rev
Log:
Add unit tests for VMRange
Subscribers: clayborg, labath, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D49415
Added:
lldb/trunk/unittests/Utility/VMRangeTest.cpp
Modified:
lldb/trunk/unittests/Utility/CMakeLists.txt
Modified: lldb/trunk/unittests/Utility/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CMakeLists.txt?rev=337873&r1=337872&r2=337873&view=diff
==
--- lldb/trunk/unittests/Utility/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Utility/CMakeLists.txt Tue Jul 24 16:52:39 2018
@@ -22,6 +22,7 @@ add_lldb_unittest(UtilityTests
UriParserTest.cpp
UUIDTest.cpp
VASprintfTest.cpp
+ VMRangeTest.cpp
LINK_LIBS
lldbUtility
Added: lldb/trunk/unittests/Utility/VMRangeTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/VMRangeTest.cpp?rev=337873&view=auto
==
--- lldb/trunk/unittests/Utility/VMRangeTest.cpp (added)
+++ lldb/trunk/unittests/Utility/VMRangeTest.cpp Tue Jul 24 16:52:39 2018
@@ -0,0 +1,152 @@
+//===-- VMRangeTest.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
+
+#include "lldb/Utility/VMRange.h"
+
+using namespace lldb_private;
+
+namespace lldb_private {
+void PrintTo(const VMRange &v, std::ostream *os) {
+ (*os) << "VMRange(" << v.GetBaseAddress() << ", " << v.GetEndAddress() <<
")";
+}
+} // namespace lldb_private
+
+TEST(VMRange, IsValid) {
+ VMRange range;
+ EXPECT_FALSE(range.IsValid());
+
+ range.Reset(0x1, 0x100);
+ EXPECT_TRUE(range.IsValid());
+
+ range.Reset(0x1, 0x1);
+ EXPECT_FALSE(range.IsValid());
+}
+
+TEST(VMRange, Clear) {
+ VMRange range(0x100, 0x200);
+ EXPECT_NE(VMRange(), range);
+ range.Clear();
+ EXPECT_EQ(VMRange(), range);
+}
+
+TEST(VMRange, Comparison) {
+ VMRange range1(0x100, 0x200);
+ VMRange range2(0x100, 0x200);
+ EXPECT_EQ(range1, range2);
+
+ EXPECT_NE(VMRange(0x100, 0x1ff), range1);
+ EXPECT_NE(VMRange(0x100, 0x201), range1);
+ EXPECT_NE(VMRange(0x0ff, 0x200), range1);
+ EXPECT_NE(VMRange(0x101, 0x200), range1);
+
+ range2.Clear();
+ EXPECT_NE(range1, range2);
+}
+
+TEST(VMRange, Reset) {
+ VMRange range(0x100, 0x200);
+ EXPECT_FALSE(VMRange(0x200, 0x200) == range);
+ range.Reset(0x200, 0x200);
+ EXPECT_TRUE(VMRange(0x200, 0x200) == range);
+}
+
+TEST(VMRange, SetEndAddress) {
+ VMRange range(0x100, 0x200);
+
+ range.SetEndAddress(0xFF);
+ EXPECT_EQ(0U, range.GetByteSize());
+ EXPECT_FALSE(range.IsValid());
+
+ range.SetEndAddress(0x101);
+ EXPECT_EQ(1U, range.GetByteSize());
+ EXPECT_TRUE(range.IsValid());
+}
+
+TEST(VMRange, ContainsAddr) {
+ VMRange range(0x100, 0x200);
+
+ EXPECT_FALSE(range.Contains(0x00));
+ EXPECT_FALSE(range.Contains(0xFF));
+ EXPECT_TRUE(range.Contains(0x100));
+ EXPECT_TRUE(range.Contains(0x101));
+ EXPECT_TRUE(range.Contains(0x1FF));
+ EXPECT_FALSE(range.Contains(0x200));
+ EXPECT_FALSE(range.Contains(0x201));
+ EXPECT_FALSE(range.Contains(0xFFF));
+ EXPECT_FALSE(range.Contains(std::numeric_limits::max()));
+}
+
+TEST(VMRange, ContainsRange) {
+ VMRange range(0x100, 0x200);
+
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x100)));
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x101)));
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x105)));
+ EXPECT_TRUE(range.Contains(VMRange(0x101, 0x105)));
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x1FF)));
+ EXPECT_TRUE(range.Contains(VMRange(0x105, 0x200)));
+ EXPECT_FALSE(range.Contains(VMRange(0x105, 0x201)));
+ EXPECT_FALSE(range.Contains(VMRange(0x200, 0x201)));
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x200)));
+ EXPECT_FALSE(
+ range.Contains(VMRange(0x105,
std::numeric_limits::max(;
+
+ // Empty range.
+ EXPECT_TRUE(range.Contains(VMRange(0x100, 0x100)));
+
+ range.Clear();
+ EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+}
+
+TEST(VMRange, Ordering) {
+ VMRange range1(0x44, 0x200);
+ VMRange range2(0x100, 0x1FF);
+ VMRange range3(0x100, 0x200);
+
+ EXPECT_LE(range1, range1);
+ EXPECT_GE(range1, range1);
+
+ EXPECT_LT(range1, range2);
+ EXPECT_LT(range2, range3);
+
+ EXPECT_GT(range2, range1);
+ EXPECT_GT(range3, range2);
+
+ // Ensure that < and > are always false when comparing ranges with
themselves.
+ EXPECT_FALSE(range1 < range1);
+ EXPECT_FALSE(range2 < range2);
+ EXPECT_FALSE(range3 < range3);
+
+ EXPECT_FALSE(range1 > range
