[Lldb-commits] [PATCH] D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties.
aprantl added a comment. @jingham: do you have any opinion about the right SBAPI for manipulating settings like Alexander outlined? https://reviews.llvm.org/D47302 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49579: Support parsing minidump files that are created by Breakpad.
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. The extra padding is unfortunate, but I guess we have to live with it now. Looks good. Thanks. https://reviews.llvm.org/D49579 ___ 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 inline comments. Comment at: cmake/modules/LLDBConfig.cmake:417-423 +option(LLDB_USE_BUILTIN_DEMANGLER "Use lldb's builtin demangler" OFF) +option(LLDB_USE_LLVM_DEMANGLER "Use llvm's new partial demangler" ON) endif() if(LLDB_USE_BUILTIN_DEMANGLER) add_definitions(-DLLDB_USE_BUILTIN_DEMANGLER) +elseif(LLDB_USE_LLVM_DEMANGLER) +add_definitions(-DLLDB_USE_LLVM_DEMANGLER) Since these options are mutually exclusive it might be better to make this a single multi-valued setting. Also, this new demangler should also be available in the MSVC case, should it not? Yhe reason we have `if(MSVC)` here is because the "system" demangler is not available there, but that should not be an issue here. 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 added a comment. Hi all, thanks for your feedback. I will update the review with code fixes in a bit. > Also we should compare demangled names between the two to ensure there are no > differences as we are doing this. Yes I can definitely do it manually. When it comes to writing tests, I would consider it more like testing the demangler itself than its integration with LLDB right? Shall we add tests in LLVM that compare to `__cxa_demangle`? https://github.com/llvm-mirror/llvm/blob/master/unittests/Demangle/PartialDemangleTest.cpp > running nm on a large executable (e.g. clang or lldb itself) and see whether > we demangle in the same exact way and measure the time needed for demangling Good idea, will try that. > this new demangler should also be available in the MSVC case, should it not? I don't think the Itanium mangler supports MSVC mangling. > always add lldb-commits to the subscribers of your phab reviews Aye aye 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
labath added a comment. In https://reviews.llvm.org/D49612#1171363, @sgraenitz wrote: > > this new demangler should also be available in the MSVC case, should it not? > > I don't think the Itanium mangler supports MSVC mangling. That's fine. It just needs to be able to demangle itanium names when running on an MSVC platform. 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 added a comment. > That's fine. It just needs to be able to demangle itanium names when running > on an MSVC platform. IIUC `cstring_mangling_scheme()` should return `eManglingSchemeItanium` in this case and switch to the case label without the `#if defined(_MSC_VER)`. 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
labath added a comment.
In https://reviews.llvm.org/D49612#1171395, @sgraenitz wrote:
> > That's fine. It just needs to be able to demangle itanium names when
> > running on an MSVC platform.
>
> IIUC `cstring_mangling_scheme()` should return `eManglingSchemeItanium` in
> this case and switch to the case label without the `#if defined(_MSC_VER)`.
Yes, and that's correct.
But I am talking about cmake code here. `option(LLDB_USE_LLVM_DEMANGLER "Use
llvm's new partial demangler" ON)` is in an `!MSVC` block, so the user will not
be able to choose which demangler to use there. My point was simply to move
that option out of that block so that it is available on msvc builds as well.
(That is if the new demangler works with msvc, but I don't see a reason why it
shouldn't.)
Combined with the other suggestion of making this a tri-valued setting, you
could have a single setting with values "system", "legacy", "new" or whatever
you think best describes them, and then do something like `if (MSVC AND
${setting} == "system") message(ERROR "System itanium demangler not available
on MSVC")`.
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] D48351: Move dumping code out of RegisterValue class
labath added a comment. In https://reviews.llvm.org/D48351#1169959, @jingham wrote: > Dump is really meant to be the private description of the object that you > would use for logging and the like - Description was the public face of a > class. So while the Description-like functionality could have a restrictions > that constrain its use to pretty high up in the stack, you really want to be > able to Dump some object you've been handed - particularly into the log > stream - wherever that might help. So I don't think moving to restrict where > you can use Dump is a good idea. I fully agree with that. It should be easy to add a simple dump function for logging purposes which omits any fancy formatting that need higher level constructs to any object. It's just that the current dump function is used for both things: logging, and displaying information to the user. I can change the name to DescribeRegisterValue or something like that if you think that makes more sense. https://reviews.llvm.org/D48351 ___ 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 156743.
sgraenitz added a comment.
Fix: Use malloc instead of new for allocating the demangled_name buffer.
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,11 +16,13 @@
#pragma comment(lib, "dbghelp.lib")
#endif
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
+#if defined(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"
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+#include "llvm/Demangle/Demangle.h"
#else
// FreeBSD9-STABLE requires this to know about size_t in cxxabi.
#include
@@ -295,15 +297,24 @@
break;
}
case eManglingSchemeItanium: {
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
+#if defined(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);
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+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 default_size = 80;
+ demangled_name = static_cast(::malloc(default_size));
+ demangled_name = IPD.finishDemangle(demangled_name, &default_size);
+}
#else
demangled_name = abi::__cxa_demangle(mangled_name, NULL, NULL, NULL);
#endif
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 = 8C3BD9951EF45D9B0016C343 /* MainThreadCheckerRuntime.cpp */; };
2689004313353E0400698AC0 /* Mangled.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26BC7E8010F1B85900F91463 /* Mangled.cpp */; };
+ 4F29D3CF21010FA3003B549A /* MangledTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4F29D3CD21010F84003B549A /* MangledTest.cpp */; };
4CD44CFC20B37C440003557C /* ManualDWARFIndex.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4CD44CF920B37C440003557C /* ManualDWARFIndex.cpp */; };
49DCF702170E70120092F75E /* Materializer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 49DCF700170E70120092F75E /* Materializer.cpp */; };
2690B3711381D5C300ECFBAE /* Memory.cpp in Sources *
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
sgraenitz updated this revision to Diff 156745.
sgraenitz added a comment.
Enable LLDB_USE_LLVM_DEMANGLER on MSVC platforms.
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,11 +16,13 @@
#pragma comment(lib, "dbghelp.lib")
#endif
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
+#if defined(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"
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+#include "llvm/Demangle/Demangle.h"
#else
// FreeBSD9-STABLE requires this to know about size_t in cxxabi.
#include
@@ -295,15 +297,24 @@
break;
}
case eManglingSchemeItanium: {
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
+#if defined(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);
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+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 default_size = 80;
+ demangled_name = static_cast(::malloc(default_size));
+ demangled_name = IPD.finishDemangle(demangled_name, &default_size);
+}
#else
demangled_name = abi::__cxa_demangle(mangled_name, NULL, NULL, NULL);
#endif
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 = 8C3BD9951EF45D9B0016C343 /* MainThreadCheckerRuntime.cpp */; };
2689004313353E0400698AC0 /* Mangled.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26BC7E8010F1B85900F91463 /* Mangled.cpp */; };
+ 4F29D3CF21010FA3003B549A /* MangledTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4F29D3CD21010F84003B549A /* MangledTest.cpp */; };
4CD44CFC20B37C440003557C /* ManualDWARFIndex.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4CD44CF920B37C440003557C /* ManualDWARFIndex.cpp */; };
49DCF702170E70120092F75E /* Materializer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 49DCF700170E70120092F75E /* Materializer.cpp */; };
2690B3711381D5C300ECFBAE /* Memory.cpp in Sources */ = {isa = PBXBuildFile
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
sgraenitz marked an inline comment as done. sgraenitz added inline comments. Comment at: cmake/modules/LLDBConfig.cmake:417-423 +option(LLDB_USE_BUILTIN_DEMANGLER "Use lldb's builtin demangler" OFF) +option(LLDB_USE_LLVM_DEMANGLER "Use llvm's new partial demangler" ON) endif() if(LLDB_USE_BUILTIN_DEMANGLER) add_definitions(-DLLDB_USE_BUILTIN_DEMANGLER) +elseif(LLDB_USE_LLVM_DEMANGLER) +add_definitions(-DLLDB_USE_LLVM_DEMANGLER) labath wrote: > Since these options are mutually exclusive it might be better to make this a > single multi-valued setting. Also, this new demangler should also be > available in the MSVC case, should it not? Yhe reason we have `if(MSVC)` here > is because the "system" demangler is not available there, but that should not > be an issue here. > I am talking about cmake code here. Sorry, just saw that too. > Since these options are mutually exclusive it might be better to make this a > single multi-valued setting. The old `LLDB_USE_BUILTIN_DEMANGLER` might be removed entirely once we have benchmark results. Otherwise I will make it multi-valued. 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 marked an inline comment as done. sgraenitz added a comment. Quick local performance check doing `target create clang` in current review version vs. master HEAD version (both loading the exact same release build of clang) looks promising. It's faster already now, so I would remove the option for the builtin demangling. review version = LLDB_USE_LLVM_DEMANGLER: TargetList::CreateTarget (file = 'clang', arch = 'x86_64') = 0.762155337 sec master HEAD version = LLDB_USE_BUILTIN_DEMANGLER: TargetList::CreateTarget (file = 'clang', arch = 'x86_64') = 1.010040359 sec 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] r337689 - [lldb-mi] Re-implement data-info-line command.
Author: apolyakov
Date: Mon Jul 23 06:02:41 2018
New Revision: 337689
URL: http://llvm.org/viewvc/llvm-project?rev=337689&view=rev
Log:
[lldb-mi] Re-implement data-info-line command.
Summary: Now data-info-line command uses SB API instead of HandleCommand.
Reviewers: aprantl, clayborg, jingham
Reviewed By: aprantl
Subscribers: ki.stfu, lldb-commits
Differential Revision: https://reviews.llvm.org/D49062
Added:
lldb/trunk/lit/tools/lldb-mi/data/
lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test
lldb/trunk/lit/tools/lldb-mi/data/inputs/
lldb/trunk/lit/tools/lldb-mi/data/inputs/data-info-line.c
lldb/trunk/lit/tools/lldb-mi/data/lit.local.cfg
Modified:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py
lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
lldb/trunk/tools/lldb-mi/MICmdCmdData.h
Added: lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test?rev=337689&view=auto
==
--- lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test (added)
+++ lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test Mon Jul 23 06:02:41
2018
@@ -0,0 +1,36 @@
+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# RUN: %cc -o %t %p/inputs/data-info-line.c -g
+# RUN: %lldbmi %t < %s | FileCheck %s
+
+# Test lldb-mi -data-info-line command.
+
+# Check that we have a valid target created via '%lldbmi %t'.
+# CHECK: ^done
+
+-break-insert main
+# CHECK: ^done,bkpt={number="1"
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
+-data-info-line *0x0
+# Test that -data-info-line fails when invalid address is specified.
+# CHECK: ^error,msg="Command 'data-info-line'. Error: The LineEntry is absent
or has an unknown format."
+
+-data-info-line unknown_file:1
+# Test that -data-info-line fails when file is unknown.
+# CHECK: ^error,msg="Command 'data-info-line'. Error: The LineEntry is absent
or has an unknown format."
+
+-data-info-line data-info-line.c:bad_line
+# Test that -data-info-line fails when line has invalid format.
+# CHECK: ^error,msg="Command 'data-info-line'. Error: The LineEntry is absent
or has an unknown format."
+
+-data-info-line data-info-line.c:0
+# Test that -data-info-line fails when invalid line is specified.
+# CHECK: ^error,msg="Command 'data-info-line'. Error: The LineEntry is absent
or has an unknown format."
+
+-data-info-line data-info-line.c:2
+# CHECK:
^done,start="0x{{[0-9a-f]+}}",end="0x{{[0-9a-f]+}}",file="{{.*}}data-info-line.c",line="{{[0-9]+}}"
Added: lldb/trunk/lit/tools/lldb-mi/data/inputs/data-info-line.c
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/tools/lldb-mi/data/inputs/data-info-line.c?rev=337689&view=auto
==
--- lldb/trunk/lit/tools/lldb-mi/data/inputs/data-info-line.c (added)
+++ lldb/trunk/lit/tools/lldb-mi/data/inputs/data-info-line.c Mon Jul 23
06:02:41 2018
@@ -0,0 +1,4 @@
+int main(void) {
+ int x = 0;
+ return 12345 + x;
+}
Added: lldb/trunk/lit/tools/lldb-mi/data/lit.local.cfg
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/tools/lldb-mi/data/lit.local.cfg?rev=337689&view=auto
==
--- lldb/trunk/lit/tools/lldb-mi/data/lit.local.cfg (added)
+++ lldb/trunk/lit/tools/lldb-mi/data/lit.local.cfg Mon Jul 23 06:02:41 2018
@@ -0,0 +1 @@
+config.suffixes = ['.test']
Modified:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py?rev=337689&r1=337688&r2=337689&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py
Mon Jul 23 06:02:41 2018
@@ -335,62 +335,6 @@ class MiDataTestCase(lldbmi_testcase.MiT
@skipIfWindows # llvm.org/pr24452: Get lldb-mi tests working on Windows
@skipIfDarwin # pexpect is known to be unreliable on Darwin
@skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread
races
-def test_lldbmi_data_info_line(self):
-"""Test that 'lldb-mi --interpreter' works for -data-info-line."""
-
-self.spawnLldbMi(args=None)
-
-# Load executable
-self.runCmd("-file-exec-and-symbols %s" % self.myexe)
-self.expect("\^done")
-
-# Run to main
-self.runCmd("-break-insert -f main")
-self.expect("\^done,bkpt={number=\"1\"")
-self.runCmd("-exec-run")
-self.expect("\^running")
-self.expect("\*stopped,reason=\"breakpoint-hit\"")
-
-# Get the address of main and its line
-self.runCmd("-data-evaluate-exp
[Lldb-commits] [PATCH] D49062: [lldb-mi] Re-implement data-info-line command.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337689: [lldb-mi] Re-implement data-info-line command.
(authored by apolyakov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49062?vs=156299&id=156763#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49062
Files:
lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test
lldb/trunk/lit/tools/lldb-mi/data/inputs/data-info-line.c
lldb/trunk/lit/tools/lldb-mi/data/lit.local.cfg
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py
lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
lldb/trunk/tools/lldb-mi/MICmdCmdData.h
Index: lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
===
--- lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
+++ lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
@@ -19,15 +19,14 @@
// CMICmdCmdDataInfoLine implementation.
// Third Party Headers:
-#include "lldb/API/SBCommandInterpreter.h"
#include "lldb/API/SBInstruction.h"
#include "lldb/API/SBInstructionList.h"
#include "lldb/API/SBStream.h"
#include "lldb/API/SBThread.h"
-#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Twine.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Regex.h"
#include // For PRIx64
+#include
// In-house headers:
#include "MICmdArgValConsume.h"
@@ -46,6 +45,12 @@
#include "MICmnMIResultRecord.h"
#include "MICmnMIValueConst.h"
+namespace {
+CMIUtilString IntToHexAddrStr(uint32_t number) {
+ return CMIUtilString("0x" + llvm::Twine::utohexstr(number).str());
+}
+} // namespace
+
//++
//
// Details: CMICmdCmdDataEvaluateExpression constructor.
@@ -1588,7 +1593,9 @@
// Throws: None.
//--
CMICmdCmdDataInfoLine::CMICmdCmdDataInfoLine()
-: m_constStrArgLocation("location") {
+: m_constStrArgLocation("location"),
+ m_resultRecord(m_cmdData.strMiCmdToken,
+ CMICmnMIResultRecord::eResultClass_Done) {
// Command factory matches this name with that received from the stdin stream
m_strMiCmd = "data-info-line";
@@ -1604,7 +1611,7 @@
// Return: None.
// Throws: None.
//--
-CMICmdCmdDataInfoLine::~CMICmdCmdDataInfoLine() {}
+CMICmdCmdDataInfoLine::~CMICmdCmdDataInfoLine() = default;
//++
//
@@ -1637,98 +1644,84 @@
bool CMICmdCmdDataInfoLine::Execute() {
CMICMDBASE_GETOPTION(pArgLocation, String, m_constStrArgLocation);
+ lldb::SBLineEntry line;
+ bool found_line = false;
const CMIUtilString &strLocation(pArgLocation->GetValue());
- CMIUtilString strCmdOptionsLocation;
+ lldb::SBTarget target = CMICmnLLDBDebugSessionInfo::Instance().GetTarget();
+
if (strLocation.at(0) == '*') {
// Parse argument:
// *0x12345
-// ^^^ -- address
-const CMIUtilString strAddress(strLocation.substr(1));
-strCmdOptionsLocation =
-CMIUtilString::Format("--address %s", strAddress.c_str());
+// ^ -- address
+lldb::addr_t address = 0x0;
+if (llvm::StringRef(strLocation.substr(1)).getAsInteger(0, address)) {
+ SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_SOME_ERROR),
+ m_cmdData.strMiCmd.c_str(),
+ "Failed to parse address."));
+ return MIstatus::failure;
+}
+line = target.ResolveFileAddress(address).GetLineEntry();
+// Check that found line is valid.
+if (line.GetLine())
+ found_line = true;
} else {
const size_t nLineStartPos = strLocation.rfind(':');
if ((nLineStartPos == std::string::npos) || (nLineStartPos == 0) ||
(nLineStartPos == strLocation.length() - 1)) {
- SetError(
- CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_LOCATION_FORMAT),
-m_cmdData.strMiCmd.c_str(), strLocation.c_str())
- .c_str());
+ SetError(CMIUtilString::Format(
+ MIRSRC(IDS_CMD_ERR_INVALID_LOCATION_FORMAT),
+ m_cmdData.strMiCmd.c_str(), strLocation.c_str()));
return MIstatus::failure;
}
// Parse argument:
// hello.cpp:5
// ^ -- file
// ^ -- line
-const CMIUtilString strFile(strLocation.substr(0, nLineStartPos));
-const CMIUtilString strLine(strLocation.substr(nLineStartPos + 1));
-strCmdOptionsLocation =
-CMIUtilString::Format("--file \"%s\" --line %s",
- strFile.AddSlashes().c_str(), strLine.c_str());
+const CMIUtilString &strFile(strLocation.substr(0, nLineStartPos));
+uint32_t numLine = 0;
+llvm::StringRef(strLocation.substr(nLineStartPos + 1))
+.getAsInteger(0, numLine);
+lldb::SBSymbolContextList sc_cu_list =
+target.FindCompileUnits(lldb::SBFileSpec(strFile.c_
[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB
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);
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?
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] D49632: [WIP] Re-implement MI HandleProcessEventStateSuspended.
clayborg added a comment. If you look at what this patch is doing, it ends sending text to stdout at the end. So it seems like this function's sole purpose is to report the process status after stop to STDOUT. Seeing as this is a machine interface (MI) to a debugger, I was wondering why it would do this. https://reviews.llvm.org/D49632 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337692 - Fix windows build after r337689
Author: apolyakov
Date: Mon Jul 23 07:10:30 2018
New Revision: 337692
URL: http://llvm.org/viewvc/llvm-project?rev=337692&view=rev
Log:
Fix windows build after r337689
Added missing header.
Modified:
lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
Modified: lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp?rev=337692&r1=337691&r2=337692&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp Mon Jul 23 07:10:30 2018
@@ -44,6 +44,7 @@
#include "MICmnLLDBUtilSBValue.h"
#include "MICmnMIResultRecord.h"
#include "MICmnMIValueConst.h"
+#include "Platform.h"
namespace {
CMIUtilString IntToHexAddrStr(uint32_t number) {
___
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 156778.
sgraenitz added a comment.
Remove CMake options LLDB_USE_BUILTIN_DEMANGLER and LLDB_USE_LLVM_DEMANGLER.
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,17 +16,6 @@
#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"
@@ -39,6 +28,7 @@
#include "Plugins/Language/ObjC/ObjCLanguage.h"
#include "llvm/ADT/StringRef.h"// for StringRef
+#include "llvm/Demangle/Demangle.h"
#include "llvm/Support/Compiler.h" // for LLVM_PRETT...
#include// for mutex, loc...
@@ -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 default_size = 80;
+ demangled_name = static_cast(::malloc(default_size));
+ demangled_name = IPD.finishDemangle(demangled_name, &default_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 = 8C3BD9951EF45D9B0016C343 /* MainThreadCheckerRuntime.cpp */; };
2689004313353E0400698AC0 /* Mangled.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26BC7E8010F1B85900F91463 /* Mangled.cpp */; };
+ 4F29D3CF21010FA3003B549A /* MangledTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4F29D3CD21010F84003B549A /* MangledTest.cpp */; };
4CD44CFC20B37C440003557C /* ManualDWARFIndex
[Lldb-commits] [PATCH] D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties.
clayborg added a comment. We could end up moving anything that is set by a target property into the lldb_private target property class if it isn't already there and then that would fix things. https://reviews.llvm.org/D47302 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337694 - Add support for parsing Breakpad minidump files that can have extra padding in the module, thread and memory lists.
Author: gclayton
Date: Mon Jul 23 07:16:08 2018
New Revision: 337694
URL: http://llvm.org/viewvc/llvm-project?rev=337694&view=rev
Log:
Add support for parsing Breakpad minidump files that can have extra padding in
the module, thread and memory lists.
Differential Revision: https://reviews.llvm.org/D49579
Added:
lldb/trunk/unittests/Process/minidump/Inputs/memory-list-not-padded.dmp
(with props)
lldb/trunk/unittests/Process/minidump/Inputs/memory-list-padded.dmp (with
props)
lldb/trunk/unittests/Process/minidump/Inputs/module-list-not-padded.dmp
(with props)
lldb/trunk/unittests/Process/minidump/Inputs/module-list-padded.dmp (with
props)
lldb/trunk/unittests/Process/minidump/Inputs/thread-list-not-padded.dmp
(with props)
lldb/trunk/unittests/Process/minidump/Inputs/thread-list-padded.dmp (with
props)
Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
lldb/trunk/unittests/Process/minidump/CMakeLists.txt
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp?rev=337694&r1=337693&r2=337694&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp Mon Jul 23
07:16:08 2018
@@ -81,11 +81,17 @@ const MinidumpThread *MinidumpThread::Pa
llvm::ArrayRef
MinidumpThread::ParseThreadList(llvm::ArrayRef &data) {
+ const auto orig_size = data.size();
const llvm::support::ulittle32_t *thread_count;
Status error = consumeObject(data, thread_count);
if (error.Fail() || *thread_count * sizeof(MinidumpThread) > data.size())
return {};
+ // Compilers might end up padding an extra 4 bytes depending on how the
+ // structure is padded by the compiler and the #pragma pack settings.
+ if (4 + *thread_count * sizeof(MinidumpThread) < orig_size)
+data = data.drop_front(4);
+
return llvm::ArrayRef(
reinterpret_cast(data.data()), *thread_count);
}
@@ -157,12 +163,17 @@ const MinidumpModule *MinidumpModule::Pa
llvm::ArrayRef
MinidumpModule::ParseModuleList(llvm::ArrayRef &data) {
-
+ const auto orig_size = data.size();
const llvm::support::ulittle32_t *modules_count;
Status error = consumeObject(data, modules_count);
if (error.Fail() || *modules_count * sizeof(MinidumpModule) > data.size())
return {};
-
+
+ // Compilers might end up padding an extra 4 bytes depending on how the
+ // structure is padded by the compiler and the #pragma pack settings.
+ if (4 + *modules_count * sizeof(MinidumpModule) < orig_size)
+data = data.drop_front(4);
+
return llvm::ArrayRef(
reinterpret_cast(data.data()), *modules_count);
}
@@ -180,11 +191,17 @@ MinidumpExceptionStream::Parse(llvm::Arr
llvm::ArrayRef
MinidumpMemoryDescriptor::ParseMemoryList(llvm::ArrayRef &data) {
+ const auto orig_size = data.size();
const llvm::support::ulittle32_t *mem_ranges_count;
Status error = consumeObject(data, mem_ranges_count);
if (error.Fail() ||
*mem_ranges_count * sizeof(MinidumpMemoryDescriptor) > data.size())
return {};
+
+ // Compilers might end up padding an extra 4 bytes depending on how the
+ // structure is padded by the compiler and the #pragma pack settings.
+ if (4 + *mem_ranges_count * sizeof(MinidumpMemoryDescriptor) < orig_size)
+data = data.drop_front(4);
return llvm::makeArrayRef(
reinterpret_cast(data.data()),
Modified: lldb/trunk/unittests/Process/minidump/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/CMakeLists.txt?rev=337694&r1=337693&r2=337694&view=diff
==
--- lldb/trunk/unittests/Process/minidump/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Process/minidump/CMakeLists.txt Mon Jul 23 07:16:08
2018
@@ -19,6 +19,12 @@ set(test_inputs
fizzbuzz_no_heap.dmp
fizzbuzz_wow64.dmp
bad_duplicate_streams.dmp
- bad_overlapping_streams.dmp)
+ bad_overlapping_streams.dmp
+ thread-list-padded.dmp
+ thread-list-not-padded.dmp
+ module-list-padded.dmp
+ module-list-not-padded.dmp
+ memory-list-padded.dmp
+ memory-list-not-padded.dmp)
add_unittest_inputs(LLDBMinidumpTests "${test_inputs}")
Added: lldb/trunk/unittests/Process/minidump/Inputs/memory-list-not-padded.dmp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/Inputs/memory-list-not-padded.dmp?rev=337694&view=auto
==
Binary file - no diff available.
Propchange:
lldb/trunk/unittests/Process/minidump/Inputs/memory-list-not-padded.dmp
[Lldb-commits] [PATCH] D49579: Support parsing minidump files that are created by Breakpad.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337694: Add support for parsing Breakpad minidump files that
can have extra padding in… (authored by gclayton, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49579?vs=156545&id=156779#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49579
Files:
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
lldb/trunk/unittests/Process/minidump/CMakeLists.txt
lldb/trunk/unittests/Process/minidump/Inputs/memory-list-not-padded.dmp
lldb/trunk/unittests/Process/minidump/Inputs/memory-list-padded.dmp
lldb/trunk/unittests/Process/minidump/Inputs/module-list-not-padded.dmp
lldb/trunk/unittests/Process/minidump/Inputs/module-list-padded.dmp
lldb/trunk/unittests/Process/minidump/Inputs/thread-list-not-padded.dmp
lldb/trunk/unittests/Process/minidump/Inputs/thread-list-padded.dmp
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
Index: lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -84,6 +84,76 @@
EXPECT_EQ(1232UL, context.size());
}
+TEST_F(MinidumpParserTest, GetThreadListNotPadded) {
+ // Verify that we can load a thread list that doesn't have 4 bytes of padding
+ // after the thread count.
+ SetUpData("thread-list-not-padded.dmp");
+ llvm::ArrayRef thread_list;
+
+ thread_list = parser->GetThreads();
+ ASSERT_EQ(2UL, thread_list.size());
+ EXPECT_EQ(0x11223344UL, thread_list[0].thread_id);
+ EXPECT_EQ(0x55667788UL, thread_list[1].thread_id);
+}
+
+TEST_F(MinidumpParserTest, GetThreadListPadded) {
+ // Verify that we can load a thread list that has 4 bytes of padding
+ // after the thread count as found in breakpad minidump files.
+ SetUpData("thread-list-padded.dmp");
+ auto thread_list = parser->GetThreads();
+ ASSERT_EQ(2UL, thread_list.size());
+ EXPECT_EQ(0x11223344UL, thread_list[0].thread_id);
+ EXPECT_EQ(0x55667788UL, thread_list[1].thread_id);
+}
+
+TEST_F(MinidumpParserTest, GetModuleListNotPadded) {
+ // Verify that we can load a module list that doesn't have 4 bytes of padding
+ // after the module count.
+ SetUpData("module-list-not-padded.dmp");
+ auto module_list = parser->GetModuleList();
+ ASSERT_EQ(2UL, module_list.size());
+ EXPECT_EQ(0x1000UL, module_list[0].base_of_image);
+ EXPECT_EQ(0x2000UL, module_list[0].size_of_image);
+ EXPECT_EQ(0x5000UL, module_list[1].base_of_image);
+ EXPECT_EQ(0x3000UL, module_list[1].size_of_image);
+}
+
+TEST_F(MinidumpParserTest, GetModuleListPadded) {
+ // Verify that we can load a module list that has 4 bytes of padding
+ // after the module count as found in breakpad minidump files.
+ SetUpData("module-list-padded.dmp");
+ auto module_list = parser->GetModuleList();
+ ASSERT_EQ(2UL, module_list.size());
+ EXPECT_EQ(0x1000UL, module_list[0].base_of_image);
+ EXPECT_EQ(0x2000UL, module_list[0].size_of_image);
+ EXPECT_EQ(0x5000UL, module_list[1].base_of_image);
+ EXPECT_EQ(0x3000UL, module_list[1].size_of_image);
+}
+
+TEST_F(MinidumpParserTest, GetMemoryListNotPadded) {
+ // Verify that we can load a memory list that doesn't have 4 bytes of padding
+ // after the memory range count.
+ SetUpData("memory-list-not-padded.dmp");
+ auto mem = parser->FindMemoryRange(0x8000);
+ ASSERT_TRUE(mem.hasValue());
+ EXPECT_EQ((lldb::addr_t)0x8000, mem->start);
+ mem = parser->FindMemoryRange(0x8010);
+ ASSERT_TRUE(mem.hasValue());
+ EXPECT_EQ((lldb::addr_t)0x8010, mem->start);
+}
+
+TEST_F(MinidumpParserTest, GetMemoryListPadded) {
+ // Verify that we can load a memory list that has 4 bytes of padding
+ // after the memory range count as found in breakpad minidump files.
+ SetUpData("memory-list-padded.dmp");
+ auto mem = parser->FindMemoryRange(0x8000);
+ ASSERT_TRUE(mem.hasValue());
+ EXPECT_EQ((lldb::addr_t)0x8000, mem->start);
+ mem = parser->FindMemoryRange(0x8010);
+ ASSERT_TRUE(mem.hasValue());
+ EXPECT_EQ((lldb::addr_t)0x8010, mem->start);
+}
+
TEST_F(MinidumpParserTest, TruncatedMinidumps) {
InvalidMinidump("linux-x86_64.dmp", 32);
InvalidMinidump("linux-x86_64.dmp", 100);
Index: lldb/trunk/unittests/Process/minidump/CMakeLists.txt
===
--- lldb/trunk/unittests/Process/minidump/CMakeLists.txt
+++ lldb/trunk/unittests/Process/minidump/CMakeLists.txt
@@ -19,6 +19,12 @@
fizzbuzz_no_heap.dmp
fizzbuzz_wow64.dmp
bad_duplicate_streams.dmp
- bad_overlapping_streams.dmp)
+ bad_overlapping_streams.dmp
+ thread-list-padded.dmp
+ thread-list-not-padded.dmp
+ module-list-padded.dmp
+ module-list-not-padded.dmp
+ memory-list-padded.dmp
+ memory-list-not-padded.dmp)
add_unittest_inputs(LLDBMinidumpTests "${test_inputs}")
[Lldb-commits] [PATCH] D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties.
apolyakov added a comment. Another approach is to implement SBTargetSettings' functionality such as serialization inside lldb_private TargetProperties class and then just add an API to the SBTarget. For example: `void SBTarget::SerializeProperties(...)` What do you think about this way? https://reviews.llvm.org/D47302 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49632: [WIP] Re-implement MI HandleProcessEventStateSuspended.
apolyakov added a comment. You mean that it's unreasonable to provide such an output to stdout since MI clients are text redactors, IDE and not people? https://reviews.llvm.org/D49632 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties.
apolyakov added a comment. In https://reviews.llvm.org/D47302#1171639, @clayborg wrote: > We could end up moving anything that is set by a target property into the > lldb_private target property class if it isn't already there and then that > would fix things. If you remember, we started this discussion from adding the `AppendImageSearchPath(const char *from, const char *to)` method. To do so, we need to modify `Target`'s member variable `m_image_search_paths`. It means that to move this option to TargetProperties we have to also move this member variable from `Target` to `TargetProperties`. We need to be sure that it won't break anything. https://reviews.llvm.org/D47302 ___ 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
erik.pilkington added a comment.
In https://reviews.llvm.org/D49612#1171550, @sgraenitz wrote:
> Quick local performance check doing `target create clang` in current review
> version vs. master HEAD version (both loading the exact same release build of
> clang) looks promising. It's faster already now, so I would remove the option
> for the builtin demangling.
>
> review version = LLDB_USE_LLVM_DEMANGLER:
> TargetList::CreateTarget (file = 'clang', arch = 'x86_64') = 0.762155337 sec
>
> master HEAD version = LLDB_USE_BUILTIN_DEMANGLER:
> TargetList::CreateTarget (file = 'clang', arch = 'x86_64') = 1.010040359 sec
>
Oh, nice! I was expecting FastDemangle to still beat the partial demangler. If
FastDemangle is now slower than maybe it should be removed (or at least
renamed!).
Comment at: source/Core/Mangled.cpp:310
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
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?
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 added a comment.
Well when repeating this test, the values are not always that far apart from
each other, but on average the old USE_BUILTIN_DEMANGLER path the slower one.
Maybe FastDemangle is still faster than IPD in success case, but the overhead
from the fallback cases is adding up. (The USE_BUILTIN_DEMANGLER code path is
also more noisy in terms of performance, probably same issue here.)
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:
> > 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.
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 added reviewers: erik.pilkington, labath, clayborg, mgorny, davide, JDevlieghere. sgraenitz added a comment. Sorry if the review is a little bumpy, it's my first one. Added all subscribers as reviewers now. Hope that's ok. The current version is a rather simple change, that slightly reduces complexity and slightly improves performance. IMHO having it committed would make it easier to discuss next steps on the list and work on improvements step by step. Thus I would prefer to keep this review small here and instead open a new one once I have a proposal. Happy to hear what you think and answer questions. 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
erik.pilkington 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);
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.
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] D49695: [cmake] Remove unused ${LLDB_PLUGINS} dependency from our Objective-C++ CMake config
teemperor created this revision.
Herald added a subscriber: mgorny.
LLDB_PLUGINS doesn't exist as a variable, so this line doesn't add any
dependencies and is
just confusing. It seems this slipped in from the gdb-remote CMake I was using
as a CMake template.
The gdb-remote CMake itself is using a local LLDB_PLUGINS variable, so that
code is fine.
https://reviews.llvm.org/D49695
Files:
source/Host/macosx/objcxx/CMakeLists.txt
source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
Index: source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
===
--- source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
+++ source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
@@ -9,7 +9,6 @@
lldbSymbol
lldbTarget
lldbUtility
-${LLDB_PLUGINS}
${EXTRA_LIBS}
LINK_COMPONENTS
Index: source/Host/macosx/objcxx/CMakeLists.txt
===
--- source/Host/macosx/objcxx/CMakeLists.txt
+++ source/Host/macosx/objcxx/CMakeLists.txt
@@ -12,7 +12,6 @@
lldbSymbol
lldbTarget
lldbUtility
-${LLDB_PLUGINS}
${EXTRA_LIBS}
LINK_COMPONENTS
Index: source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
===
--- source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
+++ source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
@@ -9,7 +9,6 @@
lldbSymbol
lldbTarget
lldbUtility
-${LLDB_PLUGINS}
${EXTRA_LIBS}
LINK_COMPONENTS
Index: source/Host/macosx/objcxx/CMakeLists.txt
===
--- source/Host/macosx/objcxx/CMakeLists.txt
+++ source/Host/macosx/objcxx/CMakeLists.txt
@@ -12,7 +12,6 @@
lldbSymbol
lldbTarget
lldbUtility
-${LLDB_PLUGINS}
${EXTRA_LIBS}
LINK_COMPONENTS
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49696: [NFC] Minor code refactoring.
teemperor created this revision. https://reviews.llvm.org/D49696 Files: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp === --- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -1163,10 +1163,7 @@ } } -if (success) - ret_success = true; -else - ret_success = false; +ret_success = success; } py_error.Reset(PyRefType::Borrowed, PyErr_Occurred()); Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp === --- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -1163,10 +1163,7 @@ } } -if (success) - ret_success = true; -else - ret_success = false; +ret_success = success; } py_error.Reset(PyRefType::Borrowed, PyErr_Occurred()); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337737 - [NFC] Minor code refactoring.
Author: teemperor Date: Mon Jul 23 13:56:49 2018 New Revision: 337737 URL: http://llvm.org/viewvc/llvm-project?rev=337737&view=rev Log: [NFC] Minor code refactoring. Subscribers: lldb-commits Differential Revision: https://reviews.llvm.org/D49696 Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=337737&r1=337736&r2=337737&view=diff == --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Mon Jul 23 13:56:49 2018 @@ -1163,10 +1163,7 @@ bool ScriptInterpreterPython::ExecuteOne } } -if (success) - ret_success = true; -else - ret_success = false; +ret_success = success; } py_error.Reset(PyRefType::Borrowed, PyErr_Occurred()); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49696: [NFC] Minor code refactoring.
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 rL337737: [NFC] Minor code refactoring. (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D49696?vs=156873&id=156874#toc Repository: rL LLVM https://reviews.llvm.org/D49696 Files: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Index: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp === --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -1163,10 +1163,7 @@ } } -if (success) - ret_success = true; -else - ret_success = false; +ret_success = success; } py_error.Reset(PyRefType::Borrowed, PyErr_Occurred()); Index: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp === --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -1163,10 +1163,7 @@ } } -if (success) - ret_success = true; -else - ret_success = false; +ret_success = success; } py_error.Reset(PyRefType::Borrowed, PyErr_Occurred()); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49695: [cmake] Remove unused ${LLDB_PLUGINS} dependency from our Objective-C++ CMake config
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D49695 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337741 - [cmake] Remove unused ${LLDB_PLUGINS} dependency from our Objective-C++ CMake config
Author: teemperor
Date: Mon Jul 23 14:14:52 2018
New Revision: 337741
URL: http://llvm.org/viewvc/llvm-project?rev=337741&view=rev
Log:
[cmake] Remove unused ${LLDB_PLUGINS} dependency from our Objective-C++ CMake
config
Summary:
LLDB_PLUGINS doesn't exist as a variable, so this line doesn't add any
dependencies and is
just confusing. It seems this slipped in from the gdb-remote CMake I was using
as a CMake template.
The gdb-remote CMake itself is using a local LLDB_PLUGINS variable, so that
code is fine.
Reviewers: davide
Reviewed By: davide
Subscribers: davide, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D49695
Modified:
lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt
lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
Modified: lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt?rev=337741&r1=337740&r2=337741&view=diff
==
--- lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt (original)
+++ lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt Mon Jul 23 14:14:52 2018
@@ -12,7 +12,6 @@ add_lldb_library(lldbHostMacOSXObjCXX
lldbSymbol
lldbTarget
lldbUtility
-${LLDB_PLUGINS}
${EXTRA_LIBS}
LINK_COMPONENTS
Modified: lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt?rev=337741&r1=337740&r2=337741&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt Mon Jul 23
14:14:52 2018
@@ -9,7 +9,6 @@ add_lldb_library(lldbPluginPlatformMacOS
lldbSymbol
lldbTarget
lldbUtility
-${LLDB_PLUGINS}
${EXTRA_LIBS}
LINK_COMPONENTS
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49695: [cmake] Remove unused ${LLDB_PLUGINS} dependency from our Objective-C++ CMake config
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337741: [cmake] Remove unused ${LLDB_PLUGINS} dependency
from our Objective-C++ CMake… (authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49695?vs=156868&id=156878#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49695
Files:
lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt
lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
Index: lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
===
--- lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
+++ lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
@@ -9,7 +9,6 @@
lldbSymbol
lldbTarget
lldbUtility
-${LLDB_PLUGINS}
${EXTRA_LIBS}
LINK_COMPONENTS
Index: lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt
===
--- lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt
+++ lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt
@@ -12,7 +12,6 @@
lldbSymbol
lldbTarget
lldbUtility
-${LLDB_PLUGINS}
${EXTRA_LIBS}
LINK_COMPONENTS
Index: lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
===
--- lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
+++ lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/CMakeLists.txt
@@ -9,7 +9,6 @@
lldbSymbol
lldbTarget
lldbUtility
-${LLDB_PLUGINS}
${EXTRA_LIBS}
LINK_COMPONENTS
Index: lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt
===
--- lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt
+++ lldb/trunk/source/Host/macosx/objcxx/CMakeLists.txt
@@ -12,7 +12,6 @@
lldbSymbol
lldbTarget
lldbUtility
-${LLDB_PLUGINS}
${EXTRA_LIBS}
LINK_COMPONENTS
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional
shafik updated this revision to Diff 156886.
shafik marked 4 inline comments as done.
shafik added a comment.
Addressing comments
- -O0 is not needed in Makefile
- engaged is not friendly terminology so switching to "Has Value"
- Switching test away from lldbinline style due to bug w/ .categories files
which does not work on these tests
https://reviews.llvm.org/D49271
Files:
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/LibCxx.cpp
source/Plugins/Language/CPlusPlus/LibCxx.h
source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,78 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+ OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+ }
+
+ size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+ }
+
+ bool MightHaveChildren() override { return true; }
+ bool Update() override;
+ size_t CalculateNumChildren() override { return m_size; }
+ ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+ size_t m_size = 0;
+ ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+ ValueObjectSP engaged_sp(
+ m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+ if (!engaged_sp)
+return false;
+
+ m_size = engaged_sp->GetValueAsSigned(0);
+
+ return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+ if (idx >= m_size)
+return ValueObjectSP();
+
+ ValueObjectSP val_sp(
+ m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+ ->GetParent()
+ ->GetChildAtIndex(0, true)
+ ->GetChildMemberWithName(ConstString("__val_"), true));
+
+ if (!val_sp)
+return ValueObjectSP();
+
+ CompilerType holder_type = val_sp->GetCompilerType();
+
+ if (!holder_type)
+return ValueObjectSP();
+
+ return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+ lldb::ValueObjectSP valobj_sp) {
+ if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+ return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
ValueObject &valobj, Stream &stream,
const TypeSummaryOptions &options); // libc++ std::wstring
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
bool LibcxxSmartPointerSummaryProvider(
ValueObject &valobj, Stream &stream,
const TypeSummaryOptions
@@ -133,6 +137,10 @@
SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
lldb::ValueObjectSP);
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+ lldb::ValueObjectSP valobj_sp);
+
} // namespace formatters
} // namespace lldb_private
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,26 @@
using namespace lldb_private;
using namespace lldb_private::formatters;
+bool lldb_private::formatters::LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+ ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+ if (!valobj_sp)
+return false;
+
+ ValueObjectSP engaged_sp(
+ v
[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional
shafik marked 6 inline comments as done.
shafik added a comment.
@davide One more pass
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:8
+
+lldbinline.MakeInlineTest(__file__, globals(), [no_debug_info_test])
jingham wrote:
> davide wrote:
> > I do wonder if you need a decorator to indicate that this is a libcxx only
> > test (and skip everywhere the library isn't supported).
> Yes, you do need to mark this or you will get spurious failures at least on
> Windows, which turns off the libcxx category.
Switched back to the old style tests due to the .categories bug we discussed
earlier.
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:42-51
+ ValueObjectSP engaged_sp(
+ valobj_sp->GetChildMemberWithName(ConstString("__engaged_"), true));
+
+ if (!engaged_sp)
+return false;
+
+ llvm::StringRef engaged_as_cstring(
davide wrote:
> This is really obscure to somebody who doesn't understand the type
> internally. Can you add a comment explaining the structure? (here or in the
> synthetic)
That is a good point, I am looking at the cppreference page for it and I think
maybe has_value might be an improvement.
https://reviews.llvm.org/D49271
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337758 - Fix Xcode project for unit tests.
Author: gclayton
Date: Mon Jul 23 15:22:46 2018
New Revision: 337758
URL: http://llvm.org/viewvc/llvm-project?rev=337758&view=rev
Log:
Fix Xcode project for unit tests.
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=337758&r1=337757&r2=337758&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Mon Jul 23 15:22:46 2018
@@ -918,7 +918,6 @@
23CB15401D66DA9300EDDDE1 /* TestClangASTContext.cpp in Sources
*/ = {isa = PBXBuildFile; fileRef = 23CB150C1D66CF5600EDDDE1 /*
TestClangASTContext.cpp */; };
9A20572D1F3B8E6600F6C293 /* TestCompletion.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 9A20572B1F3B8E6200F6C293 /* TestCompletion.cpp
*/; };
9A2057171F3B861400F6C293 /* TestDWARFCallFrameInfo.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 9A2057131F3B860D00F6C293 /*
TestDWARFCallFrameInfo.cpp */; };
- 9A2057281F3B8DDB00F6C293 /* TestELFHeader.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 9A2057241F3B8DD200F6C293 /* TestELFHeader.cpp
*/; };
9A2057291F3B8DDB00F6C293 /* TestObjectFileELF.cpp in Sources */
= {isa = PBXBuildFile; fileRef = 9A2057251F3B8DD200F6C293 /*
TestObjectFileELF.cpp */; };
4C719399207D23E300FDF430 /* TestOptionArgParser.cpp in Sources
*/ = {isa = PBXBuildFile; fileRef = 4C719398207D23E300FDF430 /*
TestOptionArgParser.cpp */; };
4CEC86A7204738EB009B37B1 /* TestPPC64InstEmulation.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 4CEC86A6204738EA009B37B1 /*
TestPPC64InstEmulation.cpp */; };
@@ -2974,7 +2973,6 @@
23CB150C1D66CF5600EDDDE1 /* TestClangASTContext.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
path = TestClangASTContext.cpp; sourceTree = ""; };
9A20572B1F3B8E6200F6C293 /* TestCompletion.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
path = TestCompletion.cpp; sourceTree = ""; };
9A2057131F3B860D00F6C293 /* TestDWARFCallFrameInfo.cpp */ =
{isa = PBXFileReference; fileEncoding = 4; lastKnownFileType =
sourcecode.cpp.cpp; path = TestDWARFCallFrameInfo.cpp; sourceTree = "";
};
- 9A2057241F3B8DD200F6C293 /* TestELFHeader.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
name = TestELFHeader.cpp; path = ObjectFile/ELF/TestELFHeader.cpp; sourceTree =
""; };
23CB15051D66CDB400EDDDE1 /* TestModule.c */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path =
TestModule.c; sourceTree = ""; };
23CB15061D66CDB400EDDDE1 /* TestModule.so */ = {isa =
PBXFileReference; lastKnownFileType = file; path = TestModule.so; sourceTree =
""; };
9A2057251F3B8DD200F6C293 /* TestObjectFileELF.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
name = TestObjectFileELF.cpp; path = ObjectFile/ELF/TestObjectFileELF.cpp;
sourceTree = ""; };
@@ -6553,7 +6551,6 @@
9A2057231F3B8DC100F6C293 /* ELF */ = {
isa = PBXGroup;
children = (
- 9A2057241F3B8DD200F6C293 /* TestELFHeader.cpp
*/,
9A2057251F3B8DD200F6C293 /*
TestObjectFileELF.cpp */,
);
name = ELF;
@@ -7411,7 +7408,6 @@
files = (
9A3D43D91F3151C400EB767C /* StatusTest.cpp in
Sources */,
23CB15331D66DA9300EDDDE1 /* GoParserTest.cpp in
Sources */,
- 9A2057281F3B8DDB00F6C293 /* TestELFHeader.cpp
in Sources */,
23CB15341D66DA9300EDDDE1 /*
CPlusPlusLanguageTest.cpp in Sources */,
9A2057381F3B8E7E00F6C293 /* FileSystemTest.cpp
in Sources */,
9A2057201F3B8D2500F6C293 /* UnixSignalsTest.cpp
in Sources */,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. This is good, but please add a comment explaining the type before committing. https://reviews.llvm.org/D49271 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337774 - Change sort-pbxproj.rb to find the project.pbxproj in the
Author: jmolenda
Date: Mon Jul 23 16:34:50 2018
New Revision: 337774
URL: http://llvm.org/viewvc/llvm-project?rev=337774&view=rev
Log:
Change sort-pbxproj.rb to find the project.pbxproj in the
most likely locations. And have it overwrite the original
file with the sorted output.
Modified:
lldb/trunk/scripts/sort-pbxproj.rb
Modified: lldb/trunk/scripts/sort-pbxproj.rb
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/sort-pbxproj.rb?rev=337774&r1=337773&r2=337774&view=diff
==
--- lldb/trunk/scripts/sort-pbxproj.rb (original)
+++ lldb/trunk/scripts/sort-pbxproj.rb Mon Jul 23 16:34:50 2018
@@ -20,11 +20,6 @@
# can lead to merge failures.
segregated_filenames = ["Swift", "repl", "RPC"]
-if !File.exists?("project.pbxproj")
-STDERR.puts "ERROR: project.pbxproj does not exist."
-exit(1)
-end
-
def read_pbxproj(fn)
beginning = Array.new # All lines before "PBXBuildFile section"
files = Array.new # PBXBuildFile section lines -- sort these
@@ -74,7 +69,20 @@ def read_pbxproj(fn)
return beginning, files, middle, refs, ending
end
-beginning, files, middle, refs, ending = read_pbxproj("project.pbxproj")
+xcodeproj_filename = nil
+[ "../lldb.xcodeproj/project.pbxproj", "lldb.xcodeproj/project.pbxproj",
"project.pbxproj" ].each do |ent|
+if File.exists?(ent)
+xcodeproj_filename = ent
+break
+end
+end
+
+if xcodeproj_filename.nil?
+STDERR.puts "Could not find xcode project file to sort."
+exit(1)
+end
+
+beginning, files, middle, refs, ending = read_pbxproj(xcodeproj_filename)
### If we're given a "canonical" project.pbxproj file, get the uuid and
fileref ids for
@@ -236,6 +244,8 @@ end
### output the sorted pbxproj
-[ beginning, files, middle, refs, ending ].each do |arr|
-arr.each {|l| puts l}
+File.open(xcodeproj_filename, 'w') do |outfile|
+[ beginning, files, middle, refs, ending ].each do |arr|
+ arr.each {|l| outfile.puts l}
+end
end
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49708: Added unit test for StreamTee
teemperor created this revision.
Herald added a subscriber: mgorny.
https://reviews.llvm.org/D49708
Files:
lldb.xcodeproj/project.pbxproj
unittests/Utility/CMakeLists.txt
unittests/Utility/StreamTeeTest.cpp
Index: unittests/Utility/StreamTeeTest.cpp
===
--- /dev/null
+++ unittests/Utility/StreamTeeTest.cpp
@@ -0,0 +1,197 @@
+//===-- StreamTeeTest.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/StreamTee.h"
+#include "lldb/Utility/StreamString.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(StreamTeeTest, DefaultConstructor) {
+ // Test the default constructor.
+ StreamTee tee;
+ ASSERT_EQ(0U, tee.GetNumStreams());
+}
+
+TEST(StreamTeeTest, Constructor1Stream) {
+ // Test the constructor for a single stream.
+ lldb::StreamSP s1(std::make_shared());
+ StreamTee tee(s1);
+
+ ASSERT_EQ(1U, tee.GetNumStreams());
+ EXPECT_EQ(s1, tee.GetStreamAtIndex(0U));
+}
+
+TEST(StreamTeeTest, Constructor2Streams) {
+ // Test the constructor for two streams.
+ lldb::StreamSP s1(std::make_shared());
+ lldb::StreamSP s2(std::make_shared());
+ StreamTee tee(s1, s2);
+
+ ASSERT_EQ(2U, tee.GetNumStreams());
+ EXPECT_EQ(s1, tee.GetStreamAtIndex(0U));
+ EXPECT_EQ(s2, tee.GetStreamAtIndex(1U));
+}
+
+TEST(StreamTeeTest, CopyConstructor) {
+ // Test the copy constructor.
+ lldb::StreamSP s1(std::make_shared());
+ lldb::StreamSP s2(std::make_shared());
+ StreamTee tee1(s1, s2);
+ StreamTee tee2(tee1);
+
+ ASSERT_EQ(2U, tee2.GetNumStreams());
+ EXPECT_EQ(s1, tee2.GetStreamAtIndex(0U));
+ EXPECT_EQ(s2, tee2.GetStreamAtIndex(1U));
+}
+
+TEST(StreamTeeTest, Assignment) {
+ // Test the assignment of StreamTee.
+ lldb::StreamSP s1(std::make_shared());
+ lldb::StreamSP s2(std::make_shared());
+ StreamTee tee1(s1, s2);
+ StreamTee tee2 = tee1;
+
+ ASSERT_EQ(2U, tee2.GetNumStreams());
+ EXPECT_EQ(s1, tee2.GetStreamAtIndex(0U));
+ EXPECT_EQ(s2, tee2.GetStreamAtIndex(1U));
+}
+
+TEST(StreamTeeTest, Write) {
+ // Test that write is sent out to all children.
+ auto ss1 = new StreamString();
+ auto ss2 = new StreamString();
+ lldb::StreamSP s1(ss1);
+ lldb::StreamSP s2(ss2);
+ StreamTee tee(s1, s2);
+
+ tee << "foo";
+ tee.Flush();
+
+ ASSERT_EQ(2U, tee.GetNumStreams());
+ EXPECT_EQ("foo", ss1->GetString().str());
+ EXPECT_EQ("foo", ss2->GetString().str());
+
+ tee << "bar";
+ tee.Flush();
+ EXPECT_EQ("foobar", ss1->GetString().str());
+ EXPECT_EQ("foobar", ss2->GetString().str());
+}
+
+namespace {
+ struct FlushTestStream : public Stream {
+unsigned m_flush_count = false;
+void Flush() override {
+ ++m_flush_count;
+}
+size_t Write(const void *src, size_t src_len) override { return src_len; }
+ };
+}
+
+TEST(StreamTeeTest, Flush) {
+ // Check that Flush is distributed to all streams.
+ auto fs1 = new FlushTestStream();
+ auto fs2 = new FlushTestStream();
+ lldb::StreamSP s1(fs1);
+ lldb::StreamSP s2(fs2);
+ StreamTee tee(s1, s2);
+
+ tee << "foo";
+ tee.Flush();
+
+ ASSERT_EQ(2U, tee.GetNumStreams());
+ EXPECT_EQ(1U, fs1->m_flush_count);
+ EXPECT_EQ(1U, fs2->m_flush_count);
+
+ tee << "bar";
+ tee.Flush();
+ EXPECT_EQ(2U, fs1->m_flush_count);
+ EXPECT_EQ(2U, fs2->m_flush_count);
+}
+
+TEST(StreamTeeTest, AppendStream) {
+ // Append new streams to our StreamTee.
+ auto ss1 = new StreamString();
+ auto ss2 = new StreamString();
+ lldb::StreamSP s1(ss1);
+ lldb::StreamSP s2(ss2);
+
+ StreamTee tee;
+
+ ASSERT_EQ(0U, tee.GetNumStreams());
+
+ tee.AppendStream(s1);
+ ASSERT_EQ(1U, tee.GetNumStreams());
+ EXPECT_EQ(s1, tee.GetStreamAtIndex(0U));
+
+ tee.AppendStream(s2);
+ ASSERT_EQ(2U, tee.GetNumStreams());
+ EXPECT_EQ(s1, tee.GetStreamAtIndex(0U));
+ EXPECT_EQ(s2, tee.GetStreamAtIndex(1U));
+}
+
+TEST(StreamTeeTest, GetStreamAtIndexOutOfBounds) {
+ // The index we check for is not in the bounds of the StreamTee.
+ lldb::StreamSP s1(std::make_shared());
+ StreamTee tee(s1);
+
+ ASSERT_EQ(1U, tee.GetNumStreams());
+ EXPECT_EQ(lldb::StreamSP(), tee.GetStreamAtIndex(1));
+}
+
+TEST(StreamTeeTest, GetStreamAtIndexOutOfBoundsEmpty) {
+ // Same as above, but with an empty StreamTee.
+ StreamTee tee;
+ ASSERT_EQ(0U, tee.GetNumStreams());
+ EXPECT_EQ(lldb::StreamSP(), tee.GetStreamAtIndex(0U));
+ EXPECT_EQ(lldb::StreamSP(), tee.GetStreamAtIndex(1U));
+}
+
+TEST(StreamTeeTest, SetStreamAtIndexOverwrite) {
+ // We overwrite an existing stream at a given index.
+ lldb::StreamSP s1(std::make_shared());
+ StreamTee tee(s1);
+
+ ASSERT_EQ(1U, tee.GetNumStreams());
+ EXPECT_EQ(s1, tee.GetStreamAtIndex(0U));
+ EXPECT_EQ(lldb::StreamSP(), tee.GetStreamAtIndex(1U));
[Lldb-commits] [PATCH] D49708: Added unit test for StreamTee
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. lg. You probably don't need pre-commit reviews for adding tests. This is obvious goodness. https://reviews.llvm.org/D49708 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49708: Added unit test for StreamTee
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337778: Added unit test for StreamTee (authored by
teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49708?vs=156926&id=156929#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49708
Files:
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/unittests/Utility/CMakeLists.txt
lldb/trunk/unittests/Utility/StreamTeeTest.cpp
Index: lldb/trunk/unittests/Utility/CMakeLists.txt
===
--- lldb/trunk/unittests/Utility/CMakeLists.txt
+++ lldb/trunk/unittests/Utility/CMakeLists.txt
@@ -13,6 +13,7 @@
LogTest.cpp
NameMatchesTest.cpp
StatusTest.cpp
+ StreamTeeTest.cpp
StringExtractorTest.cpp
StructuredDataTest.cpp
TildeExpressionResolverTest.cpp
Index: lldb/trunk/unittests/Utility/StreamTeeTest.cpp
===
--- lldb/trunk/unittests/Utility/StreamTeeTest.cpp
+++ lldb/trunk/unittests/Utility/StreamTeeTest.cpp
@@ -0,0 +1,197 @@
+//===-- StreamTeeTest.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/StreamTee.h"
+#include "lldb/Utility/StreamString.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(StreamTeeTest, DefaultConstructor) {
+ // Test the default constructor.
+ StreamTee tee;
+ ASSERT_EQ(0U, tee.GetNumStreams());
+}
+
+TEST(StreamTeeTest, Constructor1Stream) {
+ // Test the constructor for a single stream.
+ lldb::StreamSP s1(std::make_shared());
+ StreamTee tee(s1);
+
+ ASSERT_EQ(1U, tee.GetNumStreams());
+ EXPECT_EQ(s1, tee.GetStreamAtIndex(0U));
+}
+
+TEST(StreamTeeTest, Constructor2Streams) {
+ // Test the constructor for two streams.
+ lldb::StreamSP s1(std::make_shared());
+ lldb::StreamSP s2(std::make_shared());
+ StreamTee tee(s1, s2);
+
+ ASSERT_EQ(2U, tee.GetNumStreams());
+ EXPECT_EQ(s1, tee.GetStreamAtIndex(0U));
+ EXPECT_EQ(s2, tee.GetStreamAtIndex(1U));
+}
+
+TEST(StreamTeeTest, CopyConstructor) {
+ // Test the copy constructor.
+ lldb::StreamSP s1(std::make_shared());
+ lldb::StreamSP s2(std::make_shared());
+ StreamTee tee1(s1, s2);
+ StreamTee tee2(tee1);
+
+ ASSERT_EQ(2U, tee2.GetNumStreams());
+ EXPECT_EQ(s1, tee2.GetStreamAtIndex(0U));
+ EXPECT_EQ(s2, tee2.GetStreamAtIndex(1U));
+}
+
+TEST(StreamTeeTest, Assignment) {
+ // Test the assignment of StreamTee.
+ lldb::StreamSP s1(std::make_shared());
+ lldb::StreamSP s2(std::make_shared());
+ StreamTee tee1(s1, s2);
+ StreamTee tee2 = tee1;
+
+ ASSERT_EQ(2U, tee2.GetNumStreams());
+ EXPECT_EQ(s1, tee2.GetStreamAtIndex(0U));
+ EXPECT_EQ(s2, tee2.GetStreamAtIndex(1U));
+}
+
+TEST(StreamTeeTest, Write) {
+ // Test that write is sent out to all children.
+ auto ss1 = new StreamString();
+ auto ss2 = new StreamString();
+ lldb::StreamSP s1(ss1);
+ lldb::StreamSP s2(ss2);
+ StreamTee tee(s1, s2);
+
+ tee << "foo";
+ tee.Flush();
+
+ ASSERT_EQ(2U, tee.GetNumStreams());
+ EXPECT_EQ("foo", ss1->GetString().str());
+ EXPECT_EQ("foo", ss2->GetString().str());
+
+ tee << "bar";
+ tee.Flush();
+ EXPECT_EQ("foobar", ss1->GetString().str());
+ EXPECT_EQ("foobar", ss2->GetString().str());
+}
+
+namespace {
+ struct FlushTestStream : public Stream {
+unsigned m_flush_count = false;
+void Flush() override {
+ ++m_flush_count;
+}
+size_t Write(const void *src, size_t src_len) override { return src_len; }
+ };
+}
+
+TEST(StreamTeeTest, Flush) {
+ // Check that Flush is distributed to all streams.
+ auto fs1 = new FlushTestStream();
+ auto fs2 = new FlushTestStream();
+ lldb::StreamSP s1(fs1);
+ lldb::StreamSP s2(fs2);
+ StreamTee tee(s1, s2);
+
+ tee << "foo";
+ tee.Flush();
+
+ ASSERT_EQ(2U, tee.GetNumStreams());
+ EXPECT_EQ(1U, fs1->m_flush_count);
+ EXPECT_EQ(1U, fs2->m_flush_count);
+
+ tee << "bar";
+ tee.Flush();
+ EXPECT_EQ(2U, fs1->m_flush_count);
+ EXPECT_EQ(2U, fs2->m_flush_count);
+}
+
+TEST(StreamTeeTest, AppendStream) {
+ // Append new streams to our StreamTee.
+ auto ss1 = new StreamString();
+ auto ss2 = new StreamString();
+ lldb::StreamSP s1(ss1);
+ lldb::StreamSP s2(ss2);
+
+ StreamTee tee;
+
+ ASSERT_EQ(0U, tee.GetNumStreams());
+
+ tee.AppendStream(s1);
+ ASSERT_EQ(1U, tee.GetNumStreams());
+ EXPECT_EQ(s1, tee.GetStreamAtIndex(0U));
+
+ tee.AppendStream(s2);
+ ASSERT_EQ(2U, tee.GetNumStreams());
+ EXPECT_EQ(s1, tee.GetStreamAtIndex(0U));
+ EXPECT_EQ(s2, tee.GetStreamAtIndex(1U));
+}
+
+TEST(StreamTeeTest, GetStreamAtIndexOutOfBounds) {
+ // The index we check for is not in the bounds of the StreamTee.
+ lldb::StreamSP s1(s
[Lldb-commits] [lldb] r337778 - Added unit test for StreamTee
Author: teemperor
Date: Mon Jul 23 17:01:32 2018
New Revision: 337778
URL: http://llvm.org/viewvc/llvm-project?rev=337778&view=rev
Log:
Added unit test for StreamTee
Reviewers: davide
Reviewed By: davide
Subscribers: davide, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D49708
Added:
lldb/trunk/unittests/Utility/StreamTeeTest.cpp
Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/unittests/Utility/CMakeLists.txt
Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=337778&r1=33&r2=337778&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Mon Jul 23 17:01:32 2018
@@ -867,6 +867,7 @@
2689004F13353E0400698AC0 /* StreamFile.cpp in Sources */ = {isa
= PBXBuildFile; fileRef = 26BC7E9210F1B85900F91463 /* StreamFile.cpp */; };
AFC2DCF91E6E318000283714 /* StreamGDBRemote.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = AFC2DCF81E6E318000283714 /* StreamGDBRemote.cpp
*/; };
26764CA21E48F547008D3573 /* StreamString.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 26764CA11E48F547008D3573 /* StreamString.cpp */;
};
+ 58EAC73F2106A07B0029571E /* StreamTeeTest.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 58EAC73D2106A0740029571E /* StreamTeeTest.cpp
*/; };
33E5E8471A674FB60024ED68 /* StringConvert.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 33E5E8411A672A240024ED68 /* StringConvert.cpp
*/; };
268903353E8200698AC0 /* StringExtractor.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 2660D9F611922A1300958FBD /* StringExtractor.cpp
*/; };
2689011213353E8200698AC0 /* StringExtractorGDBRemote.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 2676A093119C93C8008A98EF /*
StringExtractorGDBRemote.cpp */; };
@@ -2885,6 +2886,7 @@
26764CA11E48F547008D3573 /* StreamString.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
name = StreamString.cpp; path = source/Utility/StreamString.cpp; sourceTree =
""; };
26764CA31E48F550008D3573 /* StreamString.h */ = {isa =
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = StreamString.h;
path = include/lldb/Utility/StreamString.h; sourceTree = ""; };
26764CA41E48F566008D3573 /* StreamTee.h */ = {isa =
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = StreamTee.h; path
= include/lldb/Utility/StreamTee.h; sourceTree = ""; };
+ 58EAC73D2106A0740029571E /* StreamTeeTest.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
path = StreamTeeTest.cpp; sourceTree = ""; };
33E5E8411A672A240024ED68 /* StringConvert.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
path = StringConvert.cpp; sourceTree = ""; };
33E5E8451A6736D30024ED68 /* StringConvert.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
StringConvert.h; path = include/lldb/Host/StringConvert.h; sourceTree =
SOURCE_ROOT; };
2660D9F611922A1300958FBD /* StringExtractor.cpp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp;
name = StringExtractor.cpp; path = source/Utility/StringExtractor.cpp;
sourceTree = ""; };
@@ -3495,6 +3497,7 @@
2321F9421BDD343A00BA9A93 /* Utility */ = {
isa = PBXGroup;
children = (
+ 58EAC73D2106A0740029571E /* StreamTeeTest.cpp
*/,
7F94D7172040A13A006EE3EA /* CleanUpTest.cpp */,
23E2E5161D903689006F38BB /* ArchSpecTest.cpp */,
9A3D43C81F3150D200EB767C /* ConstStringTest.cpp
*/,
@@ -7426,6 +7429,7 @@
23CB15371D66DA9300EDDDE1 /* PythonTestSuite.cpp
in Sources */,
23E2E5321D903832006F38BB /*
BreakpointIDTest.cpp in Sources */,
4CEC86A7204738EB009B37B1 /*
TestPPC64InstEmulation.cpp in Sources */,
+ 58EAC73F2106A07B0029571E /* StreamTeeTest.cpp
in Sources */,
23CB15381D66DA9300EDDDE1 /*
PythonExceptionStateTests.cpp in Sources */,
9A3D43D81F3151C400EB767C /* NameMatchesTest.cpp
in Sources */,
23CB15391D66DA9300EDDDE1 /*
DataExtractorTest.cpp in Sources */,
Modified: lldb/trunk/unittests/Utility/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CMakeLists.txt?rev=337778&r1=33&r2=337778&view=diff
=
[Lldb-commits] [PATCH] D49713: Replace StreamTee's recursive_mutex with a normal mutex.
teemperor created this revision.
teemperor added a reviewer: davide.
StreamTee doesn't need a recursive_mutex so let's use a normal mutex instead.
Also gets rid of some micro-optimizations.
Still passes the test suite and the newly added unit test.
https://reviews.llvm.org/D49713
Files:
include/lldb/Utility/StreamTee.h
Index: include/lldb/Utility/StreamTee.h
===
--- include/lldb/Utility/StreamTee.h
+++ include/lldb/Utility/StreamTee.h
@@ -41,24 +41,24 @@
StreamTee(const StreamTee &rhs)
: Stream(rhs), m_streams_mutex(), m_streams() {
// Don't copy until we lock down "rhs"
-std::lock_guard guard(rhs.m_streams_mutex);
+std::lock_guard guard(rhs.m_streams_mutex);
m_streams = rhs.m_streams;
}
~StreamTee() override {}
StreamTee &operator=(const StreamTee &rhs) {
if (this != &rhs) {
Stream::operator=(rhs);
- std::lock_guard lhs_locker(m_streams_mutex);
- std::lock_guard rhs_locker(rhs.m_streams_mutex);
+ std::lock_guard lhs_locker(m_streams_mutex);
+ std::lock_guard rhs_locker(rhs.m_streams_mutex);
m_streams = rhs.m_streams;
}
return *this;
}
void Flush() override {
-std::lock_guard guard(m_streams_mutex);
+std::lock_guard guard(m_streams_mutex);
collection::iterator pos, end;
for (pos = m_streams.begin(), end = m_streams.end(); pos != end; ++pos) {
// Allow for our collection to contain NULL streams. This allows the
@@ -71,7 +71,7 @@
}
size_t Write(const void *s, size_t length) override {
-std::lock_guard guard(m_streams_mutex);
+std::lock_guard guard(m_streams_mutex);
if (m_streams.empty())
return 0;
@@ -94,31 +94,27 @@
}
size_t AppendStream(const lldb::StreamSP &stream_sp) {
+std::lock_guard guard(m_streams_mutex);
size_t new_idx = m_streams.size();
-std::lock_guard guard(m_streams_mutex);
m_streams.push_back(stream_sp);
return new_idx;
}
size_t GetNumStreams() const {
-size_t result = 0;
-{
- std::lock_guard guard(m_streams_mutex);
- result = m_streams.size();
-}
-return result;
+std::lock_guard guard(m_streams_mutex);
+return m_streams.size();
}
lldb::StreamSP GetStreamAtIndex(uint32_t idx) {
lldb::StreamSP stream_sp;
-std::lock_guard guard(m_streams_mutex);
+std::lock_guard guard(m_streams_mutex);
if (idx < m_streams.size())
stream_sp = m_streams[idx];
return stream_sp;
}
void SetStreamAtIndex(uint32_t idx, const lldb::StreamSP &stream_sp) {
-std::lock_guard guard(m_streams_mutex);
+std::lock_guard guard(m_streams_mutex);
// Resize our stream vector as necessary to fit as many streams as needed.
// This also allows this class to be used with hard coded indexes that can
// be used contain many streams, not all of which are valid.
@@ -129,7 +125,7 @@
protected:
typedef std::vector collection;
- mutable std::recursive_mutex m_streams_mutex;
+ mutable std::mutex m_streams_mutex;
collection m_streams;
};
Index: include/lldb/Utility/StreamTee.h
===
--- include/lldb/Utility/StreamTee.h
+++ include/lldb/Utility/StreamTee.h
@@ -41,24 +41,24 @@
StreamTee(const StreamTee &rhs)
: Stream(rhs), m_streams_mutex(), m_streams() {
// Don't copy until we lock down "rhs"
-std::lock_guard guard(rhs.m_streams_mutex);
+std::lock_guard guard(rhs.m_streams_mutex);
m_streams = rhs.m_streams;
}
~StreamTee() override {}
StreamTee &operator=(const StreamTee &rhs) {
if (this != &rhs) {
Stream::operator=(rhs);
- std::lock_guard lhs_locker(m_streams_mutex);
- std::lock_guard rhs_locker(rhs.m_streams_mutex);
+ std::lock_guard lhs_locker(m_streams_mutex);
+ std::lock_guard rhs_locker(rhs.m_streams_mutex);
m_streams = rhs.m_streams;
}
return *this;
}
void Flush() override {
-std::lock_guard guard(m_streams_mutex);
+std::lock_guard guard(m_streams_mutex);
collection::iterator pos, end;
for (pos = m_streams.begin(), end = m_streams.end(); pos != end; ++pos) {
// Allow for our collection to contain NULL streams. This allows the
@@ -71,7 +71,7 @@
}
size_t Write(const void *s, size_t length) override {
-std::lock_guard guard(m_streams_mutex);
+std::lock_guard guard(m_streams_mutex);
if (m_streams.empty())
return 0;
@@ -94,31 +94,27 @@
}
size_t AppendStream(const lldb::StreamSP &stream_sp) {
+std::lock_guard guard(m_streams_mutex);
size_t new_idx = m_streams.size();
-std::lock_guard guard(m_streams_mutex);
m_streams.push_back(stream_sp);
return new_idx;
}
size_t GetNumStreams() const {
-size_t result = 0;
-{
- std::lock_guard guard(m_streams_mutex);
- result = m_streams.size();
-}
-return resul
