[Lldb-commits] [lldb] c526426 - [lldb] Don't reject empty or unnamed template parameter packs

2020-12-02 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-12-02T10:50:41+01:00
New Revision: c526426f5cba5308782ea4f86822047ee2ee3818

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

LOG: [lldb] Don't reject empty or unnamed template parameter packs

We currently reject all templates that have either zero args or that have a
parameter pack without a name. Both cases are actually allowed in C++, so
rejecting them leads to LLDB instead falling back to a dummy 'void' type. This
leads to all kind of errors later on (most notable, variables that have such
template types appear to be missing as we can't have 'void' variables and
inheriting from such a template type will cause Clang to hit some asserts when
finding that the base class is 'void').

This just removes the too strict tests and adds a few tests for this stuff (+
some combinations of these tests with preceding template parameters).

Things that I left for follow-up patches:
* All the possible interactions with template-template arguments which seem 
like a whole new source of possible bugs.
* Function templates which completely lack sanity checks.
* Variable templates are not implemented.
* Alias templates are not implemented too.
* The rather strange checks that just make sure that the separate list of
  template arg names and values always have the same length. I believe those
  ought to be asserts, but my current plan is to move both those things into a
  single list that can't end up in this inconsistent state.

Reviewed By: JDevlieghere, shafik

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

Added: 
lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/Makefile

lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/main.cpp
lldb/test/API/lang/cpp/class-template-type-parameter-pack/Makefile

lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
lldb/test/API/lang/cpp/class-template-type-parameter-pack/main.cpp

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/unittests/Symbol/TestTypeSystemClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 6633acd70a50..2c045d6dc9c0 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1944,8 +1944,6 @@ bool DWARFASTParserClang::ParseTemplateParameterInfos(
   break;
 }
   }
