[Lldb-commits] [lldb] r316982 - Split makefile for TestTopLevelExprs

2017-10-31 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 31 03:01:30 2017
New Revision: 316982

URL: http://llvm.org/viewvc/llvm-project?rev=316982&view=rev
Log:
Split makefile for TestTopLevelExprs

this test was using a single makefile to build two executables. This
setup, although not supported by Makefile.rules, happened to
work in most configurations, except when building with the android ndk
r16.

Here I move the building of the second executable to a separate
makefile, which is the solution other tests use for multiple targets.

Added:

lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/dummy.mk
Modified:

lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/Makefile

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/Makefile?rev=316982&r1=316981&r2=316982&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/Makefile 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/Makefile 
Tue Oct 31 03:01:30 2017
@@ -1,12 +1,13 @@
 LEVEL = ../../make
 
-default: a.out dummy
-
 CXX_SOURCES := main.cpp test.cpp
 
-dummy: dummy.cpp
+include $(LEVEL)/Makefile.rules
 
-clean::
-   rm -rf dummy dummy.dSYM
+a.out: dummy
 
-include $(LEVEL)/Makefile.rules
+dummy:
+   $(MAKE) -f dummy.mk
+
+clean::
+   $(MAKE) -f dummy.mk clean

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/dummy.mk
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/dummy.mk?rev=316982&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/dummy.mk 
(added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/dummy.mk 
Tue Oct 31 03:01:30 2017
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+CXX_SOURCES := dummy.cpp
+EXE := dummy
+
+include $(LEVEL)/Makefile.rules


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


[Lldb-commits] [lldb] r316985 - Android.rules: build with "unified android headers"

2017-10-31 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 31 03:33:03 2017
New Revision: 316985

URL: http://llvm.org/viewvc/llvm-project?rev=316985&view=rev
Log:
Android.rules: build with "unified android headers"

Unified headers will be the only way to build applications in NDK r16,
and it also works with NDK r15.

This also bumps the minimum supported android version to 16.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/make/Android.rules

Modified: lldb/trunk/packages/Python/lldbsuite/test/make/Android.rules
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/make/Android.rules?rev=316985&r1=316984&r2=316985&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/make/Android.rules (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/make/Android.rules Tue Oct 31 
03:33:03 2017
@@ -8,7 +8,7 @@ else ifeq "$(ARCH)" "i386"
API_LEVEL := 17
 else
# lowest supported 32-bit API level
-   API_LEVEL := 9
+   API_LEVEL := 16
 endif
 
 ifeq "$(ARCH)" "arm"
@@ -18,8 +18,8 @@ ifeq "$(ARCH)" "arm"
ARCH_CFLAGS += -march=armv7-a -mfloat-abi=softfp -mfpu=vfpv3-d16 -marm
 else ifeq "$(ARCH)" "aarch64"
SYSROOT_ARCH := arm64
-   TRIPLE := aarch64-none-linux-android
STL_ARCH := arm64-v8a
+   TRIPLE := aarch64-none-linux-android
 else ifeq "$(ARCH)" "i386"
SYSROOT_ARCH := x86
STL_ARCH := x86
@@ -70,7 +70,9 @@ ifeq "$(findstring clang,$(CC))" "clang"
ARCH_LDFLAGS += -target $(TRIPLE) -gcc-toolchain $(GCC_TOOLCHAIN)
 endif
 
-ARCH_CFLAGS += 
--sysroot=$(NDK_ROOT)/platforms/android-$(API_LEVEL)/arch-$(SYSROOT_ARCH)
+ARCH_CFLAGS += --sysroot=$(NDK_ROOT)/sysroot \
+   -isystem $(NDK_ROOT)/sysroot/usr/include/$(TOOL_PREFIX) \
+   -D__ANDROID_API__=$(API_LEVEL)
 ARCH_LDFLAGS += 
--sysroot=$(NDK_ROOT)/platforms/android-$(API_LEVEL)/arch-$(SYSROOT_ARCH) -lm
 
 ifeq (1,$(USE_LIBCPP))


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


[Lldb-commits] [lldb] r316987 - Invert ArchSpec<->Platform dependency

2017-10-31 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 31 03:56:03 2017
New Revision: 316987

URL: http://llvm.org/viewvc/llvm-project?rev=316987&view=rev
Log:
Invert ArchSpec<->Platform dependency

Summary:
ArchSpec::SetTriple was taking a Platform as an argument, and used it to
fill in missing pieces of the specified triple. I invert the dependency
by moving this code to other classes. For this purpose, I've created
three new functions.
- HostInfo::GetAugmentedArchSpec: fills in the triple using the host
  platform (this used to be implemented by passing a null platform
  pointer). By putting this code in the Host module, we can provide a
  way to anyone who does not have a platform instance (lldb-server) an
  easy way to get Host data.
- Platform::GetAugmentedArchSpec: if you have a platform instance, you
  can call this to let it fill in the triple.
- static Platform::GetAugmentedArchSpec: implements the "if platform ==
  0 then use_host() else use_platform()" part.

Reviewers: zturner, jingham, clayborg

Subscribers: mgorny, javed.absar, lldb-commits

Differential Revision: https://reviews.llvm.org/D39387

Added:
lldb/trunk/unittests/Host/HostInfoTest.cpp
Modified:
lldb/trunk/include/lldb/Core/ArchSpec.h
lldb/trunk/include/lldb/Host/HostInfoBase.h
lldb/trunk/include/lldb/Target/Platform.h
lldb/trunk/source/API/SBDebugger.cpp
lldb/trunk/source/API/SBInstruction.cpp
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/Commands/CommandObjectDisassemble.cpp
lldb/trunk/source/Commands/CommandObjectPlatform.cpp
lldb/trunk/source/Core/ArchSpec.cpp
lldb/trunk/source/Host/common/HostInfoBase.cpp
lldb/trunk/source/Interpreter/OptionGroupArchitecture.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/trunk/source/Target/Platform.cpp
lldb/trunk/source/Target/Process.cpp
lldb/trunk/unittests/Host/CMakeLists.txt
lldb/trunk/unittests/UnwindAssembly/InstEmulation/TestArm64InstEmulation.cpp
lldb/trunk/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp

Modified: lldb/trunk/include/lldb/Core/ArchSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ArchSpec.h?rev=316987&r1=316986&r2=316987&view=diff
==
--- lldb/trunk/include/lldb/Core/ArchSpec.h (original)
+++ lldb/trunk/include/lldb/Core/ArchSpec.h Tue Oct 31 03:56:03 2017
@@ -263,8 +263,6 @@ public:
   explicit ArchSpec(const llvm::Triple &triple);
   explicit ArchSpec(const char *triple_cstr);
   explicit ArchSpec(llvm::StringRef triple_str);
-  ArchSpec(const char *triple_cstr, Platform *platform);
-  ArchSpec(llvm::StringRef triple_str, Platform *platform);
   //--
   /// Constructor over architecture name.
   ///
@@ -288,6 +286,12 @@ public:
   //--
   const ArchSpec &operator=(const ArchSpec &rhs);
 
+  //---
+  /// Returns true if the OS, vendor and environment fields of the triple are
+  /// unset. The triple is expected to be normalized (llvm::Triple::normalize).
+  //---
+  static bool ContainsOnlyArch(const llvm::Triple &normalized_triple);
+
   static size_t AutoComplete(llvm::StringRef name, StringList &matches);
 
   //--
@@ -520,10 +524,6 @@ public:
   bool SetTriple(const llvm::Triple &triple);
 
   bool SetTriple(llvm::StringRef triple_str);
-  bool SetTriple(llvm::StringRef triple_str, Platform *platform);
-
-  bool SetTriple(const char *triple_cstr);
-  bool SetTriple(const char *triple_cstr, Platform *platform);
 
   //--
   /// Returns the default endianness of the architecture.

Modified: lldb/trunk/include/lldb/Host/HostInfoBase.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/HostInfoBase.h?rev=316987&r1=316986&r2=316987&view=diff
==
--- lldb/trunk/include/lldb/Host/HostInfoBase.h (original)
+++ lldb/trunk/include/lldb/Host/HostInfoBase.h Tue Oct 31 03:56:03 2017
@@ -81,6 +81,13 @@ public:
   //--
   static bool GetLLDBPath(lldb::PathType type, FileSpec &file_spec);
 
+  //---
+  /// If the triple does not specify the vendor, os, and environment parts, we
+  /// "augment" these using information from the host and return the resulting
+  /// ArchSpec object.
+  //---
+  static ArchSpec GetAugmentedArchSpec(llvm::StringRef triple);
+
 protected:
   stat

[Lldb-commits] [PATCH] D39387: Invert ArchSpec<->Platform dependency

2017-10-31 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316987: Invert ArchSpec<->Platform dependency (authored by 
labath).

Changed prior to commit:
  https://reviews.llvm.org/D39387?vs=120714&id=120957#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39387

Files:
  lldb/trunk/include/lldb/Core/ArchSpec.h
  lldb/trunk/include/lldb/Host/HostInfoBase.h
  lldb/trunk/include/lldb/Target/Platform.h
  lldb/trunk/source/API/SBDebugger.cpp
  lldb/trunk/source/API/SBInstruction.cpp
  lldb/trunk/source/API/SBTarget.cpp
  lldb/trunk/source/Commands/CommandObjectDisassemble.cpp
  lldb/trunk/source/Commands/CommandObjectPlatform.cpp
  lldb/trunk/source/Core/ArchSpec.cpp
  lldb/trunk/source/Host/common/HostInfoBase.cpp
  lldb/trunk/source/Interpreter/OptionGroupArchitecture.cpp
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/trunk/source/Target/Platform.cpp
  lldb/trunk/source/Target/Process.cpp
  lldb/trunk/unittests/Host/CMakeLists.txt
  lldb/trunk/unittests/Host/HostInfoTest.cpp
  lldb/trunk/unittests/UnwindAssembly/InstEmulation/TestArm64InstEmulation.cpp
  lldb/trunk/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp

Index: lldb/trunk/unittests/Host/CMakeLists.txt
===
--- lldb/trunk/unittests/Host/CMakeLists.txt
+++ lldb/trunk/unittests/Host/CMakeLists.txt
@@ -1,6 +1,7 @@
 set (FILES
   FileSpecTest.cpp
   FileSystemTest.cpp
+  HostInfoTest.cpp
   HostTest.cpp
   MainLoopTest.cpp
   SocketAddressTest.cpp
Index: lldb/trunk/unittests/Host/HostInfoTest.cpp
===
--- lldb/trunk/unittests/Host/HostInfoTest.cpp
+++ lldb/trunk/unittests/Host/HostInfoTest.cpp
@@ -0,0 +1,40 @@
+//===-- HostTest.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/Host/HostInfo.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace llvm;
+
+namespace {
+class HostInfoTest: public ::testing::Test {
+  public:
+void SetUp() override { HostInfo::Initialize(); }
+void TearDown() override { HostInfo::Terminate(); }
+};
+}
+
+TEST_F(HostInfoTest, GetAugmentedArchSpec) {
+  // Fully specified triple should not be changed.
+  ArchSpec spec = HostInfo::GetAugmentedArchSpec("x86_64-pc-linux-gnu");
+  EXPECT_EQ(spec.GetTriple().getTriple(), "x86_64-pc-linux-gnu");
+
+  // Same goes if we specify at least one of (os, vendor, env).
+  spec = HostInfo::GetAugmentedArchSpec("x86_64-pc");
+  EXPECT_EQ(spec.GetTriple().getTriple(), "x86_64-pc");
+
+  // But if we specify only an arch, we should fill in the rest from the host.
+  spec = HostInfo::GetAugmentedArchSpec("x86_64");
+  Triple triple(sys::getDefaultTargetTriple());
+  EXPECT_EQ(spec.GetTriple().getArch(), Triple::x86_64);
+  EXPECT_EQ(spec.GetTriple().getOS(), triple.getOS());
+  EXPECT_EQ(spec.GetTriple().getVendor(), triple.getVendor());
+  EXPECT_EQ(spec.GetTriple().getEnvironment(), triple.getEnvironment());
+}
Index: lldb/trunk/unittests/UnwindAssembly/InstEmulation/TestArm64InstEmulation.cpp
===
--- lldb/trunk/unittests/UnwindAssembly/InstEmulation/TestArm64InstEmulation.cpp
+++ lldb/trunk/unittests/UnwindAssembly/InstEmulation/TestArm64InstEmulation.cpp
@@ -55,7 +55,7 @@
 }
 
 TEST_F(TestArm64InstEmulation, TestSimpleDarwinFunction) {
-  ArchSpec arch("arm64-apple-ios10", nullptr);
+  ArchSpec arch("arm64-apple-ios10");
   UnwindAssemblyInstEmulation *engine =
   static_cast(
   UnwindAssemblyInstEmulation::CreateInstance(arch));
@@ -151,7 +151,7 @@
 }
 
 TEST_F(TestArm64InstEmulation, TestMediumDarwinFunction) {
-  ArchSpec arch("arm64-apple-ios10", nullptr);
+  ArchSpec arch("arm64-apple-ios10");
   UnwindAssemblyInstEmulation *engine =
   static_cast(
   UnwindAssemblyInstEmulation::CreateInstance(arch));
@@ -313,7 +313,7 @@
 }
 
 TEST_F(TestArm64InstEmulation, TestFramelessThreeEpilogueFunction) {
-  ArchSpec arch("arm64-apple-ios10", nullptr);
+  ArchSpec arch("arm64-apple-ios10");
   UnwindAssemblyInstEmulation *engine =
   static_cast(
   UnwindAssemblyInstEmulation::CreateInstance(arch));
@@ -408,7 +408,7 @@
 }
 
 TEST_F(TestArm64InstEmulation, TestRegisterSavedTwice) {
-  ArchSpec arch("arm64-apple-ios10", nullptr);
+  ArchSpec arch("arm64-apple-ios10");
   UnwindAssemblyInstEmulation *engine =
   static_cast(
   UnwindAssemblyInstEmulation::CreateInstance(arch));
@@ -510,7 +510,7 @@
 }
 
 TEST_F(TestArm64InstEmulation, TestRegisterDoubleSpills) {
-  ArchSpec arch("arm64-apple-ios10", nullptr);
+  ArchSpec arch("arm64-apple

[Lldb-commits] [lldb] r316990 - Fix mac build broken in r316987

2017-10-31 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 31 04:48:33 2017
New Revision: 316990

URL: http://llvm.org/viewvc/llvm-project?rev=316990&view=rev
Log:
Fix mac build broken in r316987

Forgot one occurence of ArchSpec::SetTriple in mac-specific code.

Modified:
lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp

Modified: lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp?rev=316990&r1=316989&r2=316990&view=diff
==
--- lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp (original)
+++ lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp Tue Oct 31 
04:48:33 2017
@@ -468,7 +468,7 @@ Status ProcessMachCore::DoLoadCore() {
   // it to match the core file which is always single arch.
   ArchSpec arch(m_core_module_sp->GetArchitecture());
   if (arch.GetCore() == ArchSpec::eCore_x86_32_i486) {
-arch.SetTriple("i386", GetTarget().GetPlatform().get());
+arch = Platform::GetAugmentedArchSpec(GetTarget().GetPlatform().get(), 
"i386");
   }
   if (arch.IsValid())
 GetTarget().SetArchitecture(arch);


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


[Lldb-commits] [PATCH] D35556: Add data formatter for libc++'s forward_list

2017-10-31 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks.

By the way, I have a couple of other patches lined up that you probably did not 
notice:
https://reviews.llvm.org/D35666
https://reviews.llvm.org/D35615
https://reviews.llvm.org/D35305


https://reviews.llvm.org/D35556



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


[Lldb-commits] [lldb] r316992 - Add data formatter for libc++'s forward_list

2017-10-31 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 31 05:27:43 2017
New Revision: 316992

URL: http://llvm.org/viewvc/llvm-project?rev=316992&view=rev
Log:
Add data formatter for libc++'s forward_list

Summary:
This adds a data formatter for the implementation of forward_list in
libc++. I've refactored the existing std::list data formatter a bit to
enable more sharing of code (mainly the loop detection stuff).

Reviewers: jingham, EricWF

Subscribers: srhines, eugene, lldb-commits

Differential Revision: https://reviews.llvm.org/D35556

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
Modified:
lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h
lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxList.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile?rev=316992&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile
 Tue Oct 31 05:27:43 2017
@@ -0,0 +1,6 @@
+LEVEL = ../../../../../make
+
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+include $(LEVEL)/Makefile.rules

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py?rev=316992&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
 Tue Oct 31 05:27:43 2017
@@ -0,0 +1,53 @@
+"""
+Test lldb data formatter subsystem.
+"""
+
+from __future__ import print_function
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestDataFormatterLibcxxForwardList(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+self.line = line_number('main.cpp', '// break here')
+ns = 'ndk' if lldbplatformutil.target_is_android() else ''
+self.namespace = 'std::__' + ns + '1'
+
+@add_test_categories(["libc++"])
+def test(self):
+"""Test that std::forward_list is displayed correctly"""
+self.build()
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+forward_list = self.namespace + '::forward_list'
+self.expect("frame variable empty",
+substrs=[forward_list,
+ 'size=0',
+ '{}'])
+
+self.expect("frame variable one_elt",
+substrs=[forward_list,
+ 'size=1',
+ '{',
+ '[0] = 47',
+ '}'])
+
+self.expect("frame variable five_elts",
+substrs=[forward_list,
+ 'size=5',
+ '{',
+ '[0] = 1',
+ '[1] = 22',
+ '[2] = 333',
+ '[3] = ',
+ '[4] = 5',
+ '}'])

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp?rev=316992&view=auto
==
--- 
lldb/trun

[Lldb-commits] [lldb] r316993 - Increase AdbClient read timeout

2017-10-31 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 31 05:27:46 2017
New Revision: 316993

URL: http://llvm.org/viewvc/llvm-project?rev=316993&view=rev
Log:
Increase AdbClient read timeout

The previous value was not sufficient for Pixel 2 phones. One would have
hoped that the newer phones are faster, but that does not seem to be the
case here.

Modified:
lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp

Modified: lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp?rev=316993&r1=316992&r2=316993&view=diff
==
--- lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp Tue Oct 31 
05:27:46 2017
@@ -46,7 +46,7 @@ using namespace std::chrono;
 
 namespace {
 
-const seconds kReadTimeout(12);
+const seconds kReadTimeout(20);
 const char *kOKAY = "OKAY";
 const char *kFAIL = "FAIL";
 const char *kDATA = "DATA";


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


[Lldb-commits] [PATCH] D35556: Add data formatter for libc++'s forward_list

2017-10-31 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316992: Add data formatter for libc++'s forward_list 
(authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D35556?vs=120491&id=120961#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35556

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxList.cpp

Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../../../../make
+
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+include $(LEVEL)/Makefile.rules
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+  std::forward_list empty{}, one_elt{47}, five_elts{1,22,333,,5};
+  return 0; // break here
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
@@ -0,0 +1,53 @@
+"""
+Test lldb data formatter subsystem.
+"""
+
+from __future__ import print_function
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestDataFormatterLibcxxForwardList(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+self.line = line_number('main.cpp', '// break here')
+ns = 'ndk' if lldbplatformutil.target_is_android() else ''
+self.namespace = 'std::__' + ns + '1'
+
+@add_test_categories(["libc++"])
+def test(self):
+"""Test that std::forward_list is displayed correctly"""
+self.build()
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+forward_list = self.namespace + '::forward_list'
+self.expect("frame variable empty",
+substrs=[forward_list,
+ 'size=0',
+ '{}'])
+
+self.expect("frame variable one_elt",
+substrs=[forward_list,
+ 'size=1',
+ '{',
+ '[0] = 47',
+ '}'])
+
+self.expect("frame variable five_elts",
+substrs=[forward_list,
+ 'size=5',
+ '{',
+ '[0] = 1',
+ '[1] = 22',
+ '[2] = 333',
+ '[3] = ',
+ '[4] = 5',
+ '}'])
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -428,6 +428,12 @@
   true);
   AddCXXSynthetic(
   cpp_category_sp,
+  lldb_private::formatters::LibcxxStdForwardListSyntheticFrontEndCreator,
+  "libc++ std::forward_list synthetic children",
+  ConstString("^std::__(ndk)?1::forward_list<.+>(( )?&)?$"),
+  stl_synth_flags, true);
+  AddCXXSynthetic(
+  cpp_category_sp,
   lldb_private::f

[Lldb-commits] [lldb] r316997 - Fix LLVM_LINK_LLVM_DYLIB build (pr35053)

2017-10-31 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 31 06:23:19 2017
New Revision: 316997

URL: http://llvm.org/viewvc/llvm-project?rev=316997&view=rev
Log:
Fix LLVM_LINK_LLVM_DYLIB build (pr35053)

Summary:
r316368 broke this build when it introduced a reference to a pthread
function to the Utility module. This caused cmake to generate an
incorrect link line (wrong order of libs) because it did not see the
dependency from Utility to the system libraries. Instead these libraries
were being manually added to each final target.

This changes moves the dependency management from the individual targets
to the lldbUtility module, which is consistent with how llvm does it.
The final targets will pick up these libraries as they will be a part of
the link interface of the module.

Technically, some of these dependencies could go into the host module,
as that's where most of the os-specific code is, but I did not try to
investigate which ones.

Reviewers: zturner, sylvestre.ledru

Subscribers: lldb-commits, mgorny

Differential Revision: https://reviews.llvm.org/D39246

Removed:
lldb/trunk/cmake/LLDBDependencies.cmake
Modified:
lldb/trunk/scripts/CMakeLists.txt
lldb/trunk/source/API/CMakeLists.txt
lldb/trunk/source/Utility/CMakeLists.txt
lldb/trunk/tools/argdumper/CMakeLists.txt
lldb/trunk/tools/driver/CMakeLists.txt
lldb/trunk/tools/intel-features/CMakeLists.txt
lldb/trunk/tools/lldb-server/CMakeLists.txt
lldb/trunk/unittests/CMakeLists.txt

Removed: lldb/trunk/cmake/LLDBDependencies.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/LLDBDependencies.cmake?rev=316996&view=auto
==
--- lldb/trunk/cmake/LLDBDependencies.cmake (original)
+++ lldb/trunk/cmake/LLDBDependencies.cmake (removed)
@@ -1,52 +0,0 @@
-set(LLDB_SYSTEM_LIBS)
-
-# Windows-only libraries
-if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
-  list(APPEND LLDB_SYSTEM_LIBS
-ws2_32
-rpcrt4
-)
-endif ()
-
-if (NOT LLDB_DISABLE_LIBEDIT)
-  list(APPEND LLDB_SYSTEM_LIBS edit)
-endif()
-if (NOT LLDB_DISABLE_CURSES)
-  list(APPEND LLDB_SYSTEM_LIBS ${CURSES_LIBRARIES})
-  if(LLVM_ENABLE_TERMINFO AND HAVE_TERMINFO)
-list(APPEND LLDB_SYSTEM_LIBS ${TERMINFO_LIBS})
-  endif()
-endif()
-
-if (NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB )
-list(APPEND LLDB_SYSTEM_LIBS atomic)
-endif()
-
-list(APPEND LLDB_SYSTEM_LIBS ${Backtrace_LIBRARY})
-
-if (NOT LLDB_DISABLE_PYTHON AND NOT LLVM_BUILD_STATIC)
-  list(APPEND LLDB_SYSTEM_LIBS ${PYTHON_LIBRARIES})
-endif()
-
-list(APPEND LLDB_SYSTEM_LIBS ${system_libs})
-
-if (LLVM_BUILD_STATIC)
-  if (NOT LLDB_DISABLE_PYTHON)
-list(APPEND LLDB_SYSTEM_LIBS python2.7 util)
-  endif()
-  if (NOT LLDB_DISABLE_CURSES)
-list(APPEND LLDB_SYSTEM_LIBS gpm)
-  endif()
-endif()
-
-if ( NOT LLDB_DISABLE_PYTHON )
-  set_source_files_properties(${LLDB_WRAP_PYTHON} PROPERTIES GENERATED 1)
-  if (CLANG_CL)
-set_source_files_properties(${LLDB_WRAP_PYTHON} PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
-  endif()
-  if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND
-  NOT "${CMAKE_SYSTEM_NAME}" MATCHES "Darwin")
-set_property(SOURCE ${LLDB_WRAP_PYTHON}
- APPEND_STRING PROPERTY COMPILE_FLAGS " -Wno-sequence-point 
-Wno-cast-qual")
-  endif ()
-endif()

Modified: lldb/trunk/scripts/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/CMakeLists.txt?rev=316997&r1=316996&r2=316997&view=diff
==
--- lldb/trunk/scripts/CMakeLists.txt (original)
+++ lldb/trunk/scripts/CMakeLists.txt Tue Oct 31 06:23:19 2017
@@ -48,10 +48,10 @@ add_custom_command(
   --swigExecutable=${SWIG_EXECUTABLE}
   VERBATIM
   COMMENT "Python script building LLDB Python wrapper")
-set_source_files_properties(${LLDB_WRAP_PYTHON} PROPERTIES GENERATED 1)
+add_custom_target(swig_wrapper ALL DEPENDS ${LLDB_WRAP_PYTHON})
+
 set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/lldb.py PROPERTIES 
GENERATED 1)
 
-add_custom_target(swig_wrapper ALL DEPENDS ${LLDB_WRAP_PYTHON})
 
 # Install the LLDB python module
 install(DIRECTORY ${SWIG_PYTHON_DIR} DESTINATION ${SWIG_INSTALL_DIR})

Modified: lldb/trunk/source/API/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/CMakeLists.txt?rev=316997&r1=316996&r2=316997&view=diff
==
--- lldb/trunk/source/API/CMakeLists.txt (original)
+++ lldb/trunk/source/API/CMakeLists.txt Tue Oct 31 06:23:19 2017
@@ -2,10 +2,6 @@ if ( CMAKE_SYSTEM_NAME MATCHES "Windows"
   add_definitions( -DEXPORT_LIBLLDB )
 endif()
 
-# Include this so that add_lldb_library() has the list of dependencies
-# for liblldb to link against
-include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
-
 option(LLDB_BUILD_FRAMEWORK "Build the Darwin LLDB.framework" Off)
 
 if(LLDB_BUILD_FRAMEWORK AND CMAKE_VERSION VERSION_LESS 3.7)
@@ -112,9 +108,17 @@ if

[Lldb-commits] [PATCH] D39246: Fix LLVM_LINK_LLVM_DYLIB build (pr35053)

2017-10-31 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316997: Fix LLVM_LINK_LLVM_DYLIB build (pr35053) (authored 
by labath).

Repository:
  rL LLVM

https://reviews.llvm.org/D39246

Files:
  lldb/trunk/cmake/LLDBDependencies.cmake
  lldb/trunk/scripts/CMakeLists.txt
  lldb/trunk/source/API/CMakeLists.txt
  lldb/trunk/source/Utility/CMakeLists.txt
  lldb/trunk/tools/argdumper/CMakeLists.txt
  lldb/trunk/tools/driver/CMakeLists.txt
  lldb/trunk/tools/intel-features/CMakeLists.txt
  lldb/trunk/tools/lldb-server/CMakeLists.txt
  lldb/trunk/unittests/CMakeLists.txt

Index: lldb/trunk/scripts/CMakeLists.txt
===
--- lldb/trunk/scripts/CMakeLists.txt
+++ lldb/trunk/scripts/CMakeLists.txt
@@ -48,10 +48,10 @@
   --swigExecutable=${SWIG_EXECUTABLE}
   VERBATIM
   COMMENT "Python script building LLDB Python wrapper")
-set_source_files_properties(${LLDB_WRAP_PYTHON} PROPERTIES GENERATED 1)
+add_custom_target(swig_wrapper ALL DEPENDS ${LLDB_WRAP_PYTHON})
+
 set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/lldb.py PROPERTIES GENERATED 1)
 
-add_custom_target(swig_wrapper ALL DEPENDS ${LLDB_WRAP_PYTHON})
 
 # Install the LLDB python module
 install(DIRECTORY ${SWIG_PYTHON_DIR} DESTINATION ${SWIG_INSTALL_DIR})
Index: lldb/trunk/unittests/CMakeLists.txt
===
--- lldb/trunk/unittests/CMakeLists.txt
+++ lldb/trunk/unittests/CMakeLists.txt
@@ -11,8 +11,6 @@
   list(APPEND LLVM_COMPILE_FLAGS -include ${LLDB_GTEST_COMMON_INCLUDE})
 endif ()
 
-include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
-
 if (LLDB_BUILT_STANDALONE)
   # Build the gtest library needed for unittests, if we have LLVM sources
   # handy.
@@ -46,7 +44,7 @@
 POST_BUILD
 COMMAND "${CMAKE_COMMAND}" -E make_directory ${CMAKE_CURRENT_BINARY_DIR}/Inputs)
 
-  target_link_libraries(${test_name} ${ARG_LINK_LIBS} ${LLDB_SYSTEM_LIBS})
+  target_link_libraries(${test_name} ${ARG_LINK_LIBS})
 endfunction()
 
 function(add_unittest_inputs test_name inputs)
Index: lldb/trunk/source/API/CMakeLists.txt
===
--- lldb/trunk/source/API/CMakeLists.txt
+++ lldb/trunk/source/API/CMakeLists.txt
@@ -2,10 +2,6 @@
   add_definitions( -DEXPORT_LIBLLDB )
 endif()
 
-# Include this so that add_lldb_library() has the list of dependencies
-# for liblldb to link against
-include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
-
 option(LLDB_BUILD_FRAMEWORK "Build the Darwin LLDB.framework" Off)
 
 if(LLDB_BUILD_FRAMEWORK AND CMAKE_VERSION VERSION_LESS 3.7)
@@ -112,9 +108,17 @@
 set_property(SOURCE ${LLDB_WRAP_PYTHON} APPEND_STRING PROPERTY COMPILE_FLAGS " -w")
   endif()
 endif()
+set_source_files_properties(${LLDB_WRAP_PYTHON} PROPERTIES GENERATED 1)
+if (CLANG_CL)
+  set_property(SOURCE ${LLDB_WRAP_PYTHON} APPEND_STRING
+PROPERTY COMPILE_FLAGS " -Wno-unused-function")
+endif()
+if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND
+NOT "${CMAKE_SYSTEM_NAME}" MATCHES "Darwin")
+  set_property(SOURCE ${LLDB_WRAP_PYTHON} APPEND_STRING
+PROPERTY COMPILE_FLAGS " -Wno-sequence-point -Wno-cast-qual")
+endif ()
 
-# This should not be part of LLDBDependencies.cmake, because we don't
-# want every single library taking a dependency on the script interpreters.
 target_link_libraries(liblldb PRIVATE
   lldbPluginScriptInterpreterNone
   lldbPluginScriptInterpreterPython
@@ -156,7 +160,6 @@
 if (LLDB_WRAP_PYTHON)
   add_dependencies(liblldb swig_wrapper)
 endif()
-target_link_libraries(liblldb PRIVATE ${LLDB_SYSTEM_LIBS})
 
 if(LLDB_BUILD_FRAMEWORK)
   file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h
Index: lldb/trunk/source/Utility/CMakeLists.txt
===
--- lldb/trunk/source/Utility/CMakeLists.txt
+++ lldb/trunk/source/Utility/CMakeLists.txt
@@ -1,3 +1,44 @@
+set(LLDB_SYSTEM_LIBS)
+
+# Windows-only libraries
+if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
+  list(APPEND LLDB_SYSTEM_LIBS
+ws2_32
+rpcrt4
+)
+endif ()
+
+if (NOT LLDB_DISABLE_LIBEDIT)
+  list(APPEND LLDB_SYSTEM_LIBS edit)
+endif()
+if (NOT LLDB_DISABLE_CURSES)
+  list(APPEND LLDB_SYSTEM_LIBS ${CURSES_LIBRARIES})
+  if(LLVM_ENABLE_TERMINFO AND HAVE_TERMINFO)
+list(APPEND LLDB_SYSTEM_LIBS ${TERMINFO_LIBS})
+  endif()
+endif()
+
+if (NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB )
+list(APPEND LLDB_SYSTEM_LIBS atomic)
+endif()
+
+list(APPEND LLDB_SYSTEM_LIBS ${Backtrace_LIBRARY})
+
+if (NOT LLDB_DISABLE_PYTHON AND NOT LLVM_BUILD_STATIC)
+  list(APPEND LLDB_SYSTEM_LIBS ${PYTHON_LIBRARIES})
+endif()
+
+list(APPEND LLDB_SYSTEM_LIBS ${system_libs})
+
+if (LLVM_BUILD_STATIC)
+  if (NOT LLDB_DISABLE_PYTHON)
+list(APPEND LLDB_SYSTEM_LIBS python2.7 util)
+  endif()
+  if (NOT LLDB_DISABLE_CURSES)
+list(APPEND LLDB_SYSTEM_LIBS gpm)
+  endif()
+endif()
+
 add_lldb_library(lldbUtility
   Baton.cpp
   Conn

Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Greg Clayton via lldb-commits
This is one example of how StringRef causes issues because it was adopted 
everywhere. Is there any way we can change our functions so we can't run into 
this issue? Anything we can learn from this example?

 

> On Oct 26, 2017, at 9:53 PM, Pavel Labath via lldb-commits 
>  wrote:
> 
> Author: labath
> Date: Thu Oct 26 21:53:24 2017
> New Revision: 316740
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=316740&view=rev
> Log:
> Fix a use-after-free in lldb-server
> 
> UriParser::Parse is returning a StringRef pointing the the parsed
> string, but we were calling it with a temporary string. Change this to a
> local variable to make sure the string persists as long as we need it.
> 
> Modified:
>
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
> 
> Modified: 
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp?rev=316740&r1=316739&r2=316740&view=diff
> ==
> --- 
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
>  (original)
> +++ 
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
>  Thu Oct 26 21:53:24 2017
> @@ -128,8 +128,9 @@ Status GDBRemoteCommunicationServerPlatf
>   llvm::StringRef platform_ip;
>   int platform_port;
>   llvm::StringRef platform_path;
> -  bool ok = UriParser::Parse(GetConnection()->GetURI(), platform_scheme,
> - platform_ip, platform_port, platform_path);
> +  std::string platform_uri = GetConnection()->GetURI();
> +  bool ok = UriParser::Parse(platform_uri, platform_scheme, platform_ip,
> + platform_port, platform_path);
>   UNUSED_IF_ASSERT_DISABLED(ok);
>   assert(ok);
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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


Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Don Hinton via lldb-commits
There have been a few suggestions that I could just use a script to solve
this "problem" -- poor startup performance of clangdiag.

However, this patch was not submitted to solve a particular problem.  It
was submitted in response to Jim's suggestion:

On Mon, Oct 23, 2017 at 6:25 PM, Jim Ingham  wrote:

> Yeah, that would be easy to implement from the command line, maybe add a
> --file-is-regex flag or something.
>
> From the SB API it would be better to have something like:
>
> SBFileList SBTarget.GetFileListMatchingRegex("regex")
>
> Please file an enhancement request for these of hack'em in if you're so
> motivated.
>

clangdiag was only provided as a motivating example.

As for the contention that this would be a little used option, I'd argue
the opposite.

I use regular expressions all the time, and I'm sure I'm not alone.  Once
uses are given the choice of providing multiple '-f' and/or '-s' options
compare to a single, more powerful '-z' or '-Z' option, I'd bet most of
them would start using the new options exclusively.  I certainly would.


On Mon, Oct 30, 2017 at 10:03 PM, Don Hinton  wrote:

> I'll add tests if it looks like it'll be accepted, but based on the
> initial response, that doesn't seem likely.
>
> However, it was a good exercise and addressed the issues raised.
>
> thanks again for all the feedback...
> don
>
> On Mon, Oct 30, 2017 at 9:44 PM, Zachary Turner 
> wrote:
>
>> Asking again, but why can’t this be done in th the script for clangdiag?
>> For example, there’s no tests for any of this in this patch. And it seems
>> likely that it will be rarely used anyway. So I’m still not convinced the
>> option-pollution and increased long term code maintenance burden of this
>> underutilized codepath is worth the benefit.
>>
>> Can you see if manually scanning for these files in python and then
>> setting breakpoints on the right set of files solves the problem?
>>
>> On Mon, Oct 30, 2017 at 9:00 PM Don Hinton via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>>> hintonda updated this revision to Diff 120933.
>>> hintonda added a comment.
>>>
>>> - Remove prefix and add options.
>>>
>>>
>>> https://reviews.llvm.org/D39436
>>>
>>> Files:
>>>   include/lldb/Utility/FileSpec.h
>>>   source/Commands/CommandObjectBreakpoint.cpp
>>>   source/Utility/FileSpec.cpp
>>>
>>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Zachary Turner via lldb-commits
The takeaway from this example is nothing we don't already know.  We need
better test coverage.

On Tue, Oct 31, 2017 at 8:08 AM Greg Clayton via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> This is one example of how StringRef causes issues because it was adopted
> everywhere. Is there any way we can change our functions so we can't run
> into this issue? Anything we can learn from this example?
>
>
>
> > On Oct 26, 2017, at 9:53 PM, Pavel Labath via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> > Author: labath
> > Date: Thu Oct 26 21:53:24 2017
> > New Revision: 316740
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=316740&view=rev
> > Log:
> > Fix a use-after-free in lldb-server
> >
> > UriParser::Parse is returning a StringRef pointing the the parsed
> > string, but we were calling it with a temporary string. Change this to a
> > local variable to make sure the string persists as long as we need it.
> >
> > Modified:
> >
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
> >
> > Modified:
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp?rev=316740&r1=316739&r2=316740&view=diff
> >
> ==
> > ---
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
> (original)
> > +++
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
> Thu Oct 26 21:53:24 2017
> > @@ -128,8 +128,9 @@ Status GDBRemoteCommunicationServerPlatf
> >   llvm::StringRef platform_ip;
> >   int platform_port;
> >   llvm::StringRef platform_path;
> > -  bool ok = UriParser::Parse(GetConnection()->GetURI(), platform_scheme,
> > - platform_ip, platform_port, platform_path);
> > +  std::string platform_uri = GetConnection()->GetURI();
> > +  bool ok = UriParser::Parse(platform_uri, platform_scheme, platform_ip,
> > + platform_port, platform_path);
> >   UNUSED_IF_ASSERT_DISABLED(ok);
> >   assert(ok);
> >
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Pavel Labath via lldb-commits
On 31 October 2017 at 15:12, Zachary Turner via lldb-commits
 wrote:
> The takeaway from this example is nothing we don't already know.  We need
> better test coverage.
Actually, this was caught by a test (pretty much all of them), but
only when building with libc++, as the code was "safe" with libstdc++
due to the copy-on-write implementation of std::string (the temporary
object shared storage with the longer-lived string it was copied
from).

>
> On Tue, Oct 31, 2017 at 8:08 AM Greg Clayton via lldb-commits
>  wrote:
>>
>> This is one example of how StringRef causes issues because it was adopted
>> everywhere. Is there any way we can change our functions so we can't run
>> into this issue? Anything we can learn from this example?

My takeaway from this is to be more careful when returning StringRef
objects from a function. The patch in question changed the lifetime
semantics of UrlParser. Unlike most other const char * -> StringRef
conversions, this is kind of change requires a closer look at the
existing callers to make sure they satisfy the new constraints.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Zachary Turner via lldb-commits
It's worth mentioning again that LLVM uses StringRef pretty pervasively and
doesn't have issues.  There's nothing fundamentally different about
debuggers that makes StringRefs bad whereas they can be good in other types
of software.  If we really wanted to avoid use-after-frees we would pick a
language other than C++.

Note that StringRef is now part of the C++ standard, under the name
std::string_view.  Every part of C++ can shoot you in the foot if used
incorrectly.  This is equally true for std::string , and pointers in
general.  I don't see anything special about this particular bug.  Someone
returned a pointer to stack memory.  That's a pretty common class of bug.

Actually though, I did think of another thing we learn from this example.
We should be running LLDB under ASAN

On Tue, Oct 31, 2017 at 8:12 AM Zachary Turner  wrote:

> The takeaway from this example is nothing we don't already know.  We need
> better test coverage.
>
> On Tue, Oct 31, 2017 at 8:08 AM Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> This is one example of how StringRef causes issues because it was adopted
>> everywhere. Is there any way we can change our functions so we can't run
>> into this issue? Anything we can learn from this example?
>>
>>
>>
>> > On Oct 26, 2017, at 9:53 PM, Pavel Labath via lldb-commits <
>> lldb-commits@lists.llvm.org> wrote:
>> >
>> > Author: labath
>> > Date: Thu Oct 26 21:53:24 2017
>> > New Revision: 316740
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=316740&view=rev
>> > Log:
>> > Fix a use-after-free in lldb-server
>> >
>> > UriParser::Parse is returning a StringRef pointing the the parsed
>> > string, but we were calling it with a temporary string. Change this to a
>> > local variable to make sure the string persists as long as we need it.
>> >
>> > Modified:
>> >
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
>> >
>> > Modified:
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp?rev=316740&r1=316739&r2=316740&view=diff
>> >
>> ==
>> > ---
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
>> (original)
>> > +++
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
>> Thu Oct 26 21:53:24 2017
>> > @@ -128,8 +128,9 @@ Status GDBRemoteCommunicationServerPlatf
>> >   llvm::StringRef platform_ip;
>> >   int platform_port;
>> >   llvm::StringRef platform_path;
>> > -  bool ok = UriParser::Parse(GetConnection()->GetURI(),
>> platform_scheme,
>> > - platform_ip, platform_port,
>> platform_path);
>> > +  std::string platform_uri = GetConnection()->GetURI();
>> > +  bool ok = UriParser::Parse(platform_uri, platform_scheme,
>> platform_ip,
>> > + platform_port, platform_path);
>> >   UNUSED_IF_ASSERT_DISABLED(ok);
>> >   assert(ok);
>> >
>> >
>> >
>> > ___
>> > lldb-commits mailing list
>> > lldb-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Zachary Turner via lldb-commits
On Tue, Oct 31, 2017 at 8:12 AM Don Hinton  wrote:

> There have been a few suggestions that I could just use a script to solve
> this "problem" -- poor startup performance of clangdiag.
>
> However, this patch was not submitted to solve a particular problem.  It
> was submitted in response to Jim's suggestion:
>
> On Mon, Oct 23, 2017 at 6:25 PM, Jim Ingham  wrote:
>
>> Yeah, that would be easy to implement from the command line, maybe add a
>> --file-is-regex flag or something.
>>
>> From the SB API it would be better to have something like:
>>
>> SBFileList SBTarget.GetFileListMatchingRegex("regex")
>>
>> Please file an enhancement request for these of hack'em in if you're so
>> motivated.
>>
>
> clangdiag was only provided as a motivating example.
>

Fair point, I had forgotten about that.  That said, I'm honestly still not
that big of a fan.   It's no secret that I have a higher bar than others
for options and API additions, but to me the potential use this particular
option + SB API would get is not sufficient to justify the long term
maintenance burden associated with having it.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Don Hinton via lldb-commits
On Tue, Oct 31, 2017 at 8:22 AM, Zachary Turner  wrote:

>
>
> On Tue, Oct 31, 2017 at 8:12 AM Don Hinton  wrote:
>
>> There have been a few suggestions that I could just use a script to solve
>> this "problem" -- poor startup performance of clangdiag.
>>
>> However, this patch was not submitted to solve a particular problem.  It
>> was submitted in response to Jim's suggestion:
>>
>> On Mon, Oct 23, 2017 at 6:25 PM, Jim Ingham  wrote:
>>
>>> Yeah, that would be easy to implement from the command line, maybe add a
>>> --file-is-regex flag or something.
>>>
>>> From the SB API it would be better to have something like:
>>>
>>> SBFileList SBTarget.GetFileListMatchingRegex("regex")
>>>
>>> Please file an enhancement request for these of hack'em in if you're so
>>> motivated.
>>>
>>
>> clangdiag was only provided as a motivating example.
>>
>
> Fair point, I had forgotten about that.  That said, I'm honestly still not
> that big of a fan.   It's no secret that I have a higher bar than others
> for options and API additions, but to me the potential use this particular
> option + SB API would get is not sufficient to justify the long term
> maintenance burden associated with having it.
>

I'm not that familiar with the SB API part, but will investigate, and try
to come up with a better motivation.

Thanks again for the suggestions and feedback...
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r317004 - Add a "watchpoint" test category and annotate tests appropriately

2017-10-31 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 31 08:27:19 2017
New Revision: 317004

URL: http://llvm.org/viewvc/llvm-project?rev=317004&view=rev
Log:
Add a "watchpoint" test category and annotate tests appropriately

Most of the watchpoint tests are organized into subtrees, so we can use the
file-based .categories approach to annotate them. The exception are the
concurrent_events tests, which needed to be annotated on a per-test basis.

The motivation behind this is to provide an easy way to disable watchpoint
tests on systems where the watchpoint functionality is not present/unreliable.

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/.categories
lldb/trunk/packages/Python/lldbsuite/test/python_api/watchpoint/.categories
Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/breakpoint_delay_breakpoint_one_signal/TestConcurrentBreakpointDelayBreakpointOneSignal.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/crash_with_watchpoint/TestConcurrentCrashWithWatchpoint.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/crash_with_watchpoint_breakpoint_signal/TestConcurrentCrashWithWatchpointBreakpointSignal.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/delay_signal_watch/TestConcurrentDelaySignalWatch.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/delay_watch_break/TestConcurrentDelayWatchBreak.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/delayed_crash_with_breakpoint_watchpoint/TestConcurrentDelayedCrashWithBreakpointWatchpoint.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/many_watchpoints/TestConcurrentManyWatchpoints.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/n_watch_n_break/TestConcurrentNWatchNBreak.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/signal_delay_watch/TestConcurrentSignalDelayWatch.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/signal_n_watch_n_break/TestConcurrentSignalNWatchNBreak.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/signal_watch/TestConcurrentSignalWatch.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/signal_watch_break/TestConcurrentSignalWatchBreak.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_breakpoints_one_watchpoint/TestConcurrentTwoBreakpointsOneWatchpoint.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoint_threads/TestConcurrentTwoWatchpointThreads.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_breakpoint/TestConcurrentTwoWatchpointsOneBreakpoint.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_delay_breakpoint/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_signal/TestConcurrentTwoWatchpointsOneSignal.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/watch_break/TestConcurrentWatchBreak.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/watch_break_delay/TestConcurrentWatchBreakDelay.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/watchpoint_delay_watchpoint_one_breakpoint/TestConcurrentWatchpointDelayWatchpointOneBreakpoint.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/watchpoint_with_delay_watchpoint_threads/TestConcurrentWatchpointWithDelayWatchpointThreads.py
lldb/trunk/packages/Python/lldbsuite/test/test_categories.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/breakpoint_delay_breakpoint_one_signal/TestConcurrentBreakpointDelayBreakpointOneSignal.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/breakpoint_delay_breakpoint_one_signal/TestConcurrentBreakpointDelayBreakpointOneSignal.py?rev=317004&r1=317003&r2=317004&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/breakpoint_delay_breakpoint_one_signal/TestConcurrentBreakpointDelayBreakpointOneSignal.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/breakpoint_delay_breakpoint_one_signal/TestConcurrentBreakpointDelayBreakpointOneSignal.py
 Tue Oct 31 08:27:19 2017
@@ -15,

Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Don Hinton via lldb-commits
Btw, is there a way to pass the '-m' option via the SB API?  I'd like to
exclude matches in comments when using BreakpointCreateBySourceRegex.

On Tue, Oct 31, 2017 at 8:26 AM, Don Hinton  wrote:

> On Tue, Oct 31, 2017 at 8:22 AM, Zachary Turner 
> wrote:
>
>>
>>
>> On Tue, Oct 31, 2017 at 8:12 AM Don Hinton  wrote:
>>
>>> There have been a few suggestions that I could just use a script to
>>> solve this "problem" -- poor startup performance of clangdiag.
>>>
>>> However, this patch was not submitted to solve a particular problem.  It
>>> was submitted in response to Jim's suggestion:
>>>
>>> On Mon, Oct 23, 2017 at 6:25 PM, Jim Ingham  wrote:
>>>
 Yeah, that would be easy to implement from the command line, maybe add
 a --file-is-regex flag or something.

 From the SB API it would be better to have something like:

 SBFileList SBTarget.GetFileListMatchingRegex("regex")

 Please file an enhancement request for these of hack'em in if you're so
 motivated.

>>>
>>> clangdiag was only provided as a motivating example.
>>>
>>
>> Fair point, I had forgotten about that.  That said, I'm honestly still
>> not that big of a fan.   It's no secret that I have a higher bar than
>> others for options and API additions, but to me the potential use this
>> particular option + SB API would get is not sufficient to justify the long
>> term maintenance burden associated with having it.
>>
>
> I'm not that familiar with the SB API part, but will investigate, and try
> to come up with a better motivation.
>
> Thanks again for the suggestions and feedback...
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Zachary Turner via lldb-commits
On Tue, Oct 31, 2017 at 8:20 AM Pavel Labath  wrote:

> On 31 October 2017 at 15:12, Zachary Turner via lldb-commits
>  wrote:
> > The takeaway from this example is nothing we don't already know.  We need
> > better test coverage.
> Actually, this was caught by a test (pretty much all of them), but
> only when building with libc++, as the code was "safe" with libstdc++
> due to the copy-on-write implementation of std::string (the temporary
> object shared storage with the longer-lived string it was copied
> from).
>

What version of libstdc++?  AFAIK copy-on-write std::string is no longer
conformant in C++11.

If this was really being masked due to a CoW string implementation, we need
to make absolutely sure we are running tests on a bot with a conformant
standard library.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Pavel Labath via lldb-commits
libstdc++-4.8

I think the newest versions have a conformant implementation, but I
don't know the full details (I know it was a complicated change
because of the need to maintain binary compatibility).

Our android tests are now runinng lldb-server built with libc++ (which
is how I found the bug). Eventually (= probably not this year) I may
switch the host lldb as well.

On 31 October 2017 at 15:41, Zachary Turner  wrote:
>
>
> On Tue, Oct 31, 2017 at 8:20 AM Pavel Labath  wrote:
>>
>> On 31 October 2017 at 15:12, Zachary Turner via lldb-commits
>>  wrote:
>> > The takeaway from this example is nothing we don't already know.  We
>> > need
>> > better test coverage.
>> Actually, this was caught by a test (pretty much all of them), but
>> only when building with libc++, as the code was "safe" with libstdc++
>> due to the copy-on-write implementation of std::string (the temporary
>> object shared storage with the longer-lived string it was copied
>> from).
>
>
> What version of libstdc++?  AFAIK copy-on-write std::string is no longer
> conformant in C++11.
>
> If this was really being masked due to a CoW string implementation, we need
> to make absolutely sure we are running tests on a bot with a conformant
> standard library.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Zachary Turner via lldb-commits
Well, at least the good news is this kind of bug *should* have been caught
had we been using a conformant standard library implementation.

Have you ever tried running the test suite with an ASAN instrumented build
of LLDB?  We really should have one ASAN bot for every supported
configuration.

On Tue, Oct 31, 2017 at 8:48 AM Pavel Labath  wrote:

> libstdc++-4.8
>
> I think the newest versions have a conformant implementation, but I
> don't know the full details (I know it was a complicated change
> because of the need to maintain binary compatibility).
>
> Our android tests are now runinng lldb-server built with libc++ (which
> is how I found the bug). Eventually (= probably not this year) I may
> switch the host lldb as well.
>
> On 31 October 2017 at 15:41, Zachary Turner  wrote:
> >
> >
> > On Tue, Oct 31, 2017 at 8:20 AM Pavel Labath  wrote:
> >>
> >> On 31 October 2017 at 15:12, Zachary Turner via lldb-commits
> >>  wrote:
> >> > The takeaway from this example is nothing we don't already know.  We
> >> > need
> >> > better test coverage.
> >> Actually, this was caught by a test (pretty much all of them), but
> >> only when building with libc++, as the code was "safe" with libstdc++
> >> due to the copy-on-write implementation of std::string (the temporary
> >> object shared storage with the longer-lived string it was copied
> >> from).
> >
> >
> > What version of libstdc++?  AFAIK copy-on-write std::string is no longer
> > conformant in C++11.
> >
> > If this was really being masked due to a CoW string implementation, we
> need
> > to make absolutely sure we are running tests on a bot with a conformant
> > standard library.
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Greg Clayton via lldb-commits
My takeaway is a bug was added that wasn't previously a bug. If code was 
designed to carefully use StringRef, then yes, it can be made safe. But we 
added StringRef support in all of LLDB and we didn't catch all of the possible 
misuses. My main questions is: is there anything we can do to catch these 
things now that we have them.

On Mac we can set the MallocScribble environment variable in our test suite. 
This will scribble 0x55 on all freed pages. This might catch some extra 
crashes. Malloc debug details:

https://developer.apple.com/library/content/documentation/Performance/Conceptual/ManagingMemory/Articles/MallocDebug.html
 





> On Oct 31, 2017, at 8:12 AM, Zachary Turner  wrote:
> 
> The takeaway from this example is nothing we don't already know.  We need 
> better test coverage.
> 
> On Tue, Oct 31, 2017 at 8:08 AM Greg Clayton via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
> This is one example of how StringRef causes issues because it was adopted 
> everywhere. Is there any way we can change our functions so we can't run into 
> this issue? Anything we can learn from this example?
> 
> 
> 
> > On Oct 26, 2017, at 9:53 PM, Pavel Labath via lldb-commits 
> > mailto:lldb-commits@lists.llvm.org>> wrote:
> >
> > Author: labath
> > Date: Thu Oct 26 21:53:24 2017
> > New Revision: 316740
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=316740&view=rev 
> > 
> > Log:
> > Fix a use-after-free in lldb-server
> >
> > UriParser::Parse is returning a StringRef pointing the the parsed
> > string, but we were calling it with a temporary string. Change this to a
> > local variable to make sure the string persists as long as we need it.
> >
> > Modified:
> >
> > lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
> >
> > Modified: 
> > lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp?rev=316740&r1=316739&r2=316740&view=diff
> >  
> > 
> > ==
> > --- 
> > lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
> >  (original)
> > +++ 
> > lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
> >  Thu Oct 26 21:53:24 2017
> > @@ -128,8 +128,9 @@ Status GDBRemoteCommunicationServerPlatf
> >   llvm::StringRef platform_ip;
> >   int platform_port;
> >   llvm::StringRef platform_path;
> > -  bool ok = UriParser::Parse(GetConnection()->GetURI(), platform_scheme,
> > - platform_ip, platform_port, platform_path);
> > +  std::string platform_uri = GetConnection()->GetURI();
> > +  bool ok = UriParser::Parse(platform_uri, platform_scheme, platform_ip,
> > + platform_port, platform_path);
> >   UNUSED_IF_ASSERT_DISABLED(ok);
> >   assert(ok);
> >
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org 
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
> > 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org 
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
> 

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


Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Zachary Turner via lldb-commits
On Tue, Oct 31, 2017 at 9:00 AM Greg Clayton  wrote:

> My main questions is: is there anything we can do to catch these things
> now that we have them.
>

Absolutely.  Get bots running check-lldb with ASAN instrumented LLDB.  I'm
going to go out on a limb and say you will find not just these bugs, but
many more which have been lingering for a long time.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Zachary Turner via lldb-commits
This is a case where I think a new API *would* be warranted.  But it would
not be an API specifically for the -m option.  It would instead be an API
that introduces an `SBBreakpointOptions` class, and then add a method
called `BreakpointCreateWithOptions(options)`.

Note that the general policy of the SB API is that you cannot delete or
modify an existing method.  This kind of pattern provides an API that *can*
be extended in the future, because you could, for example, add fields to
SBBreakpointOptions and existing code wouldn't break, but new code could
take advantage of the new options.

On Tue, Oct 31, 2017 at 8:32 AM Don Hinton  wrote:

> Btw, is there a way to pass the '-m' option via the SB API?  I'd like to
> exclude matches in comments when using BreakpointCreateBySourceRegex.
>
> On Tue, Oct 31, 2017 at 8:26 AM, Don Hinton  wrote:
>
>> On Tue, Oct 31, 2017 at 8:22 AM, Zachary Turner 
>> wrote:
>>
>>>
>>>
>>> On Tue, Oct 31, 2017 at 8:12 AM Don Hinton  wrote:
>>>
 There have been a few suggestions that I could just use a script to
 solve this "problem" -- poor startup performance of clangdiag.

 However, this patch was not submitted to solve a particular problem.
 It was submitted in response to Jim's suggestion:

 On Mon, Oct 23, 2017 at 6:25 PM, Jim Ingham  wrote:

> Yeah, that would be easy to implement from the command line, maybe add
> a --file-is-regex flag or something.
>
> From the SB API it would be better to have something like:
>
> SBFileList SBTarget.GetFileListMatchingRegex("regex")
>
> Please file an enhancement request for these of hack'em in if you're
> so motivated.
>

 clangdiag was only provided as a motivating example.

>>>
>>> Fair point, I had forgotten about that.  That said, I'm honestly still
>>> not that big of a fan.   It's no secret that I have a higher bar than
>>> others for options and API additions, but to me the potential use this
>>> particular option + SB API would get is not sufficient to justify the long
>>> term maintenance burden associated with having it.
>>>
>>
>> I'm not that familiar with the SB API part, but will investigate, and try
>> to come up with a better motivation.
>>
>> Thanks again for the suggestions and feedback...
>>
>>
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Davide Italiano via lldb-commits
On Tue, Oct 31, 2017 at 8:59 AM, Greg Clayton via lldb-commits
 wrote:
> My takeaway is a bug was added that wasn't previously a bug. If code was
> designed to carefully use StringRef, then yes, it can be made safe. But we
> added StringRef support in all of LLDB and we didn't catch all of the
> possible misuses. My main questions is: is there anything we can do to catch
> these things now that we have them.
>

FWIW, I'd say this is not necessarily a bad thing (the bug).
You can now try to write a test that catches this behaviour and/or run
an ASAN bot if there's a test already in case somebody breaks this
code.

Also, just to mention, this happen{s, ed} in LLVM many times, people
just revert the revision or fix immediately.
That's why we have bot and tests for. We shouldn't be really afraid of
using a variant of a standardized (!) function because it can break.
If people are judicious, then the problem will be fixed. If they're
not, the revision will be reverted.
I think the main takeaway from this test is lack of testing/infra, but
I'm not necessarily sure it's fair to pick on this commit because it
"added a bug".
We all know it could've happened regardless.

Thanks,

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


[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

A few general things: don't modify FileSpec, we have many of these objects and 
we can't increase their size without directly affecting memory usage. FileSpec 
objects represent one file on disk, not multiple. We should make a new class 
that contains a FileSpec and a regex, but not put it into FileSpec.




Comment at: include/lldb/Utility/FileSpec.h:96
 
+  explicit FileSpec(llvm::StringRef regex);
+

This constructor seem dangerous as it might be confused with a path.



Comment at: include/lldb/Utility/FileSpec.h:589
   m_syntax; ///< The syntax that this path uses (e.g. Windows / Posix)
+  RegularExpression m_regex;  ///< Regular expression in "regex:" 
prefix passed.
 };

We shouldn't add something to FileSpec here. Now every FileSpec in LLDB will 
carry around an extra RegularExpression object as part of the object? Please 
remove. We should make a new class instead of adding something to FileSpec. 
FileSpec represents a single file on disk, not N files on disk.



Comment at: source/Commands/CommandObjectBreakpoint.cpp:566
+  case 'z':
+m_filenames.AppendIfUnique(FileSpec(option_arg));
+break;

We should probably have m_file_regexes (arrary) or just m_file_regex (one).



Comment at: source/Commands/CommandObjectBreakpoint.cpp:570
+  case 'Z':
+m_modules.AppendIfUnique(FileSpec(option_arg));
+break;

We should probably have m_module_regexes (arrary) or just m_module_regex (one).



Comment at: source/Utility/FileSpec.cpp:183-188
+FileSpec::FileSpec(llvm::StringRef regex) {
+  m_syntax = ePathSyntaxHostNative;
+  m_filename.SetString(regex);
+  m_regex.Compile(regex);
+}
+

Revert. We can't add stuff to FileSpec because some piece of code wants to use 
it in a non-standard way. FileSpec represents one file.



Comment at: source/Utility/FileSpec.cpp:194-195
 : m_directory(rhs.m_directory), m_filename(rhs.m_filename),
-  m_is_resolved(rhs.m_is_resolved), m_syntax(rhs.m_syntax) {}
+  m_is_resolved(rhs.m_is_resolved), m_syntax(rhs.m_syntax),
+  m_regex(rhs.m_regex) {}
 

Revert. We can't add stuff to FileSpec because some piece of code wants to use 
it in a non-standard way. FileSpec represents one file.



Comment at: source/Utility/FileSpec.cpp:219
 m_syntax = rhs.m_syntax;
+m_regex = rhs.m_regex;
   }

Revert



Comment at: source/Utility/FileSpec.cpp:312-314
+  if (m_regex.IsValid())
+return m_regex.Execute(rhs.GetPath());
+

Revert




Comment at: source/Utility/FileSpec.cpp:425-428
+  if (a.m_regex.IsValid())
+return a.m_regex.Execute(b.GetPath());
+
   static ConstString g_dot_string(".");

revert


https://reviews.llvm.org/D39436



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


Re: [Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

2017-10-31 Thread Greg Clayton via lldb-commits
Agreed, get ASAN working and we will find this issue and any remaining issues.


> On Oct 31, 2017, at 9:42 AM, Davide Italiano  wrote:
> 
> On Tue, Oct 31, 2017 at 8:59 AM, Greg Clayton via lldb-commits
>  wrote:
>> My takeaway is a bug was added that wasn't previously a bug. If code was
>> designed to carefully use StringRef, then yes, it can be made safe. But we
>> added StringRef support in all of LLDB and we didn't catch all of the
>> possible misuses. My main questions is: is there anything we can do to catch
>> these things now that we have them.
>> 
> 
> FWIW, I'd say this is not necessarily a bad thing (the bug).
> You can now try to write a test that catches this behaviour and/or run
> an ASAN bot if there's a test already in case somebody breaks this
> code.
> 
> Also, just to mention, this happen{s, ed} in LLVM many times, people
> just revert the revision or fix immediately.
> That's why we have bot and tests for. We shouldn't be really afraid of
> using a variant of a standardized (!) function because it can break.
> If people are judicious, then the problem will be fixed. If they're
> not, the revision will be reverted.
> I think the main takeaway from this test is lack of testing/infra, but
> I'm not necessarily sure it's fair to pick on this commit because it
> "added a bug".
> We all know it could've happened regardless.
> 
> Thanks,
> 
> --
> Davide

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


Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Jim Ingham via lldb-commits


> On Oct 31, 2017, at 9:05 AM, Zachary Turner  wrote:
> 
> This is a case where I think a new API *would* be warranted.  But it would 
> not be an API specifically for the -m option.  It would instead be an API 
> that introduces an `SBBreakpointOptions` class, and then add a method called 
> `BreakpointCreateWithOptions(options)`.

That's almost right.  I want to keep the options that determine WHERE locations 
get set separate from the options that determine what happens WHEN a breakpoint 
gets hit.  The "reaction" options can be shared with the SBBreakpointName class 
and with Stop Hooks, and with a little fiddling should also be sharable with 
Watchpoints.  The only difference between watchpoints & breakpoints is the 
meaning of the ID they get passed however.  If you really wanted to share the 
options broadly, we'd probably have to also tell you what kind of stop this was 
for.  Anyway, for these reasons it is valuable to keep this part of the 
breakpoint specification separate.  But at the lldb_private layer these are 
already all gathered into their own class so promoting them would be 
straightforward.

The setting options aren't factored into their own class at the lldb_private 
layer.  They get used directly to make the search filters & kernels that 
actually implement the breakpoint, so that class wasn't really needed.  But it 
wouldn't be that hard to make an intermediary that gathers up these options for 
convenience.  Then you'd have:

SBBreakpoint BreakpointCreateWithOptions(SBBreakpointSettingOptions 
&setting_options, SBStoppointOptions &reaction_options);

> 
> Note that the general policy of the SB API is that you cannot delete or 
> modify an existing method.  This kind of pattern provides an API that *can* 
> be extended in the future, because you could, for example, add fields to 
> SBBreakpointOptions and existing code wouldn't break, but new code could take 
> advantage of the new options.
> 

Yes, we've been trying to convert all the complex information passing API's to 
this form both because it makes code using them easier to read and it's 
extensible.

Jim


> On Tue, Oct 31, 2017 at 8:32 AM Don Hinton  wrote:
> Btw, is there a way to pass the '-m' option via the SB API?  I'd like to 
> exclude matches in comments when using BreakpointCreateBySourceRegex.
> 
> On Tue, Oct 31, 2017 at 8:26 AM, Don Hinton  wrote:
> On Tue, Oct 31, 2017 at 8:22 AM, Zachary Turner  wrote:
> 
> 
> On Tue, Oct 31, 2017 at 8:12 AM Don Hinton  wrote:
> There have been a few suggestions that I could just use a script to solve 
> this "problem" -- poor startup performance of clangdiag.
> 
> However, this patch was not submitted to solve a particular problem.  It was 
> submitted in response to Jim's suggestion:
> 
> On Mon, Oct 23, 2017 at 6:25 PM, Jim Ingham  wrote:
> Yeah, that would be easy to implement from the command line, maybe add a 
> --file-is-regex flag or something.
> 
> From the SB API it would be better to have something like:
> 
> SBFileList SBTarget.GetFileListMatchingRegex("regex")
> 
> Please file an enhancement request for these of hack'em in if you're so 
> motivated.
> 
> clangdiag was only provided as a motivating example.
> 
> Fair point, I had forgotten about that.  That said, I'm honestly still not 
> that big of a fan.   It's no secret that I have a higher bar than others for 
> options and API additions, but to me the potential use this particular option 
> + SB API would get is not sufficient to justify the long term maintenance 
> burden associated with having it.
> 
> I'm not that familiar with the SB API part, but will investigate, and try to 
> come up with a better motivation.
> 
> Thanks again for the suggestions and feedback...
>  
> 
> 

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


Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Jim Ingham via lldb-commits
Not directly, though you can get the ID from the breakpoint you've made, and 
then do:

debugger.HandleCommand("breakpoint modify -m 0 %d"%(ID))

Jim


> On Oct 31, 2017, at 8:32 AM, Don Hinton  wrote:
> 
> Btw, is there a way to pass the '-m' option via the SB API?  I'd like to 
> exclude matches in comments when using BreakpointCreateBySourceRegex.
> 
> On Tue, Oct 31, 2017 at 8:26 AM, Don Hinton  wrote:
> On Tue, Oct 31, 2017 at 8:22 AM, Zachary Turner  wrote:
> 
> 
> On Tue, Oct 31, 2017 at 8:12 AM Don Hinton  wrote:
> There have been a few suggestions that I could just use a script to solve 
> this "problem" -- poor startup performance of clangdiag.
> 
> However, this patch was not submitted to solve a particular problem.  It was 
> submitted in response to Jim's suggestion:
> 
> On Mon, Oct 23, 2017 at 6:25 PM, Jim Ingham  wrote:
> Yeah, that would be easy to implement from the command line, maybe add a 
> --file-is-regex flag or something.
> 
> From the SB API it would be better to have something like:
> 
> SBFileList SBTarget.GetFileListMatchingRegex("regex")
> 
> Please file an enhancement request for these of hack'em in if you're so 
> motivated.
> 
> clangdiag was only provided as a motivating example.
> 
> Fair point, I had forgotten about that.  That said, I'm honestly still not 
> that big of a fan.   It's no secret that I have a higher bar than others for 
> options and API additions, but to me the potential use this particular option 
> + SB API would get is not sufficient to justify the long term maintenance 
> burden associated with having it.
> 
> I'm not that familiar with the SB API part, but will investigate, and try to 
> come up with a better motivation.
> 
> Thanks again for the suggestions and feedback...
>  
> 
> 

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


[Lldb-commits] [PATCH] D35305: Remove uint32_t assignment operator from Status

2017-10-31 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

Sure, this seems fine to me.  Not sure why this was introduced, the svn history 
has become hard to follow.  But this seems overly specific for an integer  
assignment operator.


https://reviews.llvm.org/D35305



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


Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Zachary Turner via lldb-commits
On Tue, Oct 31, 2017 at 10:43 AM Jim Ingham  wrote:

>
>
> > On Oct 31, 2017, at 9:05 AM, Zachary Turner  wrote:
> >
> > This is a case where I think a new API *would* be warranted.  But it
> would not be an API specifically for the -m option.  It would instead be an
> API that introduces an `SBBreakpointOptions` class, and then add a method
> called `BreakpointCreateWithOptions(options)`.
>
> That's almost right.  I want to keep the options that determine WHERE
> locations get set separate from the options that determine what happens
> WHEN a breakpoint gets hit.  The "reaction" options can be shared with the
> SBBreakpointName class and with Stop Hooks, and with a little fiddling
> should also be sharable with Watchpoints.  The only difference between
> watchpoints & breakpoints is the meaning of the ID they get passed
> however.  If you really wanted to share the options broadly, we'd probably
> have to also tell you what kind of stop this was for.  Anyway, for these
> reasons it is valuable to keep this part of the breakpoint specification
> separate.  But at the lldb_private layer these are already all gathered
> into their own class so promoting them would be straightforward.
>
> The setting options aren't factored into their own class at the
> lldb_private layer.  They get used directly to make the search filters &
> kernels that actually implement the breakpoint, so that class wasn't really
> needed.  But it wouldn't be that hard to make an intermediary that gathers
> up these options for convenience.  Then you'd have:
>
> SBBreakpoint BreakpointCreateWithOptions(SBBreakpointSettingOptions
> &setting_options, SBStoppointOptions &reaction_options);
>

That makes sense, I can get behind something like this.  I might quibble
over the names a little bit in the interest of brevity (e.g. something like
SBBreakpointLocation and SBBreakpointAction), but that's minor.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D35615: Add data formatter for libc++ std::tuple

2017-10-31 Thread Jim Ingham via Phabricator via lldb-commits
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

It seems awkward to me to be returning empty ValueObjectSP's by returning 
nullptr and then relying on conversion.  We don't do it that way in most of the 
other formatters.  If there wasn't a strong reason for doing it this way I 
think it would be clearer to return the object you're supposed to return.

Other than that this looks fine.




Comment at: source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp:47
+  m_elements.assign(m_base_sp->GetCompilerType().GetNumDirectBaseClasses(),
+nullptr);
+  return false;

nullptr -> ValueObjectSP.  It makes it clearer reading the thing what it is.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp:53
+  if (idx >= m_elements.size())
+return nullptr;
+  if (!m_base_sp)

It looks a little weird to return nullptr for a function returning a 
ValueObjectSP.  It would be clearer to return ValueObjectSP(), and that's what 
we do pretty much everywhere else.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp:55
+  if (!m_base_sp)
+return nullptr;
+  if (m_elements[idx])

ditto


https://reviews.llvm.org/D35615



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


[Lldb-commits] [PATCH] D35666: Add data formatter for libc++ std::queue

2017-10-31 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

This looks good.


https://reviews.llvm.org/D35666



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


[Lldb-commits] [lldb] r317043 - Modernize the example cmdtemplate.py.

2017-10-31 Thread Jim Ingham via lldb-commits
Author: jingham
Date: Tue Oct 31 15:38:24 2017
New Revision: 317043

URL: http://llvm.org/viewvc/llvm-project?rev=317043&view=rev
Log:
Modernize the example cmdtemplate.py.

This version relies on a newer and more convenient way
to use a class to implement a command.  It has been in place
since early 2015, so it should be pretty safe to use.

Modified:
lldb/trunk/examples/python/cmdtemplate.py

Modified: lldb/trunk/examples/python/cmdtemplate.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/examples/python/cmdtemplate.py?rev=317043&r1=317042&r2=317043&view=diff
==
--- lldb/trunk/examples/python/cmdtemplate.py (original)
+++ lldb/trunk/examples/python/cmdtemplate.py Tue Oct 31 15:38:24 2017
@@ -14,99 +14,109 @@ import commands
 import optparse
 import shlex
 
+class FrameStatCommand:
+def create_options(self):
 
-def create_framestats_options():
-usage = "usage: %prog [options]"
-description = '''This command is meant to be an example of how to make an 
LLDB command that
+usage = "usage: %prog [options]"
+description = '''This command is meant to be an example of how to make 
an LLDB command that
 does something useful, follows best practices, and exploits the SB API.
 Specifically, this command computes the aggregate and average size of the 
variables in the current frame
 and allows you to tweak exactly which variables are to be accounted in the 
computation.
 '''
-parser = optparse.OptionParser(
-description=description,
-prog='framestats',
-usage=usage)
-parser.add_option(
-'-i',
-'--in-scope',
-action='store_true',
-dest='inscope',
-help='in_scope_only = True',
-default=False)
-parser.add_option(
-'-a',
-'--arguments',
-action='store_true',
-dest='arguments',
-help='arguments = True',
-default=False)
-parser.add_option(
-'-l',
-'--locals',
-action='store_true',
-dest='locals',
-help='locals = True',
-default=False)
-parser.add_option(
-'-s',
-'--statics',
-action='store_true',
-dest='statics',
-help='statics = True',
-default=False)
-return parser
-
-
-def the_framestats_command(debugger, command, result, dict):
-# Use the Shell Lexer to properly parse up command options just like a
-# shell would
-command_args = shlex.split(command)
-parser = create_framestats_options()
-try:
-(options, args) = parser.parse_args(command_args)
-except:
-# if you don't handle exceptions, passing an incorrect argument to the 
OptionParser will cause LLDB to exit
-# (courtesy of OptParse dealing with argument errors by throwing 
SystemExit)
-result.SetError("option parsing failed")
-return
-
-# in a command - the lldb.* convenience variables are not to be used
-# and their values (if any) are undefined
-# this is the best practice to access those objects from within a command
-target = debugger.GetSelectedTarget()
-process = target.GetProcess()
-thread = process.GetSelectedThread()
-frame = thread.GetSelectedFrame()
-if not frame.IsValid():
-return "no frame here"
-# from now on, replace lldb..whatever with .whatever
-variables_list = frame.GetVariables(
-options.arguments,
-options.locals,
-options.statics,
-options.inscope)
-variables_count = variables_list.GetSize()
-if variables_count == 0:
-print >> result, "no variables here"
-return
-total_size = 0
-for i in range(0, variables_count):
-variable = variables_list.GetValueAtIndex(i)
-variable_type = variable.GetType()
-total_size = total_size + variable_type.GetByteSize()
-average_size = float(total_size) / variables_count
-print >>result, "Your frame has %d variables. Their total size is %d 
bytes. The average size is %f bytes" % (
-variables_count, total_size, average_size)
-# not returning anything is akin to returning success
+
+# Pass add_help_option = False, since this keeps the command in line 
with lldb commands, 
+# and we wire up "help command" to work by providing the long & short 
help methods below.
+self.parser = optparse.OptionParser(
+description = description,
+prog = 'framestats',
+usage = usage,
+add_help_option = False)
+
+self.parser.add_option(
+'-i',
+'--in-scope',
+action = 'store_true',
+dest = 'inscope',
+help = 'in_scope_only = True',
+default = True)
+
+self.parser.add_option(
+'-a',
+'--arguments',
+action = 'store_true',
+dest = 'arguments',
+help = 'argument

[Lldb-commits] [lldb] r317067 - Remove Sean Callanan from the CODE_OWNERS, he won't have time

2017-10-31 Thread Jason Molenda via lldb-commits
Author: jmolenda
Date: Tue Oct 31 18:38:42 2017
New Revision: 317067

URL: http://llvm.org/viewvc/llvm-project?rev=317067&view=rev
Log:
Remove Sean Callanan from the CODE_OWNERS, he won't have time
to participate in lldb going forward.  Jim Ingham is adopting
the areas he was responsible for.

Modified:
lldb/trunk/CODE_OWNERS.txt

Modified: lldb/trunk/CODE_OWNERS.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/CODE_OWNERS.txt?rev=317067&r1=317066&r2=317067&view=diff
==
--- lldb/trunk/CODE_OWNERS.txt (original)
+++ lldb/trunk/CODE_OWNERS.txt Tue Oct 31 18:38:42 2017
@@ -8,10 +8,6 @@ beautification by scripts.  The fields a
 (W), PGP key ID and fingerprint (P), description (D), and snail-mail address
 (S).
 
-N: Sean Callanan
-E: scalla...@apple.com
-D: Expression evaluator, IR interpreter, Clang integration
-
 N: Greg Clayton
 E: clayb...@gmail.com
 D: Overall LLDB architecture, Host (common+macosx), Symbol, API, ABI, 
Mac-specific code, 
@@ -22,6 +18,7 @@ N: Jim Ingham
 E: jing...@apple.com
 D: Overall LLDB architecture, Thread plans, Expression parser, ValueObject, 
Breakpoints, ABI
 D: Watchpoints, Trampolines, Target, Command Interpreter, C++ / Objective C 
Language runtime
+D: Expression evaluator, IR interpreter, Clang integration
 D: Data Formatters
 
 N: Ilia K


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


[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Don Hinton via Phabricator via lldb-commits
hintonda added a comment.



In https://reviews.llvm.org/D39436#911875, @clayborg wrote:

> A few general things: don't modify FileSpec, we have many of these objects 
> and we can't increase their size without directly affecting memory usage. 
> FileSpec objects represent one file on disk, not multiple. We should make a 
> new class that contains a FileSpec and a regex, but not put it into FileSpec.


Will do...

Btw, if size is an issue, would it make sense to add a type to the enum 
declarations?  e.g.:

  diff --git a/include/lldb/Utility/FileSpec.h b/include/lldb/Utility/FileSpec.h
  index 67926d01e..55d44d840 100644
  --- a/include/lldb/Utility/FileSpec.h
  +++ b/include/lldb/Utility/FileSpec.h
  @@ -61,7 +61,7 @@ namespace lldb_private {
   //--
   class FileSpec {
   public:
  -  enum PathSyntax {
  +  enum PathSyntax : unsigned char {
   ePathSyntaxPosix,
   ePathSyntaxWindows,
   ePathSyntaxHostNative


https://reviews.llvm.org/D39436



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


[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

2017-10-31 Thread Don Hinton via Phabricator via lldb-commits
hintonda added a comment.

Actually, it wouldn't matter for this one due to alignment.


https://reviews.llvm.org/D39436



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