[Lldb-commits] [PATCH] D124471: Disable symbol on-demand feature for Windows

2022-04-27 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

FWIW, I don't think that this feature, as it is implemented right now, has any 
chance of working on windows. Windows does not have the equivalent of .symtab 
(only .dynsym), so we cannot use that as a poor man's debug info substitute -- 
the debug info is either there -- or it isn't.

The line-table based hydration triggers could still work, I suppose.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124471/new/

https://reviews.llvm.org/D124471

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

[Lldb-commits] [PATCH] D124409: Filter non-external static members from SBType::GetFieldAtIndex.

2022-04-27 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

For future reference, it's better to generate a full diff when uploading a 
patch manually. Here 

 are some ways to do that.

Apart from the inline comment this patch seems fine. We should also add a test 
case with the non-external static member. I think the test case could be as 
simple as declaring a global variable of the given type, printing it, and 
making sure the output is correct. You can use one of the assembly tests in 
`test/Shell/SymbolFile/DWARF/x86/` for inspiration. Or you can wait until I get 
a bit of time to create one...




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2672
   // Handle static members
-  if (attrs.is_external && attrs.member_byte_offset == UINT32_MAX) {
+  if (attrs.member_byte_offset == UINT32_MAX) {
 Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());

Have you run the test suite? I'd guess that you'd need to check for 
DW_AT_data_bit_offset here as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124409/new/

https://reviews.llvm.org/D124409

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

[Lldb-commits] [lldb] 089a1d9 - [lldb] Fix TestWithLimitDebugInfo.py

2022-04-27 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2022-04-27T15:08:58+02:00
New Revision: 089a1d9deba5579487d77c97241b3f9e5e3d4745

URL: 
https://github.com/llvm/llvm-project/commit/089a1d9deba5579487d77c97241b3f9e5e3d4745
DIFF: 
https://github.com/llvm/llvm-project/commit/089a1d9deba5579487d77c97241b3f9e5e3d4745.diff

LOG: [lldb] Fix TestWithLimitDebugInfo.py

The test was broken (in the sense that it was not testing what it was
supposed to test) in two ways:
- a Makefile refactor caused it to stop being built with
  -flimit-debug-info
- clang's constructor homing changed the "home" of the type

This patch fixes the Makefile, and modifies the source code to produce
the same result with both type homing strategies. Due to constructor
homing I had to use a different implicitly-defined function for the test
-- I chose the assignment operator.

I also added some sanity checks to the test to ensure that the test is
indeed operating on limited debug info.

Added: 


Modified: 
lldb/test/API/lang/cpp/limit-debug-info/Makefile
lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
lldb/test/API/lang/cpp/limit-debug-info/base.cpp
lldb/test/API/lang/cpp/limit-debug-info/base.h
lldb/test/API/lang/cpp/limit-debug-info/derived.cpp
lldb/test/API/lang/cpp/limit-debug-info/derived.h
lldb/test/API/lang/cpp/limit-debug-info/main.cpp

Removed: 




diff  --git a/lldb/test/API/lang/cpp/limit-debug-info/Makefile 
b/lldb/test/API/lang/cpp/limit-debug-info/Makefile
index ba7e01537522..30230b3469ac 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/Makefile
+++ b/lldb/test/API/lang/cpp/limit-debug-info/Makefile
@@ -1,5 +1,5 @@
 CXX_SOURCES = main.cpp derived.cpp base.cpp
 
-CFLAGS_EXTRAS := $(LIMIT_DEBUG_INFO_FLAGS)
+CFLAGS_EXTRAS = $(LIMIT_DEBUG_INFO_FLAGS)
 
 include Makefile.rules

diff  --git a/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py 
b/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
index 69caf94bd33a..9956bed5eb50 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
+++ b/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
@@ -8,10 +8,12 @@ class TestWithLimitDebugInfo(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-@skipIf(debug_info=no_match(["dwarf"]))
+@add_test_categories(["dwarf", "dwo"])
 def test_limit_debug_info(self):
 self.build()
 
+self._check_info_is_limited()
+
 src_file = os.path.join(self.getSourceDir(), "main.cpp")
 src_file_spec = lldb.SBFileSpec(src_file)
 self.assertTrue(src_file_spec.IsValid(), "breakpoint file")
@@ -52,6 +54,12 @@ def test_limit_debug_info(self):
 self.assertTrue(
 v2.IsValid(),
 "'expr this' results in a valid SBValue object")
-self.assertTrue(
-v2.GetError().Success(),
+self.assertSuccess(
+v2.GetError(),
 "'expr this' succeeds without an error.")
+
+def _check_info_is_limited(self):
+target = self.dbg.CreateTarget(self.getBuildArtifact("main.o"))
+self.assertTrue(target.IsValid())
+Foo = target.FindFirstType("Foo")
+self.assertFalse(Foo.IsValid())

diff  --git a/lldb/test/API/lang/cpp/limit-debug-info/base.cpp 
b/lldb/test/API/lang/cpp/limit-debug-info/base.cpp
index 4023bdbc64af..296864488820 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/base.cpp
+++ b/lldb/test/API/lang/cpp/limit-debug-info/base.cpp
@@ -1,5 +1,7 @@
 #include "base.h"
 
+FooNS::FooNS() : x(12345) {}
+
 void FooNS::bar() {
 x = 54321;
 }

diff  --git a/lldb/test/API/lang/cpp/limit-debug-info/base.h 
b/lldb/test/API/lang/cpp/limit-debug-info/base.h
index d3a09572bd25..f4da76701c78 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/base.h
+++ b/lldb/test/API/lang/cpp/limit-debug-info/base.h
@@ -5,6 +5,8 @@ class FooNS
 virtual char baz() = 0;
 
 protected:
+FooNS();
+
 int x;
 };
 

diff  --git a/lldb/test/API/lang/cpp/limit-debug-info/derived.cpp 
b/lldb/test/API/lang/cpp/limit-debug-info/derived.cpp
index 9d773593eb51..911fe3d9bc17 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/derived.cpp
+++ b/lldb/test/API/lang/cpp/limit-debug-info/derived.cpp
@@ -1,5 +1,10 @@
 #include "derived.h"
 
+Foo foo1;
+Foo foo2;
+
+Foo::Foo() { a = 12345; }
+
 char Foo::baz() {
 return (char)(x&0xff);
 }

diff  --git a/lldb/test/API/lang/cpp/limit-debug-info/derived.h 
b/lldb/test/API/lang/cpp/limit-debug-info/derived.h
index 46b3f83b9f74..8f95c52a595f 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/derived.h
+++ b/lldb/test/API/lang/cpp/limit-debug-info/derived.h
@@ -3,11 +3,17 @@
 class Foo : public FooNS
 {
 public:
-Foo() {
-a = 12345;
+Foo();
+
+// Deliberately defined by hand.
+Foo &operator=(const Foo &rhs) {
+  a = rhs.a;
+  return *this;
 }
 
 char baz() override;
 int a;

[Lldb-commits] [lldb] f513b5f - [lldb] Make test names unique

2022-04-27 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2022-04-27T15:08:58+02:00
New Revision: f513b5fc47df5c040a257ab6f4643942828fa610

URL: 
https://github.com/llvm/llvm-project/commit/f513b5fc47df5c040a257ab6f4643942828fa610
DIFF: 
https://github.com/llvm/llvm-project/commit/f513b5fc47df5c040a257ab6f4643942828fa610.diff

LOG: [lldb] Make test names unique

Added: 
lldb/test/API/symbol_ondemand/shared_library/TestSharedLibOnDemand.py

Modified: 


Removed: 
lldb/test/API/symbol_ondemand/shared_library/TestSharedLib.py



diff  --git a/lldb/test/API/symbol_ondemand/shared_library/TestSharedLib.py 
b/lldb/test/API/symbol_ondemand/shared_library/TestSharedLibOnDemand.py
similarity index 100%
rename from lldb/test/API/symbol_ondemand/shared_library/TestSharedLib.py
rename to lldb/test/API/symbol_ondemand/shared_library/TestSharedLibOnDemand.py



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

[Lldb-commits] [PATCH] D124409: Filter non-external static members from SBType::GetFieldAtIndex.

2022-04-27 Thread Sigurður Ásgeirsson via Phabricator via lldb-commits 
siggi-alpheus added a comment.

Thanks for taking a look. Let me see about fixing the test failures and writing 
that test.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2672
   // Handle static members
-  if (attrs.is_external && attrs.member_byte_offset == UINT32_MAX) {
+  if (attrs.member_byte_offset == UINT32_MAX) {
 Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());

labath wrote:
> Have you run the test suite? I'd guess that you'd need to check for 
> DW_AT_data_bit_offset here as well.
Yeah, I ran the tests overnight and I see a couple of tests failing. Will see 
about fixing and writing a test for the case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124409/new/

https://reviews.llvm.org/D124409

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

[Lldb-commits] [lldb] eeaa3b5 - [lldb] Remove sanity check from TestWithLimitDebugInfo

2022-04-27 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2022-04-27T16:01:54+02:00
New Revision: eeaa3b5478539ec00817ff91e1603c1039376d02

URL: 
https://github.com/llvm/llvm-project/commit/eeaa3b5478539ec00817ff91e1603c1039376d02
DIFF: 
https://github.com/llvm/llvm-project/commit/eeaa3b5478539ec00817ff91e1603c1039376d02.diff

LOG: [lldb] Remove sanity check from TestWithLimitDebugInfo

The trick with opening the .o file does not work on arm (unhandled
relocations), and I can't think of a quick fix for that.

Added: 


Modified: 
lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py 
b/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
index 9956bed5eb50..03bb40271ab0 100644
--- a/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
+++ b/lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py
@@ -12,8 +12,6 @@ class TestWithLimitDebugInfo(TestBase):
 def test_limit_debug_info(self):
 self.build()
 
-self._check_info_is_limited()
-
 src_file = os.path.join(self.getSourceDir(), "main.cpp")
 src_file_spec = lldb.SBFileSpec(src_file)
 self.assertTrue(src_file_spec.IsValid(), "breakpoint file")
@@ -57,9 +55,3 @@ def test_limit_debug_info(self):
 self.assertSuccess(
 v2.GetError(),
 "'expr this' succeeds without an error.")
-
-def _check_info_is_limited(self):
-target = self.dbg.CreateTarget(self.getBuildArtifact("main.o"))
-self.assertTrue(target.IsValid())
-Foo = target.FindFirstType("Foo")
-self.assertFalse(Foo.IsValid())



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

[Lldb-commits] [PATCH] D124430: [lldb] Remove Python 2 checks from the test suite

2022-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D124430#3475136 , @clayborg wrote:

> Are we removing Python2 support?

Yep, AFAIK we were the only ones that still needed it. We dropped support 
downstream in the previous release and we just rebranched for the next one, so 
I'm no longer worried about merge conflicts for cherrypicks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124430/new/

https://reviews.llvm.org/D124430

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

[Lldb-commits] [lldb] 9053767 - Remove Python 2 support from the ScriptInterpreter plugin

2022-04-27 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-04-27T08:26:25-07:00
New Revision: 90537673302f13e92ffabba84901164c6b974b2d

URL: 
https://github.com/llvm/llvm-project/commit/90537673302f13e92ffabba84901164c6b974b2d
DIFF: 
https://github.com/llvm/llvm-project/commit/90537673302f13e92ffabba84901164c6b974b2d.diff

LOG: Remove Python 2 support from the ScriptInterpreter plugin

We dropped downstream support for Python 2 in the previous release. Now
that we have branched for the next release the window where this kind of
change could introduce conflicts is closing too. Start by getting rid of
Python 2 support in the Script Interpreter plugin.

Differential revision: https://reviews.llvm.org/D124429

Added: 


Modified: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index 7392968b7e707..ae61736fbbb3e 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -71,9 +71,7 @@ Expected 
python::As(Expected &&obj) {
 }
 
 static bool python_is_finalizing() {
-#if PY_MAJOR_VERSION == 2
-  return false;
-#elif PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION < 7
+#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION < 7
   return _Py_Finalizing != nullptr;
 #else
   return _Py_IsFinalizing();
@@ -97,12 +95,6 @@ void PythonObject::Reset() {
 Expected PythonObject::AsLongLong() const {
   if (!m_py_obj)
 return nullDeref();
-#if PY_MAJOR_VERSION < 3
-  if (!PyLong_Check(m_py_obj)) {
-PythonInteger i(PyRefType::Borrowed, m_py_obj);
-return i.AsLongLong();
-  }
-#endif
   assert(!PyErr_Occurred());
   long long r = PyLong_AsLongLong(m_py_obj);
   if (PyErr_Occurred())
@@ -113,12 +105,6 @@ Expected PythonObject::AsLongLong() const {
 Expected PythonObject::AsUnsignedLongLong() const {
   if (!m_py_obj)
 return nullDeref();
-#if PY_MAJOR_VERSION < 3
-  if (!PyLong_Check(m_py_obj)) {
-PythonInteger i(PyRefType::Borrowed, m_py_obj);
-return i.AsUnsignedLongLong();
-  }
-#endif
   assert(!PyErr_Occurred());
   long long r = PyLong_AsUnsignedLongLong(m_py_obj);
   if (PyErr_Occurred())
@@ -130,12 +116,6 @@ Expected PythonObject::AsUnsignedLongLong() 
const {
 Expected PythonObject::AsModuloUnsignedLongLong() const {
   if (!m_py_obj)
 return nullDeref();
-#if PY_MAJOR_VERSION < 3
-  if (!PyLong_Check(m_py_obj)) {
-PythonInteger i(PyRefType::Borrowed, m_py_obj);
-return i.AsModuloUnsignedLongLong();
-  }
-#endif
   assert(!PyErr_Occurred());
   unsigned long long r = PyLong_AsUnsignedLongLongMask(m_py_obj);
   if (PyErr_Occurred())
@@ -183,10 +163,8 @@ PyObjectType PythonObject::GetObjectType() const {
 return PyObjectType::Dictionary;
   if (PythonString::Check(m_py_obj))
 return PyObjectType::String;
-#if PY_MAJOR_VERSION >= 3
   if (PythonBytes::Check(m_py_obj))
 return PyObjectType::Bytes;
-#endif
   if (PythonByteArray::Check(m_py_obj))
 return PyObjectType::ByteArray;
   if (PythonBoolean::Check(m_py_obj))
@@ -282,9 +260,7 @@ PythonObject 
PythonObject::GetAttributeValue(llvm::StringRef attr) const {
 }
 
 StructuredData::ObjectSP PythonObject::CreateStructuredObject() const {
-#if PY_MAJOR_VERSION >= 3
   assert(PyGILState_Check());
-#endif
   switch (GetObjectType()) {
   case PyObjectType::Dictionary:
 return PythonDictionary(PyRefType::Borrowed, m_py_obj)
@@ -398,11 +374,7 @@ StructuredData::StringSP 
PythonByteArray::CreateStructuredString() const {
 // PythonString
 
 Expected PythonString::FromUTF8(llvm::StringRef string) {
-#if PY_MAJOR_VERSION >= 3
   PyObject *str = PyUnicode_FromStringAndSize(string.data(), string.size());
-#else
-  PyObject *str = PyString_FromStringAndSize(string.data(), string.size());
-#endif
   if (!str)
 return llvm::make_error();
   return Take(str);
@@ -416,35 +388,9 @@ bool PythonString::Check(PyObject *py_obj) {
 
   if (PyUnicode_Check(py_obj))
 return true;
-#if PY_MAJOR_VERSION < 3
-  if (PyString_Check(py_obj))
-return true;
-#endif
   return false;
 }
 
-void PythonString::Convert(PyRefType &type, PyObject *&py_obj) {
-#if PY_MAJOR_VERSION < 3
-  // In Python 2, Don't store PyUnicode objects directly, because we need
-  // access to their underlying character buffers which Python 2 doesn't
-  // provide.
-  if (PyUnicode_Check(py_obj)) {
-PyObject *s = PyUnicode_AsUTF8String(py_obj);
-if (s == nullptr) {
-  PyErr_C

[Lldb-commits] [PATCH] D124429: [lldb] Remove Python 2 support from the ScriptInterpreter plugin

2022-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG90537673302f: Remove Python 2 support from the 
ScriptInterpreter plugin (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124429/new/

https://reviews.llvm.org/D124429

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -51,11 +51,7 @@
 // callbacks. Because they're defined in libLLDB which we cannot link for the
 // unit test, we have a 'default' implementation here.
 
-#if PY_MAJOR_VERSION >= 3
 extern "C" PyObject *PyInit__lldb(void) { return nullptr; }
-#else
-extern "C" void init_lldb(void) {}
-#endif
 
 llvm::Expected lldb_private::LLDBSwigPythonBreakpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -164,18 +164,6 @@
 TEST_F(PythonDataObjectsTest, TestPythonInteger) {
   // Test that integers behave correctly when wrapped by a PythonInteger.
 
-#if PY_MAJOR_VERSION < 3
-  // Verify that `PythonInt` works correctly when given a PyInt object.
-  // Note that PyInt doesn't exist in Python 3.x, so this is only for 2.x
-  PyObject *py_int = PyInt_FromLong(12);
-  EXPECT_TRUE(PythonInteger::Check(py_int));
-  PythonInteger python_int(PyRefType::Owned, py_int);
-
-  EXPECT_EQ(PyObjectType::Integer, python_int.GetObjectType());
-  auto python_int_value = As(python_int);
-  EXPECT_THAT_EXPECTED(python_int_value, llvm::HasValue(12));
-#endif
-
   // Verify that `PythonInteger` works correctly when given a PyLong object.
   PyObject *py_long = PyLong_FromLong(12);
   EXPECT_TRUE(PythonInteger::Check(py_long));
@@ -225,13 +213,8 @@
   EXPECT_TRUE(PythonBytes::Check(py_bytes));
   PythonBytes python_bytes(PyRefType::Owned, py_bytes);
 
-#if PY_MAJOR_VERSION < 3
-  EXPECT_TRUE(PythonString::Check(py_bytes));
-  EXPECT_EQ(PyObjectType::String, python_bytes.GetObjectType());
-#else
   EXPECT_FALSE(PythonString::Check(py_bytes));
   EXPECT_EQ(PyObjectType::Bytes, python_bytes.GetObjectType());
-#endif
 
   llvm::ArrayRef bytes = python_bytes.GetBytes();
   EXPECT_EQ(bytes.size(), strlen(test_bytes));
@@ -258,23 +241,12 @@
   static const char *test_string = "PythonDataObjectsTest::TestPythonString1";
   static const char *test_string2 = "PythonDataObjectsTest::TestPythonString2";
 
-#if PY_MAJOR_VERSION < 3
-  // Verify that `PythonString` works correctly when given a PyString object.
-  // Note that PyString doesn't exist in Python 3.x, so this is only for 2.x
-  PyObject *py_string = PyString_FromString(test_string);
-  EXPECT_TRUE(PythonString::Check(py_string));
-  PythonString python_string(PyRefType::Owned, py_string);
-
-  EXPECT_EQ(PyObjectType::String, python_string.GetObjectType());
-  EXPECT_STREQ(test_string, python_string.GetString().data());
-#else
   // Verify that `PythonString` works correctly when given a PyUnicode object.
   PyObject *py_unicode = PyUnicode_FromString(test_string);
   EXPECT_TRUE(PythonString::Check(py_unicode));
   PythonString python_unicode(PyRefType::Owned, py_unicode);
   EXPECT_EQ(PyObjectType::String, python_unicode.GetObjectType());
   EXPECT_STREQ(test_string, python_unicode.GetString().data());
-#endif
 
   // Test that creating a `PythonString` object works correctly with the
   // string constructor
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -60,17 +60,10 @@
 LLDB_PLUGIN_DEFINE(ScriptInterpreterPython)
 
 // Defined in the SWIG source file
-#if PY_MAJOR_VERSION >= 3
 extern "C" PyObject *PyInit__lldb(void);
 
 #define LLDBSwigPyInit PyInit__lldb
 
-#else
-extern "C" void init_lldb(void);
-
-#define LLDBSwigPyInit init_lldb
-#endif
-
 #if defined(_WIN32)
 // Don't mess with the signal handlers on Windows.
 #define LLDB_USE_PYTHON_SET_INTERRUPT 0
@@ -145,11 +138,7 @@
 privat

[Lldb-commits] [lldb] 0e9af88 - Remove Python 2 checks from the test suite

2022-04-27 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-04-27T08:26:26-07:00
New Revision: 0e9af88b7099fa0588661b5b8d5024b2f25f68d8

URL: 
https://github.com/llvm/llvm-project/commit/0e9af88b7099fa0588661b5b8d5024b2f25f68d8
DIFF: 
https://github.com/llvm/llvm-project/commit/0e9af88b7099fa0588661b5b8d5024b2f25f68d8.diff

LOG: Remove Python 2 checks from the test suite

We dropped downstream support for Python 2 in the previous release. Now
that we have branched for the next release the window where this kind of
change could introduce conflicts is closing too. Remove Python 2 checks
from the test suite.

Differential revision: https://reviews.llvm.org/D124429

Added: 


Modified: 
lldb/test/API/functionalities/step_scripted/TestStepScripted.py
lldb/test/API/lldbtest.py
lldb/test/API/macosx/nslog/TestDarwinNSLogOutput.py
lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
lldb/test/API/python_api/file_handle/TestFileHandle.py
lldb/test/API/terminal/TestSTTYBeforeAndAfter.py

Removed: 




diff  --git a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py 
b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
index dc4e83fd57a38..fdd00861809d1 100644
--- a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
+++ b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
@@ -19,13 +19,13 @@ def setUp(self):
 self.runCmd("command script import Steps.py")
 
 def test_standard_step_out(self):
-"""Tests stepping with the scripted thread plan laying over a standard 
+"""Tests stepping with the scripted thread plan laying over a standard
 thread plan for stepping out."""
 self.build()
 self.step_out_with_scripted_plan("Steps.StepOut")
 
 def test_scripted_step_out(self):
-"""Tests stepping with the scripted thread plan laying over an another 
+"""Tests stepping with the scripted thread plan laying over an another
 scripted thread plan for stepping out."""
 self.build()
 self.step_out_with_scripted_plan("Steps.StepScripted")
@@ -54,23 +54,23 @@ def test_misspelled_plan_name(self):
 stop_id = process.GetStopID()
 # Pass a non-existent class for the plan class:
 err = thread.StepUsingScriptedThreadPlan("NoSuchModule.NoSuchPlan")
-
+
 # Make sure we got a good error:
 self.assertTrue(err.Fail(), "We got a failure state")
 msg = err.GetCString()
 self.assertIn("NoSuchModule.NoSuchPlan", msg, "Mentioned missing 
class")
-
+
 # Make sure we didn't let the process run:
 self.assertEqual(stop_id, process.GetStopID(), "Process didn't run")
-
+
 def test_checking_variable(self):
 """Test that we can call SBValue API's from a scripted thread plan - 
using SBAPI's to step"""
 self.do_test_checking_variable(False)
-
+
 def test_checking_variable_cli(self):
 """Test that we can call SBValue API's from a scripted thread plan - 
using cli to step"""
 self.do_test_checking_variable(True)
-
+
 def do_test_checking_variable(self, use_cli):
 self.build()
 (target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
@@ -101,7 +101,7 @@ def do_test_checking_variable(self, use_cli):
 
 # We should not have exited:
 self.assertEqual(process.GetState(), lldb.eStateStopped, "We are 
stopped")
-
+
 # We should still be in foo:
 self.assertEqual("foo", frame.GetFunctionName())
 
@@ -127,18 +127,14 @@ def run_step(self, stop_others_value, run_mode, token):
 print(Steps.StepReportsStopOthers.stop_mode_dict)
 value = Steps.StepReportsStopOthers.stop_mode_dict[token]
 self.assertEqual(value, stop_others_value, "Stop others has the 
correct value.")
-
+
 def do_test_stop_others(self):
 self.build()
 (target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
 
"Set a breakpoint here",
 
self.main_source_file)
 # First run with stop others false and see that we got that.
-thread_id = ""
-if sys.version_info.major == 2:
-thread_id = str(threading._get_ident())
-else:
-thread_id = str(threading.get_ident())
+thread_id = str(threading.get_ident())
 
 # all-threads should set stop others to False.
 self.run_step(False, "all-threads", thread_id)
@@ -152,13 +148,8 @@ def do_test_stop_others(self):
 # The target.process.run-all-threads should override this:
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
-
+
 interp.HandleC

[Lldb-commits] [PATCH] D124430: [lldb] Remove Python 2 checks from the test suite

2022-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere closed this revision.
JDevlieghere added a comment.

0e9af88b7099 



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124430/new/

https://reviews.llvm.org/D124430

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

[Lldb-commits] [PATCH] D124535: [lldb] Fix crash when launching in terminal

2022-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added reviewers: mgorny, labath.
Herald added a project: All.
JDevlieghere requested review of this revision.

This patch fixes a crash when using `process launch -t` to launch the inferior 
from a TTY. The issue is that on Darwin, Host.mm is calling 
`ConnectionFileDescriptor::Connect` without a `socket_id_callback_type`. The 
overload passes `nullptr` as the function ref, which gets called 
unconditionally as the `socket_id_callback`. One potential way to fix this is 
to change all the lambdas to include a null check, but instead I went with a 
NOOP.

The patch doesn't include a test even though it's trivial to write one. The 
reason is that we really don't want the test suite spawning (and leaving 
around) Terminal.app instances on every run.


https://reviews.llvm.org/D124535

Files:
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -122,7 +122,8 @@
 
 ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
Status *error_ptr) {
-  return Connect(path, nullptr, error_ptr);
+  return Connect(
+  path, [](llvm::StringRef) {}, error_ptr);
 }
 
 ConnectionStatus


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -122,7 +122,8 @@
 
 ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
Status *error_ptr) {
-  return Connect(path, nullptr, error_ptr);
+  return Connect(
+  path, [](llvm::StringRef) {}, error_ptr);
 }
 
 ConnectionStatus
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] afd6390 - [LLDB][NativePDB] Minor fix ParseInlinesite.

2022-04-27 Thread Zequan Wu via lldb-commits 

Author: Zequan Wu
Date: 2022-04-27T10:56:03-07:00
New Revision: afd639071bb32baae4ca390b3f0f5ab700d83222

URL: 
https://github.com/llvm/llvm-project/commit/afd639071bb32baae4ca390b3f0f5ab700d83222
DIFF: 
https://github.com/llvm/llvm-project/commit/afd639071bb32baae4ca390b3f0f5ab700d83222.diff

LOG: [LLDB][NativePDB] Minor fix ParseInlinesite.

- Don't reset cur_line_offset to llvm::None when we don't have 
next_line_offset, because we may need to reuse it in new range after a code end.
- Don't use CombineConsecutiveEntriesWithEqualData for inline_site_sp->ranges, 
because that will combine consecutive entries with same data in the vector 
regardless of the entry's range. Originally, I thought that it only combine 
consecutive entries if adjacent entries' ranges are adjoining or intersecting 
with each other.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index fcbf667798644..22d8977e117b0 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1311,8 +1311,8 @@ void 
SymbolFileNativePDB::ParseInlineSite(PdbCompilandSymId id,
   int32_t line_offset = 0;
   llvm::Optional code_offset_base;
   llvm::Optional code_offset_end;
-  llvm::Optional cur_line_offset;
-  llvm::Optional next_line_offset;
+  llvm::Optional cur_line_offset;
+  llvm::Optional next_line_offset;
   llvm::Optional next_file_offset;
 
   bool is_terminal_entry = false;
@@ -1384,9 +1384,12 @@ void 
SymbolFileNativePDB::ParseInlineSite(PdbCompilandSymId id,
   // Set base, end, file offset and line offset for next range.
   if (next_file_offset)
 file_offset = *next_file_offset;
-  cur_line_offset = next_line_offset ? next_line_offset : llvm::None;
+  if (next_line_offset) {
+cur_line_offset = next_line_offset;
+next_line_offset = llvm::None;
+  }
   code_offset_base = is_terminal_entry ? llvm::None : code_offset_end;
-  code_offset_end = next_line_offset = next_file_offset = llvm::None;
+  code_offset_end = next_file_offset = llvm::None;
 }
 if (code_offset_base && cur_line_offset) {
   if (is_terminal_entry) {
@@ -1410,7 +1413,6 @@ void 
SymbolFileNativePDB::ParseInlineSite(PdbCompilandSymId id,
   }
 
   inline_site_sp->ranges.Sort();
-  inline_site_sp->ranges.CombineConsecutiveEntriesWithEqualData();
 
   // Get the inlined function callsite info.
   std::unique_ptr callsite_up;



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

[Lldb-commits] [PATCH] D124471: Disable symbol on-demand feature for Windows

2022-04-27 Thread jeffrey tan via Phabricator via lldb-commits 
yinghuitan added a comment.

@labath, that's a good point. If I remember correctly, exported function 
symbols will be included in Windows COFF export address table (which I assume 
will be parsed by lldb symtab). So in theory, if we change the tests (or add 
new tests) to set symbolic function breakpoint on exported function symbols, 
they might work on Windows. I can spend some time looking once I got a Windows 
machine. But I agree it is a lower priority based on your input.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124471/new/

https://reviews.llvm.org/D124471

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

[Lldb-commits] [PATCH] D124409: Filter non-external static members from SBType::GetFieldAtIndex.

2022-04-27 Thread Sigurður Ásgeirsson via Phabricator via lldb-commits 
siggi-alpheus updated this revision to Diff 425585.
siggi-alpheus added a comment.

Now passes all tests, still needs a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124409/new/

https://reviews.llvm.org/D124409

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2444,8 +2444,6 @@
   /// structure.
   uint32_t member_byte_offset;
   bool is_artificial = false;
-  /// On DW_TAG_members, this means the member is static.
-  bool is_external = false;
 };
 
 /// Parsed form of all attributes that are relevant for parsing Objective-C
@@ -2521,9 +2519,6 @@
   case DW_AT_artificial:
 is_artificial = form_value.Boolean();
 break;
-  case DW_AT_external:
-is_external = form_value.Boolean();
-break;
   default:
 break;
   }
@@ -2668,8 +2663,10 @@
   if (class_is_objc_object_or_interface)
 attrs.accessibility = eAccessNone;
 
-  // Handle static members
-  if (attrs.is_external && attrs.member_byte_offset == UINT32_MAX) {
+  // Handle static members, which is any member that doesn't have a bit or a
+  // byte member offset.
+  if (attrs.member_byte_offset == UINT32_MAX &&
+  attrs.data_bit_offset == UINT64_MAX) {
 Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
 
 if (var_type) {


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2444,8 +2444,6 @@
   /// structure.
   uint32_t member_byte_offset;
   bool is_artificial = false;
-  /// On DW_TAG_members, this means the member is static.
-  bool is_external = false;
 };
 
 /// Parsed form of all attributes that are relevant for parsing Objective-C
@@ -2521,9 +2519,6 @@
   case DW_AT_artificial:
 is_artificial = form_value.Boolean();
 break;
-  case DW_AT_external:
-is_external = form_value.Boolean();
-break;
   default:
 break;
   }
@@ -2668,8 +2663,10 @@
   if (class_is_objc_object_or_interface)
 attrs.accessibility = eAccessNone;
 
-  // Handle static members
-  if (attrs.is_external && attrs.member_byte_offset == UINT32_MAX) {
+  // Handle static members, which is any member that doesn't have a bit or a
+  // byte member offset.
+  if (attrs.member_byte_offset == UINT32_MAX &&
+  attrs.data_bit_offset == UINT64_MAX) {
 Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
 
 if (var_type) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D124409: Filter non-external static members from SBType::GetFieldAtIndex.

2022-04-27 Thread Sigurður Ásgeirsson via Phabricator via lldb-commits 
siggi-alpheus marked an inline comment as done.
siggi-alpheus added a comment.

Now passes all tests - but I still need to write a test for the problem.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124409/new/

https://reviews.llvm.org/D124409

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

[Lldb-commits] [PATCH] D124568: [lldb] Fix escaping when launching in terminal with AppleScript

2022-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added a reviewer: aprantl.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
JDevlieghere requested review of this revision.

Fix escaping when launching in terminal with AppleScript. The invocation we're 
building up is wrapped in single quotes when passed to bash and wrapped in 
double quotes for AppleScript.

Here's an example invocation with the new escaping:

  tell application "Terminal"
 activate
  do script "/bin/bash -c 'arch -arch arm64 
'/Users/jonas/llvm/build-ra/bin/darwin-debug' --unix-socket=/tmp/dL2jSh 
--arch=arm64 --working-dir \"/private/tmp/with spaces\" --disable-aslr 
--env=\"OS_ACTIVITY_DT_MODE=enable\" --  
\"/Users/jonas/llvm/build-ra/bin/lldb-instr\" \"foo bar\" \"baz quux\" ; echo 
Process exited with status $?';exit"
  end tell

Previously we were using unescaped single quotes which resulted in the whole 
bash invocation being passed in pieces. That works most of the time but breaks 
when you have a space in your current working directory for example.


https://reviews.llvm.org/D124568

Files:
  lldb/source/Host/macosx/objcxx/Host.mm


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -219,11 +219,11 @@
 
   FileSpec working_dir{launch_info.GetWorkingDirectory()};
   if (working_dir)
-command.Printf(" --working-dir '%s'", working_dir.GetCString());
+command.Printf(" --working-dir \\\"%s\\\"", working_dir.GetCString());
   else {
 char cwd[PATH_MAX];
 if (getcwd(cwd, PATH_MAX))
-  command.Printf(" --working-dir '%s'", cwd);
+  command.Printf(" --working-dir \\\"%s\\\"", cwd);
   }
 
   if (launch_info.GetFlags().Test(eLaunchFlagDisableASLR))
@@ -239,7 +239,7 @@
   for (const auto &KV : launch_info.GetEnvironment()) {
 auto host_entry = host_env.find(KV.first());
 if (host_entry == host_env.end() || host_entry->second != KV.second)
-  command.Format(" --env='{0}'", Environment::compose(KV));
+  command.Format(" --env=\\\"{0}\\\"", Environment::compose(KV));
   }
 
   command.PutCString(" -- ");
@@ -248,12 +248,12 @@
   if (argv) {
 for (size_t i = 0; argv[i] != NULL; ++i) {
   if (i == 0)
-command.Printf(" '%s'", exe_path);
+command.Printf(" \\\"%s\\\"", exe_path);
   else
-command.Printf(" '%s'", argv[i]);
+command.Printf(" \\\"%s\\\"", argv[i]);
 }
   } else {
-command.Printf(" '%s'", exe_path);
+command.Printf(" \\\"%s\\\"", exe_path);
   }
   command.PutCString(" ; echo Process exited with status $?");
   if (launch_info.GetFlags().Test(lldb::eLaunchFlagCloseTTYOnExit))
@@ -263,6 +263,9 @@
 
   applescript_source.Printf(applscript_in_new_tty,
 command.GetString().str().c_str());
+
+  printf("%s\n", applescript_source.GetString().str().c_str());
+
   NSAppleScript *applescript = [[NSAppleScript alloc]
   initWithSource:[NSString stringWithCString:applescript_source.GetString()
  .str()


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -219,11 +219,11 @@
 
   FileSpec working_dir{launch_info.GetWorkingDirectory()};
   if (working_dir)
-command.Printf(" --working-dir '%s'", working_dir.GetCString());
+command.Printf(" --working-dir \\\"%s\\\"", working_dir.GetCString());
   else {
 char cwd[PATH_MAX];
 if (getcwd(cwd, PATH_MAX))
-  command.Printf(" --working-dir '%s'", cwd);
+  command.Printf(" --working-dir \\\"%s\\\"", cwd);
   }
 
   if (launch_info.GetFlags().Test(eLaunchFlagDisableASLR))
@@ -239,7 +239,7 @@
   for (const auto &KV : launch_info.GetEnvironment()) {
 auto host_entry = host_env.find(KV.first());
 if (host_entry == host_env.end() || host_entry->second != KV.second)
-  command.Format(" --env='{0}'", Environment::compose(KV));
+  command.Format(" --env=\\\"{0}\\\"", Environment::compose(KV));
   }
 
   command.PutCString(" -- ");
@@ -248,12 +248,12 @@
   if (argv) {
 for (size_t i = 0; argv[i] != NULL; ++i) {
   if (i == 0)
-command.Printf(" '%s'", exe_path);
+command.Printf(" \\\"%s\\\"", exe_path);
   else
-command.Printf(" '%s'", argv[i]);
+command.Printf(" \\\"%s\\\"", argv[i]);
 }
   } else {
-command.Printf(" '%s'", exe_path);
+command.Printf(" \\\"%s\\\"", exe_path);
   }
   command.PutCString(" ; echo Process exited with status $?");
   if (launch_info.GetFlags().Test(lldb::eLaunchFlagCloseTTYOnExit))
@@ -263,6 +263,9 @@
 
   applescript_source.Printf(applscript_in_new_tty,
 command.GetString().str().c_str());
+
+  printf("%s\n", applescript_source.GetString().str().c_str());

[Lldb-commits] [PATCH] D124568: [lldb] Fix escaping when launching in terminal with AppleScript

2022-04-27 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: lldb/source/Host/macosx/objcxx/Host.mm:222
   if (working_dir)
-command.Printf(" --working-dir '%s'", working_dir.GetCString());
+command.Printf(" --working-dir \\\"%s\\\"", working_dir.GetCString());
   else {

Should we use raw C++ string literals for this?
https://en.cppreference.com/w/cpp/language/string_literal



Comment at: lldb/source/Host/macosx/objcxx/Host.mm:267
+
+  printf("%s\n", applescript_source.GetString().str().c_str());
+

Is this leftover debugging code?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124568/new/

https://reviews.llvm.org/D124568

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

[Lldb-commits] [PATCH] D124568: [lldb] Fix escaping when launching in terminal with AppleScript

2022-04-27 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added a comment.

I have some trivial suggestions inside, but otherwise, thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124568/new/

https://reviews.llvm.org/D124568

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

[Lldb-commits] [PATCH] D124568: [lldb] Fix escaping when launching in terminal with AppleScript

2022-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Host/macosx/objcxx/Host.mm:267
+
+  printf("%s\n", applescript_source.GetString().str().c_str());
+

aprantl wrote:
> Is this leftover debugging code?
Yup, I added that to generate the invocation I put in the summary and forgot to 
take it out again.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124568/new/

https://reviews.llvm.org/D124568

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

[Lldb-commits] [PATCH] D124568: [lldb] Fix escaping when launching in terminal with AppleScript

2022-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 425638.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.

Address Adrian's code review feedback.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124568/new/

https://reviews.llvm.org/D124568

Files:
  lldb/source/Host/macosx/objcxx/Host.mm


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -212,18 +212,18 @@
   arch_spec.GetCore() != ArchSpec::eCore_x86_64_x86_64h)
 command.Printf("arch -arch %s ", arch_spec.GetArchitectureName());
 
-  command.Printf("'%s' --unix-socket=%s", launcher_path, unix_socket_name);
+  command.Printf(R"(\"%s\" --unix-socket=%s)", launcher_path, 
unix_socket_name);
 
   if (arch_spec.IsValid())
 command.Printf(" --arch=%s", arch_spec.GetArchitectureName());
 
   FileSpec working_dir{launch_info.GetWorkingDirectory()};
   if (working_dir)
-command.Printf(" --working-dir '%s'", working_dir.GetCString());
+command.Printf(R"( --working-dir \"%s\")", working_dir.GetCString());
   else {
 char cwd[PATH_MAX];
 if (getcwd(cwd, PATH_MAX))
-  command.Printf(" --working-dir '%s'", cwd);
+  command.Printf(R"( --working-dir \"%s\")", cwd);
   }
 
   if (launch_info.GetFlags().Test(eLaunchFlagDisableASLR))
@@ -239,7 +239,7 @@
   for (const auto &KV : launch_info.GetEnvironment()) {
 auto host_entry = host_env.find(KV.first());
 if (host_entry == host_env.end() || host_entry->second != KV.second)
-  command.Format(" --env='{0}'", Environment::compose(KV));
+  command.Format(R"( --env=\"{0}\")", Environment::compose(KV));
   }
 
   command.PutCString(" -- ");
@@ -248,12 +248,12 @@
   if (argv) {
 for (size_t i = 0; argv[i] != NULL; ++i) {
   if (i == 0)
-command.Printf(" '%s'", exe_path);
+command.Printf(R"( \"%s\")", exe_path);
   else
-command.Printf(" '%s'", argv[i]);
+command.Printf(R"( \"%s\")", argv[i]);
 }
   } else {
-command.Printf(" '%s'", exe_path);
+command.Printf(R"( \"%s\")", exe_path);
   }
   command.PutCString(" ; echo Process exited with status $?");
   if (launch_info.GetFlags().Test(lldb::eLaunchFlagCloseTTYOnExit))
@@ -263,6 +263,7 @@
 
   applescript_source.Printf(applscript_in_new_tty,
 command.GetString().str().c_str());
+
   NSAppleScript *applescript = [[NSAppleScript alloc]
   initWithSource:[NSString stringWithCString:applescript_source.GetString()
  .str()


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -212,18 +212,18 @@
   arch_spec.GetCore() != ArchSpec::eCore_x86_64_x86_64h)
 command.Printf("arch -arch %s ", arch_spec.GetArchitectureName());
 
-  command.Printf("'%s' --unix-socket=%s", launcher_path, unix_socket_name);
+  command.Printf(R"(\"%s\" --unix-socket=%s)", launcher_path, unix_socket_name);
 
   if (arch_spec.IsValid())
 command.Printf(" --arch=%s", arch_spec.GetArchitectureName());
 
   FileSpec working_dir{launch_info.GetWorkingDirectory()};
   if (working_dir)
-command.Printf(" --working-dir '%s'", working_dir.GetCString());
+command.Printf(R"( --working-dir \"%s\")", working_dir.GetCString());
   else {
 char cwd[PATH_MAX];
 if (getcwd(cwd, PATH_MAX))
-  command.Printf(" --working-dir '%s'", cwd);
+  command.Printf(R"( --working-dir \"%s\")", cwd);
   }
 
   if (launch_info.GetFlags().Test(eLaunchFlagDisableASLR))
@@ -239,7 +239,7 @@
   for (const auto &KV : launch_info.GetEnvironment()) {
 auto host_entry = host_env.find(KV.first());
 if (host_entry == host_env.end() || host_entry->second != KV.second)
-  command.Format(" --env='{0}'", Environment::compose(KV));
+  command.Format(R"( --env=\"{0}\")", Environment::compose(KV));
   }
 
   command.PutCString(" -- ");
@@ -248,12 +248,12 @@
   if (argv) {
 for (size_t i = 0; argv[i] != NULL; ++i) {
   if (i == 0)
-command.Printf(" '%s'", exe_path);
+command.Printf(R"( \"%s\")", exe_path);
   else
-command.Printf(" '%s'", argv[i]);
+command.Printf(R"( \"%s\")", argv[i]);
 }
   } else {
-command.Printf(" '%s'", exe_path);
+command.Printf(R"( \"%s\")", exe_path);
   }
   command.PutCString(" ; echo Process exited with status $?");
   if (launch_info.GetFlags().Test(lldb::eLaunchFlagCloseTTYOnExit))
@@ -263,6 +263,7 @@
 
   applescript_source.Printf(applscript_in_new_tty,
 command.GetString().str().c_str());
+
   NSAppleScript *applescript = [[NSAppleScript alloc]
   initWithSource:[NSString stringWithCString:applescript_source.GetString()
  .str()
_

[Lldb-commits] [PATCH] D124572: Fix the encoding and decoding of UniqueCStringMap objects when saved to cache files.

2022-04-27 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere, jingham, aadsm, wallace, 
yinghuitan.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

UniqueCStringMap objects are a std::vector objects 
where the Entry object contains a ConstString + T. The values in the vector are 
sorted first by ConstString and then by the T value. ConstString objects are 
simply uniqued "const char *" values and when we compare we use the actual 
string pointer as the value we sort by. This caused a problem when we saved the 
symbol table name indexes and debug info indexes to disk in one process when 
they were sorted, and then loaded them into another process when decoding them 
from the cache files. Why? Because the order in which the ConstString objects 
were created are now completely different and the string pointers will no 
longer be sorted in the new process the cache was loaded into.

The unit tests created for the initial patch didn't catch the encoding and 
decoding issues of UniqueCStringMap because they were happening in the same 
process and encoding and decoding would end up createing sorted 
UniqueCStringMap objects due to the constant string pool being exactly the 
same.

This patch does the sort and also reserves the right amount of entries in the 
UniqueCStringMap::m_map prior to adding them all to avoid doing multiple 
allocations.

Added a unit test that loads an object file from yaml, and then I created a 
cache file for the original file and removed the cache file's signature mod 
time check since we will generate an object file from the YAML, and use that as 
the object file for the Symtab object. Then we load the cache data from the 
array of symtab cache bytes so that the ConstString "const char *" values will 
not match the current process, and verify we can lookup the 4 names from the 
object file in the symbol table.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124572

Files:
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  lldb/source/Symbol/Symtab.cpp
  lldb/unittests/Symbol/SymtabTest.cpp

Index: lldb/unittests/Symbol/SymtabTest.cpp
===
--- lldb/unittests/Symbol/SymtabTest.cpp
+++ lldb/unittests/Symbol/SymtabTest.cpp
@@ -303,3 +303,417 @@
   module_symtab->PreloadSymbols();
   EncodeDecode(*module_symtab);
 }
+
+TEST_F(SymtabTest, TestDecodeCStringMaps) {
+  // Symbol tables save out the symbols, but they also save out the symbol table
+  // name indexes. These name indexes are a map of sorted ConstString + T pairs
+  // and when they are decoded from a file, they are no longer sorted since
+  // ConstString objects can be sorted by "const char *" and the order in which
+  // these strings are created won't be the same in a new process. We need to
+  // ensure these name lookups happen correctly when we load the name indexes,
+  // so this test loads a symbol table from a cache file from
+  // "lldb/unittests/Symbol/Inputs/indexnames-symtab-cache" and make sure we
+  // can correctly lookup each of the names in the symbol table.
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x10C
+  cpusubtype:  0x0
+  filetype:0x2
+  ncmds:   16
+  sizeofcmds:  744
+  flags:   0x200085
+  reserved:0x0
+LoadCommands:
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  16384
+fileoff: 0
+filesize:16384
+maxprot: 5
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x13F64
+size:76
+offset:  0x3F64
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x8400
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: 80018052C0035FD6E0028052C0035FD640048052C0035FD6FF8300D1FD7B01A9FD43009108008052E80B00B9BFC31FB8F497F597F697E00B40B9FD7B41A9FF830091C0035FD6
+  - sectname:__unwind_info
+segname: __TEXT
+addr:0x13FB0
+size:80
+offset:  0x3FB0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+

[Lldb-commits] [PATCH] D124568: [lldb] Fix escaping when launching in terminal with AppleScript

2022-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5205c1774950: [lldb] Fix escaping when launching in terminal 
with AppleScript (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124568/new/

https://reviews.llvm.org/D124568

Files:
  lldb/source/Host/macosx/objcxx/Host.mm


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -212,18 +212,18 @@
   arch_spec.GetCore() != ArchSpec::eCore_x86_64_x86_64h)
 command.Printf("arch -arch %s ", arch_spec.GetArchitectureName());
 
-  command.Printf("'%s' --unix-socket=%s", launcher_path, unix_socket_name);
+  command.Printf(R"(\"%s\" --unix-socket=%s)", launcher_path, 
unix_socket_name);
 
   if (arch_spec.IsValid())
 command.Printf(" --arch=%s", arch_spec.GetArchitectureName());
 
   FileSpec working_dir{launch_info.GetWorkingDirectory()};
   if (working_dir)
-command.Printf(" --working-dir '%s'", working_dir.GetCString());
+command.Printf(R"( --working-dir \"%s\")", working_dir.GetCString());
   else {
 char cwd[PATH_MAX];
 if (getcwd(cwd, PATH_MAX))
-  command.Printf(" --working-dir '%s'", cwd);
+  command.Printf(R"( --working-dir \"%s\")", cwd);
   }
 
   if (launch_info.GetFlags().Test(eLaunchFlagDisableASLR))
@@ -239,7 +239,7 @@
   for (const auto &KV : launch_info.GetEnvironment()) {
 auto host_entry = host_env.find(KV.first());
 if (host_entry == host_env.end() || host_entry->second != KV.second)
-  command.Format(" --env='{0}'", Environment::compose(KV));
+  command.Format(R"( --env=\"{0}\")", Environment::compose(KV));
   }
 
   command.PutCString(" -- ");
@@ -248,12 +248,12 @@
   if (argv) {
 for (size_t i = 0; argv[i] != NULL; ++i) {
   if (i == 0)
-command.Printf(" '%s'", exe_path);
+command.Printf(R"( \"%s\")", exe_path);
   else
-command.Printf(" '%s'", argv[i]);
+command.Printf(R"( \"%s\")", argv[i]);
 }
   } else {
-command.Printf(" '%s'", exe_path);
+command.Printf(R"( \"%s\")", exe_path);
   }
   command.PutCString(" ; echo Process exited with status $?");
   if (launch_info.GetFlags().Test(lldb::eLaunchFlagCloseTTYOnExit))
@@ -263,6 +263,7 @@
 
   applescript_source.Printf(applscript_in_new_tty,
 command.GetString().str().c_str());
+
   NSAppleScript *applescript = [[NSAppleScript alloc]
   initWithSource:[NSString stringWithCString:applescript_source.GetString()
  .str()


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -212,18 +212,18 @@
   arch_spec.GetCore() != ArchSpec::eCore_x86_64_x86_64h)
 command.Printf("arch -arch %s ", arch_spec.GetArchitectureName());
 
-  command.Printf("'%s' --unix-socket=%s", launcher_path, unix_socket_name);
+  command.Printf(R"(\"%s\" --unix-socket=%s)", launcher_path, unix_socket_name);
 
   if (arch_spec.IsValid())
 command.Printf(" --arch=%s", arch_spec.GetArchitectureName());
 
   FileSpec working_dir{launch_info.GetWorkingDirectory()};
   if (working_dir)
-command.Printf(" --working-dir '%s'", working_dir.GetCString());
+command.Printf(R"( --working-dir \"%s\")", working_dir.GetCString());
   else {
 char cwd[PATH_MAX];
 if (getcwd(cwd, PATH_MAX))
-  command.Printf(" --working-dir '%s'", cwd);
+  command.Printf(R"( --working-dir \"%s\")", cwd);
   }
 
   if (launch_info.GetFlags().Test(eLaunchFlagDisableASLR))
@@ -239,7 +239,7 @@
   for (const auto &KV : launch_info.GetEnvironment()) {
 auto host_entry = host_env.find(KV.first());
 if (host_entry == host_env.end() || host_entry->second != KV.second)
-  command.Format(" --env='{0}'", Environment::compose(KV));
+  command.Format(R"( --env=\"{0}\")", Environment::compose(KV));
   }
 
   command.PutCString(" -- ");
@@ -248,12 +248,12 @@
   if (argv) {
 for (size_t i = 0; argv[i] != NULL; ++i) {
   if (i == 0)
-command.Printf(" '%s'", exe_path);
+command.Printf(R"( \"%s\")", exe_path);
   else
-command.Printf(" '%s'", argv[i]);
+command.Printf(R"( \"%s\")", argv[i]);
 }
   } else {
-command.Printf(" '%s'", exe_path);
+command.Printf(R"( \"%s\")", exe_path);
   }
   command.PutCString(" ; echo Process exited with status $?");
   if (launch_info.GetFlags().Test(lldb::eLaunchFlagCloseTTYOnExit))
@@ -263,6 +263,7 @@
 
   applescript_source.Printf(applscript_in_new_tty,
 command.GetString().str().c_str());
+
   NSAppleScript *applescript = [[NSAppleScript alloc]
   initWithSource:[NSString stringWithC

[Lldb-commits] [lldb] 5205c17 - [lldb] Fix escaping when launching in terminal with AppleScript

2022-04-27 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-04-27T16:37:42-07:00
New Revision: 5205c1774950b62dd410c69474295cd0f9351c7d

URL: 
https://github.com/llvm/llvm-project/commit/5205c1774950b62dd410c69474295cd0f9351c7d
DIFF: 
https://github.com/llvm/llvm-project/commit/5205c1774950b62dd410c69474295cd0f9351c7d.diff

LOG: [lldb] Fix escaping when launching in terminal with AppleScript

Fix escaping when launching in terminal with AppleScript. The invocation
we're building up is wrapped in single quotes when passed to bash and
wrapped in double quotes for AppleScript.

Here's an example invocation with the new escaping:

  tell application "Terminal"
activate
  do script "/bin/bash -c 'arch -arch arm64 'darwin-debug'
--unix-socket=/tmp/dL2jSh --arch=arm64 --working-dir
\"/private/tmp/with spaces\" --disable-aslr --  \"foo\"
\"bar\" \"baz\" ; echo Process exited with status $?';exit"
  end tell

Previously we were using unescaped single quotes which resulted in the
whole bash invocation being passed in pieces. That works most of the
time but breaks when you have a space in your current working directory
for example.

rdar://91870763

Differential revision: https://reviews.llvm.org/D124568

Added: 


Modified: 
lldb/source/Host/macosx/objcxx/Host.mm

Removed: 




diff  --git a/lldb/source/Host/macosx/objcxx/Host.mm 
b/lldb/source/Host/macosx/objcxx/Host.mm
index 0a51e534896a1..ca6a408642ae8 100644
--- a/lldb/source/Host/macosx/objcxx/Host.mm
+++ b/lldb/source/Host/macosx/objcxx/Host.mm
@@ -212,18 +212,18 @@ repeat with the_window in (get windows)\n\
   arch_spec.GetCore() != ArchSpec::eCore_x86_64_x86_64h)
 command.Printf("arch -arch %s ", arch_spec.GetArchitectureName());
 
-  command.Printf("'%s' --unix-socket=%s", launcher_path, unix_socket_name);
+  command.Printf(R"(\"%s\" --unix-socket=%s)", launcher_path, 
unix_socket_name);
 
   if (arch_spec.IsValid())
 command.Printf(" --arch=%s", arch_spec.GetArchitectureName());
 
   FileSpec working_dir{launch_info.GetWorkingDirectory()};
   if (working_dir)
-command.Printf(" --working-dir '%s'", working_dir.GetCString());
+command.Printf(R"( --working-dir \"%s\")", working_dir.GetCString());
   else {
 char cwd[PATH_MAX];
 if (getcwd(cwd, PATH_MAX))
-  command.Printf(" --working-dir '%s'", cwd);
+  command.Printf(R"( --working-dir \"%s\")", cwd);
   }
 
   if (launch_info.GetFlags().Test(eLaunchFlagDisableASLR))
@@ -239,7 +239,7 @@ repeat with the_window in (get windows)\n\
   for (const auto &KV : launch_info.GetEnvironment()) {
 auto host_entry = host_env.find(KV.first());
 if (host_entry == host_env.end() || host_entry->second != KV.second)
-  command.Format(" --env='{0}'", Environment::compose(KV));
+  command.Format(R"( --env=\"{0}\")", Environment::compose(KV));
   }
 
   command.PutCString(" -- ");
@@ -248,12 +248,12 @@ repeat with the_window in (get windows)\n\
   if (argv) {
 for (size_t i = 0; argv[i] != NULL; ++i) {
   if (i == 0)
-command.Printf(" '%s'", exe_path);
+command.Printf(R"( \"%s\")", exe_path);
   else
-command.Printf(" '%s'", argv[i]);
+command.Printf(R"( \"%s\")", argv[i]);
 }
   } else {
-command.Printf(" '%s'", exe_path);
+command.Printf(R"( \"%s\")", exe_path);
   }
   command.PutCString(" ; echo Process exited with status $?");
   if (launch_info.GetFlags().Test(lldb::eLaunchFlagCloseTTYOnExit))
@@ -263,6 +263,7 @@ repeat with the_window in (get windows)\n\
 
   applescript_source.Printf(applscript_in_new_tty,
 command.GetString().str().c_str());
+
   NSAppleScript *applescript = [[NSAppleScript alloc]
   initWithSource:[NSString stringWithCString:applescript_source.GetString()
  .str()



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

[Lldb-commits] [PATCH] D115324: Added the ability to cache the finalized symbol tables subsequent debug sessions to start faster.

2022-04-27 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.
Herald added a project: All.

The unit testing I added to this patch was unable to catch a serious issue that 
I have fixed and is needed if anyone wants to enable this feature. So please 
make sure if you use this patch to also make sure that the following patch is 
also applied to your repository if you are not using top of tree:

https://reviews.llvm.org/D124572


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115324/new/

https://reviews.llvm.org/D115324

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

[Lldb-commits] [PATCH] D123954: [lldb] Add setting for max depth of value object printing (NFC)

2022-04-27 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:718
+  /// been told.
+  ChildrenOmissionWarningStatus m_truncation_warning;
+  /// Whether we reached the maximum child nesting depth and whether the user

shafik wrote:
> Why not use in class member initialization? It looks like they have default 
> values. I am not sure why the rest of the values where not caught the other 
> day when I ran clang-tidy.
I was following the convention within the file. What's the ideal change, 
initialize all in this change? Or initialize the two that I have edited?



Comment at: lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp:180
 use_dynamic = target_sp->GetPreferDynamicValue();
-  else {
+std::tie(max_depth, max_depth_is_default) =
+target_sp->GetMaximumDepthOfChildrenToDisplay();

shafik wrote:
> My kingdom for structured bindings.
soon, hopefully.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123954/new/

https://reviews.llvm.org/D123954

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

[Lldb-commits] [PATCH] D124573: [trace][intelpt] Support system-wide tracing [1] - Add a method for accessing the list of logical core ids

2022-04-27 Thread walter erquinigo via Phabricator via lldb-commits 
wallace created this revision.
wallace added a reviewer: jj10306.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

In order to open perf events per core, we need to first get the list of
core ids available in the system. So I'm adding a function that does
that by parsing /proc/cpuinfo. That seems to be the simplest and most
portable way to do that.

Besides that, I made a few refactors and renames to reflect better that
the cpu info that we use in lldb-server comes from procfs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124573

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp

Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -13,6 +13,9 @@
 
 namespace lldb_private {
 
+const char *IntelPTDataKinds::kProcFsCpuInfo = "procFsCpuInfo";
+const char *IntelPTDataKinds::kThreadTraceBuffer = "threadTraceBuffer";
+
 bool fromJSON(const json::Value &value, TraceIntelPTStartRequest &packet,
   Path path) {
   ObjectMapper o(value, path);
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
@@ -48,8 +48,8 @@
 return json_intel_pt_trace.takeError();
 
   llvm::Expected json_session_description =
-  TraceSessionSaver::BuildProcessesSection(*live_process,
-   "threadTraceBuffer", directory);
+  TraceSessionSaver::BuildProcessesSection(
+  *live_process, IntelPTDataKinds::kThreadTraceBuffer, directory);
 
   if (!json_session_description)
 return json_session_description.takeError();
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -81,7 +81,8 @@
   for (const ThreadPostMortemTraceSP &thread : traced_threads) {
 m_thread_decoders.emplace(thread->GetID(),
   std::make_unique(thread, *this));
-SetPostMortemThreadDataFile(thread->GetID(), "threadTraceBuffer",
+SetPostMortemThreadDataFile(thread->GetID(),
+IntelPTDataKinds::kThreadTraceBuffer,
 thread->GetTraceFile());
   }
 }
@@ -179,7 +180,8 @@
 }
 
 Expected TraceIntelPT::GetCPUInfoForLiveProcess() {
-  Expected> cpu_info = GetLiveProcessBinaryData("cpuInfo");
+  Expected> cpu_info =
+  GetLiveProcessBinaryData(IntelPTDataKinds::kProcFsCpuInfo);
   if (!cpu_info)
 return cpu_info.takeError();
 
@@ -393,7 +395,8 @@
 
 Error TraceIntelPT::OnThreadBufferRead(lldb::tid_t tid,
OnBinaryDataReadCallback callback) {
-  return OnThreadBinaryDataRead(tid, "threadTraceBuffer", callback);
+  return OnThreadBinaryDataRead(tid, IntelPTDataKinds::kThreadTraceBuffer,
+callback);
 }
 
 TaskTimer &TraceIntelPT::GetTimer() { return m_task_timer; }
Index: lldb/source/Plugins/Process/Linux/Perf.h
===
--- lldb/source/Plugins/Process/Linux/Perf.h
+++ lldb/source/Plugins/Process/Linux/Perf.h
@@ -76,7 +76,7 @@
 
 /// \return
 /// The content of /proc/cpuinfo and cache it if errors didn't happen.
-llvm::Expected> GetCPUInfo();
+llvm::Expected> GetProcFsCpuInfo();
 
 /// \return
 /// A list of available logical core ids given the contents of
Index: lldb/source/Plugins/Process/Linux/Perf.cpp
===
--- lldb/source/Plugins/Process/Linux/Perf.cpp
+++ lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -22,7 +22,7 @@
 using namespace process_linux;
 using namespace llvm;
 
-Expected> lldb_private::process_linux::GetCPUInfo() {
+Expected> lldb_private::process_linux::GetProcFsCpuInfo() {
   static Optional> cpu_info;
   if (!cpu_info) {
 auto buffer_or_error = errorOrToExpected(getProcFile("cpuinfo"));
@@ -63,7 +63,7 @@
   static Optional> logical_cores_ids;
   if (!logical_cores_ids) {
 // We find the actual list of core ids by parsing /proc/cpuinfo
-Expected> cpuinfo = GetCPUInfo();
+Expected> cpuinfo = GetProcFsCpuInfo();

[Lldb-commits] [PATCH] D124000: [lldb] Add FixAnyAddress to ABI plugins

2022-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.

Yes, this makes a lot of sense to me. Thanks for taking care of this. LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124000/new/

https://reviews.llvm.org/D124000

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

[Lldb-commits] [PATCH] D124499: Rename conflict testcase

2022-04-27 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Don't forget to rename the class name as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124499/new/

https://reviews.llvm.org/D124499

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

[Lldb-commits] [PATCH] D124579: Make partial function name matching match on context boundaries

2022-04-27 Thread Jim Ingham via Phabricator via lldb-commits 
jingham created this revision.
jingham added reviewers: JDevlieghere, labath, clayborg, shafik.
Herald added a subscriber: mgorny.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

We don't require users to type out the full context of a function, for symbol 
name matches.  Instead, we extract the incoming path's base name, look up all 
the symbols with that base name, and then compare the rest of the context that 
the user provided to make sure it matches.  However, we do this comparison 
using just a strstr.  So for instance:

break set -n foo::bar

will match not only "a::foo::bar" but "notherfoo::bar".  The former is pretty 
clearly the user's intent, but I don't think the latter is, and results in 
breakpoints picking up too many matches.

This change adds a Language::DemangledNameContainsPath API which can do a 
language aware match against the path provided.  If the language doesn't 
provide this we fall back to the strstr (though that's changed to 
StringRef::contains in the patch).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124579

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Target/Language.cpp
  lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
  lldb/test/API/functionalities/breakpoint/cpp/main.cpp
  lldb/test/API/functionalities/return-value/TestReturnValue.py
  lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py
  lldb/tools/CMakeLists.txt
  lldb/tools/lldb-instr/CMakeLists.txt
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -123,6 +123,45 @@
   }
 }
 
+TEST(CPlusPlusLanguage, ContainsPath) {
+  CPlusPlusLanguage::MethodName 
+  reference_1(ConstString("foo::bar::func01(int a, double b)"));
+  CPlusPlusLanguage::MethodName
+  reference_2(ConstString("foofoo::bar::func01(std::string a, int b)"));
+  CPlusPlusLanguage::MethodName reference_3(ConstString("func01()"));
+  
+  struct TestCase {
+std::string path;
+bool result;
+  };
+  TestCase test_cases_1[] = {
+{"func01", true},
+{"bar::func01", true},
+{"foo::bar::func01", true},
+{"func", false},
+{"baz::func01", false},
+{"::bar::func01", false},
+{"::foo::baz::func01", false},
+{"foo::bar::baz::func01", false}
+  };
+  TestCase test_cases_2[] {
+{"foofoo::bar::func01", true},
+{"foo::bar::func01", false}
+  };
+  TestCase test_cases_3[] {
+{"func01", true},
+{"func", false},
+{"bar::func01", false},
+  };
+
+  for (auto test : test_cases_1) {
+EXPECT_EQ(reference_1.ContainsPath(test.path), test.result);
+  }
+  for (auto test : test_cases_2) {
+EXPECT_EQ(reference_2.ContainsPath(test.path), test.result);
+  }
+}
+
 TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) {
   struct TestCase {
 std::string input;
Index: lldb/tools/lldb-instr/CMakeLists.txt
===
--- lldb/tools/lldb-instr/CMakeLists.txt
+++ lldb/tools/lldb-instr/CMakeLists.txt
@@ -9,7 +9,7 @@
 clangLex
 clangRewrite
 clangSerialization
-clangTooling
+clangToolingCore
 
   LINK_COMPONENTS
 Support
Index: lldb/tools/CMakeLists.txt
===
--- lldb/tools/CMakeLists.txt
+++ lldb/tools/CMakeLists.txt
@@ -8,7 +8,7 @@
 add_subdirectory(lldb-test EXCLUDE_FROM_ALL)
 add_subdirectory(lldb-fuzzer EXCLUDE_FROM_ALL)
 
-add_lldb_tool_subdirectory(lldb-instr)
+#add_lldb_tool_subdirectory(lldb-instr)
 add_lldb_tool_subdirectory(lldb-vscode)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
Index: lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py
===
--- lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py
+++ lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py
@@ -98,7 +98,7 @@
 # make sure we are again in out target function.
 break_reexported = target.BreakpointCreateByName(
 "reexport_to_indirect")
-self.assertTrue(break_reexported, VALID_BREAKPOINT)
+self.assertEqual(break_reexported.GetNumLocations(), 1, VALID_BREAKPOINT)
 
 # Now continue should take us to the second call through the indirect
 # symbol:
Index: lldb/test/API/functionalities/return-value/TestReturnValue.py
===
--- lldb/test/API/functionalities/return-value/TestReturnValue.py
+++ lldb/test/API/functionalities/ret

[Lldb-commits] [PATCH] D124579: Make partial function name matching match on context boundaries

2022-04-27 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added a comment.

You might want to try fun cases like `operator<` and `operator()()` from a 
lambda. They should work but might be worth throwing them in.




Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:330
+
+//  size_t from = 0;
+//  llvm::StringRef demangled_ref = demangled.GetStringRef();

dead code?



Comment at: lldb/tools/CMakeLists.txt:11
 
-add_lldb_tool_subdirectory(lldb-instr)
+#add_lldb_tool_subdirectory(lldb-instr)
 add_lldb_tool_subdirectory(lldb-vscode)

Did you mean to delete this line?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124579/new/

https://reviews.llvm.org/D124579

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

[Lldb-commits] [PATCH] D124572: Fix the encoding and decoding of UniqueCStringMap objects when saved to cache files.

2022-04-27 Thread jeffrey tan via Phabricator via lldb-commits 
yinghuitan accepted this revision.
yinghuitan added a comment.
This revision is now accepted and ready to land.

This is a great finding! The change looks good. Some questions though:

- Do you have any theory why we only see this issue on Mac not Linux? (For 
anyone else reading this, I found this bug during testing Mac but the same 
reproduce steps work on Linux)
- Do we use/have data structures in other part of caching feature need similar 
change?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124572/new/

https://reviews.llvm.org/D124572

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