-  if (template_param_infos.args.empty())
-return false;
   return template_param_infos.args.size() == template_param_infos.names.size();
 }
 

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 5eb492f0dbc2..6879d2566183 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -332,10 +332,11 @@ class TypeSystemClang : public TypeSystem {
   class TemplateParameterInfos {
   public:
 bool IsValid() const {
-  if (args.empty())
+  // Having a pack name but no packed args doesn't make sense, so mark
+  // these template parameters as invalid.
+  if (pack_name && !packed_args)
 return false;
   return args.size() == names.size() &&
-((bool)pack_name == (bool)packed_args) &&
 (!packed_args || !packed_args->packed_args);
 }
 

diff  --git 
a/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/Makefile 
b/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/Makefile
new file mode 100644
index ..8b20bcb0
--- /dev/null
+++ b/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
 
b/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
new file mode 100644
index ..d366d9d69222
--- /dev/null
+++ 
b/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
@@ -0,0 +1,76 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test(self):
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+

[Lldb-commits] [PATCH] D92425: [lldb] Don't reject empty or unnamed template parameter packs

2020-12-02 Thread Raphael Isemann via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc526426f5cba: [lldb] Don't reject empty or unnamed 
template parameter packs (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92425

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/Makefile
  
lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
  lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/main.cpp
  lldb/test/API/lang/cpp/class-template-type-parameter-pack/Makefile
  
lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
  lldb/test/API/lang/cpp/class-template-type-parameter-pack/main.cpp
  lldb/unittests/Symbol/TestTypeSystemClang.cpp

Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -516,6 +516,12 @@
   }
 }
 
+TEST_F(TestTypeSystemClang, OnlyPackName) {
+  TypeSystemClang::TemplateParameterInfos infos;
+  infos.pack_name = "A";
+  EXPECT_FALSE(infos.IsValid());
+}
+
 static QualType makeConstInt(clang::ASTContext &ctxt) {
   QualType result(ctxt.IntTy);
   result.addConst();
Index: lldb/test/API/lang/cpp/class-template-type-parameter-pack/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/class-template-type-parameter-pack/main.cpp
@@ -0,0 +1,69 @@
+// Named type parameter pack.
+template 
+struct TypePack { int a; };
+TypePack<> emptyTypePack;
+TypePack oneElemTypePack;
+TypePack twoElemTypePack;
+
+// Unnamed type parameter pack.
+template 
+struct AnonTypePack { int b; };
+AnonTypePack<> emptyAnonTypePack;
+AnonTypePack oneElemAnonTypePack;
+AnonTypePack twoElemAnonTypePack;
+
+// Test type parameter packs combined with non-pack type template parameters.
+
+// Unnamed type parameter pack behind a named type parameter.
+template 
+struct AnonTypePackAfterTypeParam { T c; };
+AnonTypePackAfterTypeParam emptyAnonTypePackAfterTypeParam;
+AnonTypePackAfterTypeParam oneElemAnonTypePackAfterTypeParam;
+
+// Unnamed type parameter pack behind an unnamed type parameter.
+template 
+struct AnonTypePackAfterAnonTypeParam { float d; };
+AnonTypePackAfterAnonTypeParam emptyAnonTypePackAfterAnonTypeParam;
+AnonTypePackAfterAnonTypeParam oneElemAnonTypePackAfterAnonTypeParam;
+
+// Named type parameter pack behind an unnamed type parameter.
+template 
+struct TypePackAfterAnonTypeParam { int e; };
+TypePackAfterAnonTypeParam emptyTypePackAfterAnonTypeParam;
+TypePackAfterAnonTypeParam oneElemTypePackAfterAnonTypeParam;
+
+// Named type parameter pack behind a named type parameter.
+template 
+struct TypePackAfterTypeParam { int f; };
+TypePackAfterTypeParam emptyTypePackAfterTypeParam;
+TypePackAfterTypeParam oneElemTypePackAfterTypeParam;
+
+// Test type parameter packs combined with non-pack non-type template parameters.
+
+// Unnamed type parameter pack behind a named type parameter.
+template 
+struct AnonTypePackAfterNonTypeParam { int g; };
+AnonTypePackAfterNonTypeParam<1> emptyAnonTypePackAfterNonTypeParam;
+AnonTypePackAfterNonTypeParam<1, int> oneElemAnonTypePackAfterNonTypeParam;
+
+// Unnamed type parameter pack behind an unnamed type parameter.
+template 
+struct AnonTypePackAfterAnonNonTypeParam { float h; };
+AnonTypePackAfterAnonNonTypeParam<1> emptyAnonTypePackAfterAnonNonTypeParam;
+AnonTypePackAfterAnonNonTypeParam<1, int> oneElemAnonTypePackAfterAnonNonTypeParam;
+
+// Named type parameter pack behind an unnamed type parameter.
+template 
+struct TypePackAfterAnonNonTypeParam { int i; };
+TypePackAfterAnonNonTypeParam<1> emptyTypePackAfterAnonNonTypeParam;
+TypePackAfterAnonNonTypeParam<1, int> oneElemTypePackAfterAnonNonTypeParam;
+
+// Named type parameter pack behind an unnamed type parameter.
+template 
+struct TypePackAfterNonTypeParam { int j; };
+TypePackAfterNonTypeParam<1> emptyTypePackAfterNonTypeParam;
+TypePackAfterNonTypeParam<1, int> oneElemTypePackAfterNonTypeParam;
+
+int main() {
+  return 0; // break here
+}
Index: lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
@@ -0,0 +1,76 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug

[Lldb-commits] [PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-12-02 Thread Gabor Marton via Phabricator via lldb-commits 
martong added a subscriber: balazske.
martong added a comment.

Hey Raphael, thanks for looking into the CTU crash!

I also had a look and I could reproduce that on Linux Ubuntu 18.04 with GCC 7. 
I think the discrepancy stems from GCC's libstdc++ and MacOS's libc++ 
implementation differences of `basic_streambuf`. This is the CXXRecordDecl 
which does not need an injected class name type (so there is the assert). 
However, the situation is more complex because there is a 5 long redecl chain 
(streambuf is forward declared in many places).

Anyway, I don't think that this patch itself caused the crash, it just revealed 
some other issues that are related to the possibly flawed import logic of 
injected class (name) types. I am adding another college @balazske as a 
subscriber because he started to investigate the crash deeper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[Lldb-commits] [PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-12-02 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor added a comment.

In D92103#2428139 , @martong wrote:

> Hey Raphael, thanks for looking into the CTU crash!
>
> I also had a look and I could reproduce that on Linux Ubuntu 18.04 with GCC 
> 7. I think the discrepancy stems from GCC's libstdc++ and MacOS's libc++ 
> implementation differences of `basic_streambuf`. This is the CXXRecordDecl 
> which does not need an injected class name type (so there is the assert). 
> However, the situation is more complex because there is a 5 long redecl chain 
> (streambuf is forward declared in many places).
>
> Anyway, I don't think that this patch itself caused the crash, it just 
> revealed some other issues that are related to the possibly flawed import 
> logic of injected class (name) types. I am adding another college @balazske 
> as a subscriber because he started to investigate the crash deeper.

Quick note: I should have mention that I actually tried reproducing this issue 
with libstdc++/GCC 10.2.0 on Arch Linux.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[Lldb-commits] [lldb] d055e3a - [LLDB/Python] Fix segfault on Python scripted entrypoints

2020-12-02 Thread Pedro Tammela via lldb-commits 

Author: Pedro Tammela
Date: 2020-12-02T11:25:31Z
New Revision: d055e3a0eb4e957159b075c0937a960beb75c975

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

LOG: [LLDB/Python] Fix segfault on Python scripted entrypoints

The code that gets the ScriptInterpreter was not considering the
case that it receives a Lua interpreter.

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

Added: 
lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test

Modified: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index e5802ad041e4..0d13884e8089 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "lldb/Host/Config.h"
+#include "lldb/lldb-enumerations.h"
 
 #if LLDB_ENABLE_PYTHON
 
@@ -214,6 +215,12 @@ extern "C" void *
 LLDBSWIGPython_GetDynamicSetting(void *module, const char *setting,
  const lldb::TargetSP &target_sp);
 
+static ScriptInterpreterPythonImpl *GetPythonInterpreter(Debugger &debugger) {
+  ScriptInterpreter *script_interpreter =
+  debugger.GetScriptInterpreter(true, lldb::eScriptLanguagePython);
+  return static_cast(script_interpreter);
+}
+
 static bool g_initialized = false;
 
 namespace {
@@ -1825,11 +1832,10 @@ StructuredData::ObjectSP 
ScriptInterpreterPythonImpl::CreateScriptedThreadPlan(
 return {};
 
   Debugger &debugger = thread_plan_sp->GetTarget().GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return {};
 
   void *ret_val;
@@ -1929,11 +1935,10 @@ 
ScriptInterpreterPythonImpl::CreateScriptedBreakpointResolver(
 return StructuredData::GenericSP();
 
   Debugger &debugger = bkpt_sp->GetTarget().GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return StructuredData::GenericSP();
 
   void *ret_val;
@@ -2003,11 +2008,10 @@ StructuredData::GenericSP 
ScriptInterpreterPythonImpl::CreateScriptedStopHook(
 return StructuredData::GenericSP();
   }
 
-  ScriptInterpreter *script_interpreter = m_debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(m_debugger);
 
-  if (!script_interpreter) {
+  if (!python_interpreter) {
 error.SetErrorString("No script interpreter for scripted stop-hook.");
 return StructuredData::GenericSP();
   }
@@ -2103,11 +2107,10 @@ 
ScriptInterpreterPythonImpl::CreateSyntheticScriptedProvider(
 return StructuredData::ObjectSP();
 
   Debugger &debugger = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return StructuredData::ObjectSP();
 
   void *ret_val = nullptr;
@@ -2274,11 +2277,10 @@ bool 
ScriptInterpreterPythonImpl::BreakpointCallbackFunction(
 return true;
 
   Debugger &debugger = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return true;
 
   if (python_function_name && python_function_name[0]) {
@@ -2340,11 +2342,10 @@ bool 
ScriptInterpreterPythonImpl::WatchpointCallbackFunction(
 return true;
 
   Debugger &debugger = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return true;
 
   if (python_function_name && python_function_name[0]) {

diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Pytho

[Lldb-commits] [PATCH] D92249: [LLDB/Python] Fix segfault on Python scripted breakpoints

2020-12-02 Thread Pedro Tammela via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd055e3a0eb4e: [LLDB/Python] Fix segfault on Python scripted 
entrypoints (authored by tammela).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92249

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test

Index: lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test
@@ -0,0 +1,8 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o 'print(frame); return False'
+run
+# CHECK: frame #0
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "lldb/Host/Config.h"
+#include "lldb/lldb-enumerations.h"
 
 #if LLDB_ENABLE_PYTHON
 
@@ -214,6 +215,12 @@
 LLDBSWIGPython_GetDynamicSetting(void *module, const char *setting,
  const lldb::TargetSP &target_sp);
 
+static ScriptInterpreterPythonImpl *GetPythonInterpreter(Debugger &debugger) {
+  ScriptInterpreter *script_interpreter =
+  debugger.GetScriptInterpreter(true, lldb::eScriptLanguagePython);
+  return static_cast(script_interpreter);
+}
+
 static bool g_initialized = false;
 
 namespace {
@@ -1825,11 +1832,10 @@
 return {};
 
   Debugger &debugger = thread_plan_sp->GetTarget().GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return {};
 
   void *ret_val;
@@ -1929,11 +1935,10 @@
 return StructuredData::GenericSP();
 
   Debugger &debugger = bkpt_sp->GetTarget().GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return StructuredData::GenericSP();
 
   void *ret_val;
@@ -2003,11 +2008,10 @@
 return StructuredData::GenericSP();
   }
 
-  ScriptInterpreter *script_interpreter = m_debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(m_debugger);
 
-  if (!script_interpreter) {
+  if (!python_interpreter) {
 error.SetErrorString("No script interpreter for scripted stop-hook.");
 return StructuredData::GenericSP();
   }
@@ -2103,11 +2107,10 @@
 return StructuredData::ObjectSP();
 
   Debugger &debugger = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return StructuredData::ObjectSP();
 
   void *ret_val = nullptr;
@@ -2274,11 +2277,10 @@
 return true;
 
   Debugger &debugger = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return true;
 
   if (python_function_name && python_function_name[0]) {
@@ -2340,11 +2342,10 @@
 return true;
 
   Debugger &debugger = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return true;
 
   if (python_function_name && python_function_name[0]) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82857: [LLDB] Add per-thread register infos shared pointer in gdb-remote

2020-12-02 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid updated this revision to Diff 308932.
omjavaid added a comment.

Added copy constructor.


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

https://reviews.llvm.org/D82857

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -14,6 +14,8 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/StructuredData.h"
 
+#include "GDBRemoteRegisterContext.h"
+
 class StringExtractor;
 
 namespace lldb_private {
@@ -101,6 +103,8 @@
   m_queue_serial_number; // Queue info from stop reply/stop info for thread
   lldb_private::LazyBool m_associated_with_libdispatch_queue;
 
+  GDBRemoteDynamicRegisterInfoSP m_reg_info_sp;
+
   bool PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef data);
 
   bool PrivateSetRegisterValue(uint32_t reg, uint64_t regval);
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -42,6 +42,22 @@
   Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_THREAD));
   LLDB_LOG(log, "this = {0}, pid = {1}, tid = {2}", this, process.GetID(),
GetID());
+  // At this point we can clone reg_info for architectures supporting
+  // run-time update to register sizes and offsets..
+  ProcessSP process_sp(GetProcess());
+  if (process_sp) {
+ProcessGDBRemote *gdb_process =
+static_cast(process_sp.get());
+
+if (!m_reg_info_sp) {
+  if (!gdb_process->m_register_info_sp->IsReconfigurable())
+m_reg_info_sp = gdb_process->m_register_info_sp;
+  else {
+m_reg_info_sp = std::make_shared(
+*gdb_process->m_register_info_sp);
+  }
+}
+  }
 }
 
 ThreadGDBRemote::~ThreadGDBRemote() {
@@ -307,8 +323,8 @@
   !pSupported || gdb_process->m_use_g_packet_for_reading;
   bool write_all_registers_at_once = !pSupported;
   reg_ctx_sp = std::make_shared(
-  *this, concrete_frame_idx, gdb_process->m_register_info,
-  read_all_registers_at_once, write_all_registers_at_once);
+  *this, concrete_frame_idx, m_reg_info_sp, read_all_registers_at_once,
+  write_all_registers_at_once);
 }
   } else {
 reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame);
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -254,7 +254,7 @@
  // the last stop
  // packet variable
   std::recursive_mutex m_last_stop_packet_mutex;
-  GDBRemoteDynamicRegisterInfo m_register_info;
+  GDBRemoteDynamicRegisterInfoSP m_register_info_sp;
   Broadcaster m_async_broadcaster;
   lldb::ListenerSP m_async_listener_sp;
   HostThread m_async_thread;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -249,7 +249,7 @@
ListenerSP listener_sp)
 : Process(target_sp, listener_sp),
   m_debugserver_pid(LLDB_INVALID_PROCESS_ID), m_last_stop_packet_mutex(),
-  m_register_info(),
+  m_register_info_sp(nullptr),
   m_async_broadcaster(nullptr, "lldb.process.gdb-remote.async-broadcaster"),
   m_async_listener_sp(
   Listener::MakeListener("lldb.process.gdb-remote.async-listener")),
@@ -368,8 +368,8 @@
   m_breakpoint_pc_offset = breakpoint_pc_int_value->GetValue();
   }
 
-  if (m_register_info.SetRegisterInfo(*target_definition_sp,
-  GetTarget().GetArchitecture()) > 0) {
+  if (m_register_info_sp->SetRegisterInfo(
+  *target_definition_sp, GetTarget().GetArchitecture()) > 0) {
 return true;
   }
 }
@@ -396,10 +396,11 @@
 }
 
 void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) {
-  if (!force && m_register_info.GetNumR

[Lldb-commits] [PATCH] D92425: [lldb] Don't reject empty or unnamed template parameter packs

2020-12-02 Thread Stella Stamenova via Phabricator via lldb-commits 
stella.stamenova added a comment.

This caused failures on the Windows lldb buildbot:

http://lab.llvm.org:8011/#/builders/83/builds/1294

Tests have to have unique names, so the two new added tests that use TestCase 
as the class name are both trying to write to the same log file and failing to 
do so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92425

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

[Lldb-commits] [lldb] 291cc1b - [lldb][NFC] Give class template pack test files unique class names

2020-12-02 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-12-02T19:19:35+01:00
New Revision: 291cc1bbea1f4a6cab829509e95b3efe40af908f

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

LOG: [lldb][NFC] Give class template pack test files unique class names

Added: 


Modified: 

lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py

lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py

Removed: 




diff  --git 
a/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
 
b/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
index d366d9d69222..6b4b96049cdb 100644
--- 
a/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
+++ 
b/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
@@ -3,7 +3,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestCase(TestBase):
+class TestCaseClassTemplateNonTypeParameterPack(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
@@ -73,4 +73,4 @@ def test(self):
 self.expect_expr("emptyNonTypePackAfterNonTypeParam", 
result_type="NonTypePackAfterNonTypeParam<1>",
 result_children=[ValueCheck(name="j", type="int")])
 self.expect_expr("oneElemNonTypePackAfterNonTypeParam", 
result_type="NonTypePackAfterNonTypeParam<1, 2>",
-result_children=[ValueCheck(name="j", type="int")])
\ No newline at end of file
+result_children=[ValueCheck(name="j", type="int")])

diff  --git 
a/lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
 
b/lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
index 48ea59d6e5eb..223ba5224274 100644
--- 
a/lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
+++ 
b/lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
@@ -3,7 +3,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestCase(TestBase):
+class TestCaseClassTemplateTypeParameterPack(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
@@ -73,4 +73,4 @@ def test(self):
 self.expect_expr("emptyTypePackAfterNonTypeParam", 
result_type="TypePackAfterNonTypeParam<1>",
 result_children=[ValueCheck(name="j", type="int")])
 self.expect_expr("oneElemTypePackAfterNonTypeParam", 
result_type="TypePackAfterNonTypeParam<1, int>",
-result_children=[ValueCheck(name="j", type="int")])
\ No newline at end of file
+result_children=[ValueCheck(name="j", type="int")])



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

[Lldb-commits] [PATCH] D92425: [lldb] Don't reject empty or unnamed template parameter packs

2020-12-02 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor added a comment.

In D92425#2428844 , @stella.stamenova 
wrote:

> This caused failures on the Windows lldb buildbot:
>
> http://lab.llvm.org:8011/#/builders/83/builds/1294
>
> Tests have to have unique names, so the two new added tests that use TestCase 
> as the class name are both trying to write to the same log file and failing 
> to do so.

Thanks, fixed.

FWIW, this (frankly silly) requirement is actually supposed to be fixed by 
D83767 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92425

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

[Lldb-commits] [PATCH] D83767: [lldb] Use the basename of the Python test for the log name instead of the class name

2020-12-02 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor abandoned this revision.
teemperor added a comment.

Abandoned in favor of D92498 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83767

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

[Lldb-commits] [PATCH] D92510: [lldb] set created function decl to public access in TypeSystemClang

2020-12-02 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu created this revision.
zequanwu added reviewers: teemperor, shafik.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
zequanwu requested review of this revision.
Herald added a subscriber: JDevlieghere.

Test case stack_unwinding01.cpp crashed in Debug build of lldb on Windows due 
to access specifier not set for function decl.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92510

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp


Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2045,6 +2045,8 @@
   func_decl->setConstexprKind(isConstexprSpecified
   ? ConstexprSpecKind::Constexpr
   : ConstexprSpecKind::Unspecified);
+  if (decl_ctx->isRecord())
+func_decl->setAccess(clang::AccessSpecifier::AS_public);
   SetOwningModule(func_decl, owning_module);
   if (func_decl)
 decl_ctx->addDecl(func_decl);


Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2045,6 +2045,8 @@
   func_decl->setConstexprKind(isConstexprSpecified
   ? ConstexprSpecKind::Constexpr
   : ConstexprSpecKind::Unspecified);
+  if (decl_ctx->isRecord())
+func_decl->setAccess(clang::AccessSpecifier::AS_public);
   SetOwningModule(func_decl, owning_module);
   if (func_decl)
 decl_ctx->addDecl(func_decl);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92510: [lldb] set created function decl to public access in TypeSystemClang

2020-12-02 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

D85993  was trying to do the same thing and 
contains some explanation why I think this isn't a good solution. TL;DR is that 
functions in a record should call `TypeSystemClang::AddMethodToCXXRecordType` 
instead. Moving the `isRecord` check you add here to the PDB parser code and 
then calling `AddMethodToCXXRecordType` depending on whether the check is 
true/false is one way to fix this properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92510

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

[Lldb-commits] [PATCH] D92510: [lldb] set created function decl to public access in TypeSystemClang

2020-12-02 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added a comment.

+1 to what @teemperor said


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92510

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

[Lldb-commits] [PATCH] D92425: [lldb] Don't reject empty or unnamed template parameter packs

2020-12-02 Thread Stella Stamenova via Phabricator via lldb-commits 
stella.stamenova added a comment.

In D92425#2428952 , @teemperor wrote:

> In D92425#2428844 , 
> @stella.stamenova wrote:
>
>> This caused failures on the Windows lldb buildbot:
>>
>> http://lab.llvm.org:8011/#/builders/83/builds/1294
>>
>> Tests have to have unique names, so the two new added tests that use 
>> TestCase as the class name are both trying to write to the same log file and 
>> failing to do so.
>
> Thanks, fixed.
>
> FWIW, this (frankly silly) requirement is actually supposed to be fixed by 
> D83767 .

It would be great to finally have it fixed so don't have random failures 
because of file names!

It looks like the two tests are still failing though (different reason):

http://lab.llvm.org:8011/#/builders/83/builds/1332/steps/7/logs/stdio


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92425

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

[Lldb-commits] [PATCH] D92513: [lldb] Return the original path when tilde expansion fails.

2020-12-02 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added a reviewer: teemperor.
JDevlieghere requested review of this revision.

I was playing around with changing the $HOME directory and when I provided a 
non-existing path LLDB would crash. I traced it down to the 
TildeExpressionResolver which would return an empty string when expansion 
fails, which in turn would be converted to the current working directory when 
calling MakeAbsolute.


https://reviews.llvm.org/D92513

Files:
  lldb/source/Utility/TildeExpressionResolver.cpp


Index: lldb/source/Utility/TildeExpressionResolver.cpp
===
--- lldb/source/Utility/TildeExpressionResolver.cpp
+++ lldb/source/Utility/TildeExpressionResolver.cpp
@@ -73,11 +73,15 @@
 #endif
 }
 
+static void Assign(llvm::StringRef s, llvm::SmallVectorImpl &v) {
+  v.clear();
+  v.append(s.begin(), s.end());
+}
+
 bool TildeExpressionResolver::ResolveFullPath(
 StringRef Expr, llvm::SmallVectorImpl &Output) {
-  Output.clear();
   if (!Expr.startswith("~")) {
-Output.append(Expr.begin(), Expr.end());
+Assign(Expr, Output);
 return false;
   }
 
@@ -85,8 +89,10 @@
   StringRef Left =
   Expr.take_until([](char c) { return path::is_separator(c); });
 
-  if (!ResolveExact(Left, Output))
+  if (!ResolveExact(Left, Output)) {
+Assign(Expr, Output);
 return false;
+  }
 
   Output.append(Expr.begin() + Left.size(), Expr.end());
   return true;


Index: lldb/source/Utility/TildeExpressionResolver.cpp
===
--- lldb/source/Utility/TildeExpressionResolver.cpp
+++ lldb/source/Utility/TildeExpressionResolver.cpp
@@ -73,11 +73,15 @@
 #endif
 }
 
+static void Assign(llvm::StringRef s, llvm::SmallVectorImpl &v) {
+  v.clear();
+  v.append(s.begin(), s.end());
+}
+
 bool TildeExpressionResolver::ResolveFullPath(
 StringRef Expr, llvm::SmallVectorImpl &Output) {
-  Output.clear();
   if (!Expr.startswith("~")) {
-Output.append(Expr.begin(), Expr.end());
+Assign(Expr, Output);
 return false;
   }
 
@@ -85,8 +89,10 @@
   StringRef Left =
   Expr.take_until([](char c) { return path::is_separator(c); });
 
-  if (!ResolveExact(Left, Output))
+  if (!ResolveExact(Left, Output)) {
+Assign(Expr, Output);
 return false;
+  }
 
   Output.append(Expr.begin() + Left.size(), Expr.end());
   return true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] c49e718 - [lldb][NFC] Make DeclOrigin::Valid() const

2020-12-02 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-12-03T00:08:19+01:00
New Revision: c49e71805142ac3a27a5567ce516890e9243b34e

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

LOG: [lldb][NFC] Make DeclOrigin::Valid() const

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h
index b8c751479d4b..bf4ad174cf9c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h
@@ -160,7 +160,7 @@ class ClangASTImporter {
   decl = rhs.decl;
 }
 
-bool Valid() { return (ctx != nullptr || decl != nullptr); }
+bool Valid() const { return (ctx != nullptr || decl != nullptr); }
 
 clang::ASTContext *ctx;
 clang::Decl *decl;



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

[Lldb-commits] [PATCH] D92513: [lldb] Return the original path when tilde expansion fails.

2020-12-02 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 309073.
JDevlieghere added a comment.

Use `SmallVector::assign`


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

https://reviews.llvm.org/D92513

Files:
  lldb/source/Utility/TildeExpressionResolver.cpp


Index: lldb/source/Utility/TildeExpressionResolver.cpp
===
--- lldb/source/Utility/TildeExpressionResolver.cpp
+++ lldb/source/Utility/TildeExpressionResolver.cpp
@@ -75,9 +75,8 @@
 
 bool TildeExpressionResolver::ResolveFullPath(
 StringRef Expr, llvm::SmallVectorImpl &Output) {
-  Output.clear();
   if (!Expr.startswith("~")) {
-Output.append(Expr.begin(), Expr.end());
+Output.assign(Expr.begin(), Expr.end());
 return false;
   }
 
@@ -85,8 +84,10 @@
   StringRef Left =
   Expr.take_until([](char c) { return path::is_separator(c); });
 
-  if (!ResolveExact(Left, Output))
+  if (!ResolveExact(Left, Output)) {
+Output.assign(Expr.begin(), Expr.end());
 return false;
+  }
 
   Output.append(Expr.begin() + Left.size(), Expr.end());
   return true;


Index: lldb/source/Utility/TildeExpressionResolver.cpp
===
--- lldb/source/Utility/TildeExpressionResolver.cpp
+++ lldb/source/Utility/TildeExpressionResolver.cpp
@@ -75,9 +75,8 @@
 
 bool TildeExpressionResolver::ResolveFullPath(
 StringRef Expr, llvm::SmallVectorImpl &Output) {
-  Output.clear();
   if (!Expr.startswith("~")) {
-Output.append(Expr.begin(), Expr.end());
+Output.assign(Expr.begin(), Expr.end());
 return false;
   }
 
@@ -85,8 +84,10 @@
   StringRef Left =
   Expr.take_until([](char c) { return path::is_separator(c); });
 
-  if (!ResolveExact(Left, Output))
+  if (!ResolveExact(Left, Output)) {
+Output.assign(Expr.begin(), Expr.end());
 return false;
+  }
 
   Output.append(Expr.begin() + Left.size(), Expr.end());
   return true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92452: [lldb] Treat remote macOS debugging like any other remote darwin platform

2020-12-02 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp:129
+   ArchSpec &arch) {
+#if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
+  // macOS for ARM64 support both native and translated x86_64 processes

Given that this is the *remote* platform — does this #if even make sense?



Comment at: lldb/test/API/commands/platform/sdk/TestPlatformSDK.py:20
+# The port used by debugserver.
+PORT = 54637
+

What happens if two tests run at the same time?



Comment at: lldb/test/API/commands/platform/sdk/TestPlatformSDK.py:26
+# system.
+TIMEOUT = 2
+

2 seconds timeout is definitely not enough for an asan bot. Can this be closer 
to 20s? Perhaps the result of reading 
`target.process.utility-expression-timeout`?



Comment at: lldb/test/API/commands/platform/sdk/TestPlatformSDK.py:90
+# Give debugserver time to attach.
+time.sleep(self.TIMEOUT)
+

I guess there is no way of implementing this without sleeping? Maybe a loop 
with 10 smaller sleeps?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92452

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

[Lldb-commits] [PATCH] D92513: [lldb] Return the original path when tilde expansion fails.

2020-12-02 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 309078.
JDevlieghere added a comment.

Update the unittest


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

https://reviews.llvm.org/D92513

Files:
  lldb/source/Utility/TildeExpressionResolver.cpp
  lldb/unittests/Utility/TildeExpressionResolverTest.cpp


Index: lldb/unittests/Utility/TildeExpressionResolverTest.cpp
===
--- lldb/unittests/Utility/TildeExpressionResolverTest.cpp
+++ lldb/unittests/Utility/TildeExpressionResolverTest.cpp
@@ -31,6 +31,9 @@
   EXPECT_EQ("/lars", Result);
 
   ASSERT_FALSE(Resolver.ResolveFullPath("~Jaso", Result));
+  EXPECT_EQ("~Jaso", Result);
   ASSERT_FALSE(Resolver.ResolveFullPath("", Result));
+  EXPECT_EQ("", Result);
   ASSERT_FALSE(Resolver.ResolveFullPath("Jason", Result));
+  EXPECT_EQ("Jason", Result);
 }
Index: lldb/source/Utility/TildeExpressionResolver.cpp
===
--- lldb/source/Utility/TildeExpressionResolver.cpp
+++ lldb/source/Utility/TildeExpressionResolver.cpp
@@ -75,9 +75,8 @@
 
 bool TildeExpressionResolver::ResolveFullPath(
 StringRef Expr, llvm::SmallVectorImpl &Output) {
-  Output.clear();
   if (!Expr.startswith("~")) {
-Output.append(Expr.begin(), Expr.end());
+Output.assign(Expr.begin(), Expr.end());
 return false;
   }
 
@@ -85,8 +84,10 @@
   StringRef Left =
   Expr.take_until([](char c) { return path::is_separator(c); });
 
-  if (!ResolveExact(Left, Output))
+  if (!ResolveExact(Left, Output)) {
+Output.assign(Expr.begin(), Expr.end());
 return false;
+  }
 
   Output.append(Expr.begin() + Left.size(), Expr.end());
   return true;


Index: lldb/unittests/Utility/TildeExpressionResolverTest.cpp
===
--- lldb/unittests/Utility/TildeExpressionResolverTest.cpp
+++ lldb/unittests/Utility/TildeExpressionResolverTest.cpp
@@ -31,6 +31,9 @@
   EXPECT_EQ("/lars", Result);
 
   ASSERT_FALSE(Resolver.ResolveFullPath("~Jaso", Result));
+  EXPECT_EQ("~Jaso", Result);
   ASSERT_FALSE(Resolver.ResolveFullPath("", Result));
+  EXPECT_EQ("", Result);
   ASSERT_FALSE(Resolver.ResolveFullPath("Jason", Result));
+  EXPECT_EQ("Jason", Result);
 }
Index: lldb/source/Utility/TildeExpressionResolver.cpp
===
--- lldb/source/Utility/TildeExpressionResolver.cpp
+++ lldb/source/Utility/TildeExpressionResolver.cpp
@@ -75,9 +75,8 @@
 
 bool TildeExpressionResolver::ResolveFullPath(
 StringRef Expr, llvm::SmallVectorImpl &Output) {
-  Output.clear();
   if (!Expr.startswith("~")) {
-Output.append(Expr.begin(), Expr.end());
+Output.assign(Expr.begin(), Expr.end());
 return false;
   }
 
@@ -85,8 +84,10 @@
   StringRef Left =
   Expr.take_until([](char c) { return path::is_separator(c); });
 
-  if (!ResolveExact(Left, Output))
+  if (!ResolveExact(Left, Output)) {
+Output.assign(Expr.begin(), Expr.end());
 return false;
+  }
 
   Output.append(Expr.begin() + Left.size(), Expr.end());
   return true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92513: [lldb] Return the original path when tilde expansion fails.

2020-12-02 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

Thanks, I do féél this ameliorates the current behavióúr


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

https://reviews.llvm.org/D92513

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

[Lldb-commits] [lldb] 640567d - [lldb] X-FAIL class template parameter pack tests on Windows

2020-12-02 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-12-03T00:38:05+01:00
New Revision: 640567d4646292f77e87e77b8710ebf1bde1f390

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

LOG: [lldb] X-FAIL class template parameter pack tests on Windows

Both seem to fail to read values from the non-running target.

Added: 


Modified: 

lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py

lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py

Removed: 




diff  --git 
a/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
 
b/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
index 6b4b96049cdb1..3a51e08d75e08 100644
--- 
a/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
+++ 
b/lldb/test/API/lang/cpp/class-template-non-type-parameter-pack/TestClassTemplateNonTypeParameterPack.py
@@ -7,6 +7,7 @@ class TestCaseClassTemplateNonTypeParameterPack(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@expectedFailureAll(oslist=["windows"]) # Fails to read memory from target.
 @no_debug_info_test
 def test(self):
 self.build()

diff  --git 
a/lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
 
b/lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
index 223ba52242742..88beac18e891a 100644
--- 
a/lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
+++ 
b/lldb/test/API/lang/cpp/class-template-type-parameter-pack/TestClassTemplateTypeParameterPack.py
@@ -7,6 +7,7 @@ class TestCaseClassTemplateTypeParameterPack(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@expectedFailureAll(oslist=["windows"]) # Fails to read memory from target.
 @no_debug_info_test
 def test(self):
 self.build()



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

[Lldb-commits] [PATCH] D92452: [lldb] Treat remote macOS debugging like any other remote darwin platform

2020-12-02 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 309087.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

Address @aprantl's feedback


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

https://reviews.llvm.org/D92452

Files:
  lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h
  lldb/test/API/commands/platform/sdk/Makefile
  lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
  lldb/test/API/commands/platform/sdk/main.c

Index: lldb/test/API/commands/platform/sdk/main.c
===
--- /dev/null
+++ lldb/test/API/commands/platform/sdk/main.c
@@ -0,0 +1,19 @@
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  FILE *f = fopen(argv[1], "wx");
+  if (f) {
+fputs("\n", f);
+fflush(f);
+fclose(f);
+  } else {
+return 1;
+  }
+
+  for (int i = 0; i != 10; ++i) {
+sleep(10);
+  }
+
+  return 0;
+}
Index: lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
===
--- /dev/null
+++ lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
@@ -0,0 +1,115 @@
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import os
+import platform
+import shutil
+import time
+import socket
+
+
+class PlatformSDKTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# The port used by debugserver.
+PORT = 54637
+
+# The number of attempts.
+ATTEMPTS = 10
+
+# Time given to the binary to launch and to debugserver to attach to it for
+# every attempt. We'll wait a maximum of 10 times 2 seconds while the
+# inferior will wait 10 times 10 seconds.
+TIMEOUT = 2
+
+def no_debugserver(self):
+if os.getenv('LLDB_DEBUGSERVER_PATH') is None:
+return 'no debugserver'
+return None
+
+def port_not_available(self):
+s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+if s.connect_ex(('127.0.0.1', self.PORT)) == 0:
+return '{} not available'.format(self.PORT)
+return None
+
+@no_debug_info_test
+@skipUnlessDarwin
+@expectedFailureIfFn(no_debugserver)
+@expectedFailureIfFn(port_not_available)
+def test_macos_sdk(self):
+self.build()
+
+exe = self.getBuildArtifact('a.out')
+token = self.getBuildArtifact('token')
+
+# Remove the old token.
+try:
+os.remove(token)
+except:
+pass
+
+# Create a fake 'SDK' directory.
+test_home = os.path.join(self.getBuildDir(), 'fake_home.noindex')
+macos_version = platform.mac_ver()[0]
+sdk_dir = os.path.join(test_home, 'Library', 'Developer', 'Xcode',
+   'macOS DeviceSupport', macos_version)
+symbols_dir = os.path.join(sdk_dir, 'Symbols')
+lldbutil.mkdir_p(symbols_dir)
+
+# Save the current home directory and restore it afterwards.
+old_home = os.getenv('HOME')
+
+def cleanup():
+if not old_home:
+del os.environ['HOME']
+else:
+os.environ['HOME'] = old_home
+
+self.addTearDownHook(cleanup)
+os.environ['HOME'] = test_home
+
+# Launch our test binary.
+inferior = self.spawnSubprocess(exe, [token])
+pid = inferior.pid
+
+# Wait for the binary to launch.
+lldbutil.wait_for_file_on_target(self, token)
+
+# Move the binary into the 'SDK'.
+rel_exe_path = os.path.relpath(exe, '/')
+exe_sdk_path = os.path.join(symbols_dir, rel_exe_path)
+lldbutil.mkdir_p(os.path.dirname(exe_sdk_path))
+shutil.move(exe, exe_sdk_path)
+
+# Attach to it with debugserver.
+debugserver = os.getenv('LLDB_DEBUGSERVER_PATH')
+debugserver_args = [
+'localhost:{}'.format(self.PORT), '--attach={}'.format(pid)
+]
+self.spawnSubprocess(debugserver, debugserver_args)
+
+# Select the platform.
+self.expect('platform select remote-macosx', substrs=[sdk_dir])
+
+# Connect to debugserver
+interpreter = self.dbg.GetCommandInterpreter()
+connected = False
+for i in range(self.ATTEMPTS):
+result = lldb.SBCommandReturnObject()
+interpreter.HandleCommand('gdb-remote {}'.format(self.PORT),
+  result)
+connected = result.Succeeded()
+if connected:
+break
+time.sleep(self.TIMEOUT)
+
+self.assertTrue(connected, "could not connect to debugserver")
+
+

[Lldb-commits] [PATCH] D92452: [lldb] Treat remote macOS debugging like any other remote darwin platform

2020-12-02 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere marked 4 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/test/API/commands/platform/sdk/TestPlatformSDK.py:20
+# The port used by debugserver.
+PORT = 54637
+

aprantl wrote:
> What happens if two tests run at the same time?
The test will be XFAILED because of `@expectedFailureIfFn(port_not_available)`.


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

https://reviews.llvm.org/D92452

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

[Lldb-commits] [lldb] dcdd231 - [lldb] Return the original path when tilde expansion fails.

2020-12-02 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2020-12-02T16:01:30-08:00
New Revision: dcdd231df6c7b6f4a635fe7941c1f87955328183

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

LOG: [lldb] Return the original path when tilde expansion fails.

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

Added: 


Modified: 
lldb/source/Utility/TildeExpressionResolver.cpp
lldb/unittests/Utility/TildeExpressionResolverTest.cpp

Removed: 




diff  --git a/lldb/source/Utility/TildeExpressionResolver.cpp 
b/lldb/source/Utility/TildeExpressionResolver.cpp
index c8a0800cb807..75d9c47e656d 100644
--- a/lldb/source/Utility/TildeExpressionResolver.cpp
+++ b/lldb/source/Utility/TildeExpressionResolver.cpp
@@ -75,9 +75,8 @@ bool 
StandardTildeExpressionResolver::ResolvePartial(StringRef Expr,
 
 bool TildeExpressionResolver::ResolveFullPath(
 StringRef Expr, llvm::SmallVectorImpl &Output) {
-  Output.clear();
   if (!Expr.startswith("~")) {
-Output.append(Expr.begin(), Expr.end());
+Output.assign(Expr.begin(), Expr.end());
 return false;
   }
 
@@ -85,8 +84,10 @@ bool TildeExpressionResolver::ResolveFullPath(
   StringRef Left =
   Expr.take_until([](char c) { return path::is_separator(c); });
 
-  if (!ResolveExact(Left, Output))
+  if (!ResolveExact(Left, Output)) {
+Output.assign(Expr.begin(), Expr.end());
 return false;
+  }
 
   Output.append(Expr.begin() + Left.size(), Expr.end());
   return true;

diff  --git a/lldb/unittests/Utility/TildeExpressionResolverTest.cpp 
b/lldb/unittests/Utility/TildeExpressionResolverTest.cpp
index bc1ca7a44811..bcb7fdb8604d 100644
--- a/lldb/unittests/Utility/TildeExpressionResolverTest.cpp
+++ b/lldb/unittests/Utility/TildeExpressionResolverTest.cpp
@@ -31,6 +31,9 @@ TEST(TildeExpressionResolver, ResolveFullPath) {
   EXPECT_EQ("/lars", Result);
 
   ASSERT_FALSE(Resolver.ResolveFullPath("~Jaso", Result));
+  EXPECT_EQ("~Jaso", Result);
   ASSERT_FALSE(Resolver.ResolveFullPath("", Result));
+  EXPECT_EQ("", Result);
   ASSERT_FALSE(Resolver.ResolveFullPath("Jason", Result));
+  EXPECT_EQ("Jason", Result);
 }



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

[Lldb-commits] [PATCH] D92513: [lldb] Return the original path when tilde expansion fails.

2020-12-02 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdcdd231df6c7: [lldb] Return the original path when tilde 
expansion fails. (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92513

Files:
  lldb/source/Utility/TildeExpressionResolver.cpp
  lldb/unittests/Utility/TildeExpressionResolverTest.cpp


Index: lldb/unittests/Utility/TildeExpressionResolverTest.cpp
===
--- lldb/unittests/Utility/TildeExpressionResolverTest.cpp
+++ lldb/unittests/Utility/TildeExpressionResolverTest.cpp
@@ -31,6 +31,9 @@
   EXPECT_EQ("/lars", Result);
 
   ASSERT_FALSE(Resolver.ResolveFullPath("~Jaso", Result));
+  EXPECT_EQ("~Jaso", Result);
   ASSERT_FALSE(Resolver.ResolveFullPath("", Result));
+  EXPECT_EQ("", Result);
   ASSERT_FALSE(Resolver.ResolveFullPath("Jason", Result));
+  EXPECT_EQ("Jason", Result);
 }
Index: lldb/source/Utility/TildeExpressionResolver.cpp
===
--- lldb/source/Utility/TildeExpressionResolver.cpp
+++ lldb/source/Utility/TildeExpressionResolver.cpp
@@ -75,9 +75,8 @@
 
 bool TildeExpressionResolver::ResolveFullPath(
 StringRef Expr, llvm::SmallVectorImpl &Output) {
-  Output.clear();
   if (!Expr.startswith("~")) {
-Output.append(Expr.begin(), Expr.end());
+Output.assign(Expr.begin(), Expr.end());
 return false;
   }
 
@@ -85,8 +84,10 @@
   StringRef Left =
   Expr.take_until([](char c) { return path::is_separator(c); });
 
-  if (!ResolveExact(Left, Output))
+  if (!ResolveExact(Left, Output)) {
+Output.assign(Expr.begin(), Expr.end());
 return false;
+  }
 
   Output.append(Expr.begin() + Left.size(), Expr.end());
   return true;


Index: lldb/unittests/Utility/TildeExpressionResolverTest.cpp
===
--- lldb/unittests/Utility/TildeExpressionResolverTest.cpp
+++ lldb/unittests/Utility/TildeExpressionResolverTest.cpp
@@ -31,6 +31,9 @@
   EXPECT_EQ("/lars", Result);
 
   ASSERT_FALSE(Resolver.ResolveFullPath("~Jaso", Result));
+  EXPECT_EQ("~Jaso", Result);
   ASSERT_FALSE(Resolver.ResolveFullPath("", Result));
+  EXPECT_EQ("", Result);
   ASSERT_FALSE(Resolver.ResolveFullPath("Jason", Result));
+  EXPECT_EQ("Jason", Result);
 }
Index: lldb/source/Utility/TildeExpressionResolver.cpp
===
--- lldb/source/Utility/TildeExpressionResolver.cpp
+++ lldb/source/Utility/TildeExpressionResolver.cpp
@@ -75,9 +75,8 @@
 
 bool TildeExpressionResolver::ResolveFullPath(
 StringRef Expr, llvm::SmallVectorImpl &Output) {
-  Output.clear();
   if (!Expr.startswith("~")) {
-Output.append(Expr.begin(), Expr.end());
+Output.assign(Expr.begin(), Expr.end());
 return false;
   }
 
@@ -85,8 +84,10 @@
   StringRef Left =
   Expr.take_until([](char c) { return path::is_separator(c); });
 
-  if (!ResolveExact(Left, Output))
+  if (!ResolveExact(Left, Output)) {
+Output.assign(Expr.begin(), Expr.end());
 return false;
+  }
 
   Output.append(Expr.begin() + Left.size(), Expr.end());
   return true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92452: [lldb] Treat remote macOS debugging like any other remote darwin platform

2020-12-02 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added inline comments.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp:98-99
+PlatformSP PlatformMacOSX::CreateInstance(bool force, const ArchSpec *arch) {
+  // The only time we create an instance is when we are creating a remote
+  // macosx platform which is handled by PlatformRemoteMacOSX.
+  return PlatformSP();

Where again do we create the host platform then?



Comment at: lldb/test/API/commands/platform/sdk/main.c:4-6
+  for (int i = 0; i != 10; ++i) {
+sleep(10);
+  }

You can call pause() which will stop and wait for a signal to be delivered to 
the program if you don't want to call sleep. When the debugger attaches, this 
will cause the pause() to exit when you continue the process.


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

https://reviews.llvm.org/D92452

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

[Lldb-commits] [PATCH] D92452: [lldb] Treat remote macOS debugging like any other remote darwin platform

2020-12-02 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp:98-99
+PlatformSP PlatformMacOSX::CreateInstance(bool force, const ArchSpec *arch) {
+  // The only time we create an instance is when we are creating a remote
+  // macosx platform which is handled by PlatformRemoteMacOSX.
+  return PlatformSP();

clayborg wrote:
> Where again do we create the host platform then?
On line 58 in `PlatformMacOSX::Initialize`



Comment at: lldb/test/API/commands/platform/sdk/main.c:4-6
+  for (int i = 0; i != 10; ++i) {
+sleep(10);
+  }

clayborg wrote:
> You can call pause() which will stop and wait for a signal to be delivered to 
> the program if you don't want to call sleep. When the debugger attaches, this 
> will cause the pause() to exit when you continue the process.
Oh cool, learned something new today! 


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

https://reviews.llvm.org/D92452

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

[Lldb-commits] [PATCH] D92452: [lldb] Treat remote macOS debugging like any other remote darwin platform

2020-12-02 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.
Closed by commit rGdd2054d38a84: [lldb] Treat remote macOS debugging like any 
other remote darwin platform (authored by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D92452?vs=309087&id=309106#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92452

Files:
  lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h
  lldb/test/API/commands/platform/sdk/Makefile
  lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
  lldb/test/API/commands/platform/sdk/main.c
  lldb/unittests/Process/ProcessEventDataTest.cpp

Index: lldb/unittests/Process/ProcessEventDataTest.cpp
===
--- lldb/unittests/Process/ProcessEventDataTest.cpp
+++ lldb/unittests/Process/ProcessEventDataTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
@@ -151,7 +152,7 @@
 TEST_F(ProcessEventDataTest, DoOnRemoval) {
   ArchSpec arch("x86_64-apple-macosx-");
 
-  Platform::SetHostPlatform(PlatformMacOSX::CreateInstance(true, &arch));
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
 
   DebuggerSP debugger_sp = Debugger::CreateInstance();
   ASSERT_TRUE(debugger_sp);
@@ -191,7 +192,7 @@
 TEST_F(ProcessEventDataTest, ShouldStop) {
   ArchSpec arch("x86_64-apple-macosx-");
 
-  Platform::SetHostPlatform(PlatformMacOSX::CreateInstance(true, &arch));
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
 
   DebuggerSP debugger_sp = Debugger::CreateInstance();
   ASSERT_TRUE(debugger_sp);
Index: lldb/test/API/commands/platform/sdk/main.c
===
--- /dev/null
+++ lldb/test/API/commands/platform/sdk/main.c
@@ -0,0 +1,16 @@
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  FILE *f = fopen(argv[1], "wx");
+  if (f) {
+fputs("\n", f);
+fflush(f);
+fclose(f);
+  } else {
+return 1;
+  }
+
+  pause();
+  return 0;
+}
Index: lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
===
--- /dev/null
+++ lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
@@ -0,0 +1,115 @@
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import os
+import platform
+import shutil
+import time
+import socket
+
+
+class PlatformSDKTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# The port used by debugserver.
+PORT = 54637
+
+# The number of attempts.
+ATTEMPTS = 10
+
+# Time given to the binary to launch and to debugserver to attach to it for
+# every attempt. We'll wait a maximum of 10 times 2 seconds while the
+# inferior will wait 10 times 10 seconds.
+TIMEOUT = 2
+
+def no_debugserver(self):
+if os.getenv('LLDB_DEBUGSERVER_PATH') is None:
+return 'no debugserver'
+return None
+
+def port_not_available(self):
+s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+if s.connect_ex(('127.0.0.1', self.PORT)) == 0:
+return '{} not available'.format(self.PORT)
+return None
+
+@no_debug_info_test
+@skipUnlessDarwin
+@expectedFailureIfFn(no_debugserver)
+@expectedFailureIfFn(port_not_available)
+def test_macos_sdk(self):
+self.build()
+
+exe = self.getBuildArtifact('a.out')
+token = self.getBuildArtifact('token')
+
+# Remove the old token.
+try:
+os.remove(token)
+except:
+pass
+
+# Create a fake 'SDK' directory.
+test_home = os.path.join(self.getBuildDir(), 'fake_home.noindex')
+macos_version = platform.mac_ver()[0]
+sdk_dir = os.path.join(test_home, 'Library', 'Developer', 'Xcode',
+   'macOS DeviceSupport', macos_version)
+symbols_dir = os.path.join(sdk_dir, 'Symbols')
+lldbutil.mkdir_p(symbols_dir)
+
+# Save the current home directory and restore it afterwards.
+old_home = os.getenv('HOME')
+
+def cleanup():
+if not old_home:
+del os.environ['HOME']
+else:
+os.environ['HOME'] = old_home
+
+self.add

[Lldb-commits] [lldb] dd2054d - [lldb] Treat remote macOS debugging like any other remote darwin platform

2020-12-02 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2020-12-02T17:03:22-08:00
New Revision: dd2054d38a848a75fe84fb68d9c3a97e5ade6753

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

LOG: [lldb] Treat remote macOS debugging like any other remote darwin platform

Extract remote debugging logic from PlatformMacOSX and move it into
PlatformRemoteMacOSX so it can benefit from all the logic necessary for
remote debugging.

Until now, remote macOS debugging was treated almost identical to local
macOS debugging. By moving in into its own class, we can have it inherit
from PlatformRemoteDarwinDevice and all the functionality it provides,
such as looking at the correct DeviceSupport directory.

rdar://68167374

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

Added: 
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h
lldb/test/API/commands/platform/sdk/Makefile
lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
lldb/test/API/commands/platform/sdk/main.c

Modified: 
lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
lldb/unittests/Process/ProcessEventDataTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt 
b/lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
index 8b5be337f45b..bd9343773b3c 100644
--- a/lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
+++ b/lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
@@ -10,11 +10,12 @@ list(APPEND PLUGIN_PLATFORM_MACOSX_SOURCES
   PlatformDarwin.cpp
   PlatformDarwinKernel.cpp
   PlatformMacOSX.cpp
-  PlatformRemoteiOS.cpp
+  PlatformRemoteAppleBridge.cpp
   PlatformRemoteAppleTV.cpp
   PlatformRemoteAppleWatch.cpp
   PlatformRemoteDarwinDevice.cpp
-  PlatformRemoteAppleBridge.cpp
+  PlatformRemoteMacOSX.cpp
+  PlatformRemoteiOS.cpp
   )
 
 list(APPEND PLUGIN_PLATFORM_MACOSX_DARWIN_ONLY_SOURCES

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
index a139d4a2c454..24df03e18dda 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "PlatformMacOSX.h"
+#include "PlatformRemoteMacOSX.h"
 #include "PlatformRemoteiOS.h"
 #if defined(__APPLE__)
 #include "PlatformAppleSimulator.h"
@@ -44,6 +45,7 @@ static uint32_t g_initialize_count = 0;
 void PlatformMacOSX::Initialize() {
   PlatformDarwin::Initialize();
   PlatformRemoteiOS::Initialize();
+  PlatformRemoteMacOSX::Initialize();
 #if defined(__APPLE__)
   PlatformAppleSimulator::Initialize();
   PlatformDarwinKernel::Initialize();
@@ -54,12 +56,12 @@ void PlatformMacOSX::Initialize() {
 
   if (g_initialize_count++ == 0) {
 #if defined(__APPLE__)
-PlatformSP default_platform_sp(new PlatformMacOSX(true));
+PlatformSP default_platform_sp(new PlatformMacOSX());
 default_platform_sp->SetSystemArchitecture(HostInfo::GetArchitecture());
 Platform::SetHostPlatform(default_platform_sp);
 #endif
-PluginManager::RegisterPlugin(PlatformMacOSX::GetPluginNameStatic(false),
-  PlatformMacOSX::GetDescriptionStatic(false),
+PluginManager::RegisterPlugin(PlatformMacOSX::GetPluginNameStatic(),
+  PlatformMacOSX::GetDescriptionStatic(),
   PlatformMacOSX::CreateInstance);
   }
 }
@@ -78,98 +80,28 @@ void PlatformMacOSX::Terminate() {
   PlatformDarwinKernel::Terminate();
   PlatformAppleSimulator::Terminate();
 #endif
+  PlatformRemoteMacOSX::Initialize();
   PlatformRemoteiOS::Terminate();
   PlatformDarwin::Terminate();
 }
 
-PlatformSP PlatformMacOSX::CreateInstance(bool force, const ArchSpec *arch) {
-  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
-  if (log) {
-const char *arch_name;
-if (arch && arch->GetArchitectureName())
-  arch_name = arch->GetArchitectureName();
-else
-  arch_name = "";
-
-const char *triple_cstr =
-arch ? arch->GetTriple().getTriple().c_str() : "";
-
-LLDB_LOGF(log, "PlatformMacOSX::%s(force=%s, arch={%s,%s})", __FUNCTION__,
-  force ? "true" : "false", arch_name, triple_cstr);
-  }
-
-  // The only time we create an instance is when we are creating a remote
-  // macosx platform
-  const bool is_host = false;
-
-  bool create = force;
-  if (!create && arch && arch->IsValid()) {
-const llvm::Triple &triple = arch->GetTriple();
-switch (triple.getVendor()) {
-case llvm::Triple::Apple:
-  create = true;
-  b