[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz updated this revision to Diff 367401.
saschwartz added a comment.

Also propagate platform mode return codes and add additional test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/test/Shell/lldb-server/TestGdbserverPort.test
  lldb/tools/lldb-server/lldb-platform.cpp
  lldb/tools/lldb-server/lldb-server.cpp

Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -52,29 +52,32 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
+  int ret = 0;
   switch (argv[1][0]) {
   case 'g':
 llgs::initialize();
-main_gdbserver(argc, argv);
+ret = main_gdbserver(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'p':
 llgs::initialize();
-main_platform(argc, argv);
+ret = main_platform(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+ret = 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+ret = 1;
+break;
   }
+  return ret;
 }
Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -92,7 +92,6 @@
   "log-channel-list] [--port-file port-file-path] --server "
   "--listen port\n",
   progname, subcommand);
-  exit(0);
 }
 
 static Status save_socket_id_to_file(const std::string _id,
@@ -269,7 +268,7 @@
 
   if (show_usage || option_error) {
 display_usage(progname, subcommand);
-exit(option_error);
+return option_error;
   }
 
   // Skip any options we consumed with getopt_long_only.
@@ -288,13 +287,13 @@
   listen_host_port, children_inherit_listen_socket, error));
   if (error.Fail()) {
 fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+return socket_error;
   }
 
   error = acceptor_up->Listen(backlog);
   if (error.Fail()) {
 printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+return socket_error;
   }
   if (socket_file) {
 error =
@@ -322,7 +321,7 @@
 error = acceptor_up->Accept(children_inherit_accept_socket, conn);
 if (error.Fail()) {
   WithColor::error() << error.AsCString() << '\n';
-  exit(socket_error);
+  return socket_error;
 }
 printf("Connection established.\n");
 if (g_server) {
Index: lldb/test/Shell/lldb-server/TestGdbserverPort.test
===
--- lldb/test/Shell/lldb-server/TestGdbserverPort.test
+++ lldb/test/Shell/lldb-server/TestGdbserverPort.test
@@ -1,4 +1,4 @@
 # Windows does not build lldb-server.
 # UNSUPPORTED: system-windows
-# RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
+# RUN: not %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
 # CHECK: error: --min-gdbserver-port (1234) is not lower than --max-gdbserver-port (1234)
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,14 +1,26 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,GDB_REMOTE_ALL %s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,GDB_REMOTE_ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,GDB_REMOTE_ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,GDB_REMOTE_ALL %s
 CONN: error: no connection arguments
 
-ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.
+RUN: %lldb-server platform 2>&1 | FileCheck --check-prefixes=LLDB_PLATFORM_ALL %s
+
+RUN: %lldb-server platform --fd 2>&1 | FileCheck --check-prefixes=FD3,LLDB_PLATFORM_ALL %s

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz updated this revision to Diff 367397.
saschwartz added a comment.

Fix unannotated fallthrough in switch statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/tools/lldb-server/lldb-server.cpp


Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -52,29 +52,32 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
+  int ret = 0;
   switch (argv[1][0]) {
   case 'g':
 llgs::initialize();
-main_gdbserver(argc, argv);
+ret = main_gdbserver(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'p':
 llgs::initialize();
-main_platform(argc, argv);
+ret = main_platform(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+ret = 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+ret = 1;
+break;
   }
+  return ret;
 }
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,13 +1,13 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL 
%s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck 
--check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck 
--check-prefixes=FD2,ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck 
--check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck 
--check-prefixes=BOGUS,ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
 CONN: error: no connection arguments
 
 ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.


Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -52,29 +52,32 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
+  int ret = 0;
   switch (argv[1][0]) {
   case 'g':
 llgs::initialize();
-main_gdbserver(argc, argv);
+ret = main_gdbserver(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'p':
 llgs::initialize();
-main_platform(argc, argv);
+ret = main_platform(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+ret = 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+ret = 1;
+break;
   }
+  return ret;
 }
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,13 +1,13 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
 CONN: error: no connection arguments
 
 ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz created this revision.
saschwartz added a reviewer: clayborg.
saschwartz requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This diff modifies the LLDB server return codes to more accurately
reflect usage error paths. It additionally modifies the associated unit test
to check that we have nonzero return codes on error conditions.

Test Plan:
LLDB tests pass:

  ninja check-lldb


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/tools/lldb-server/lldb-server.cpp


Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -52,29 +52,29 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
+  int ret = 0;
   switch (argv[1][0]) {
   case 'g':
 llgs::initialize();
-main_gdbserver(argc, argv);
+ret = main_gdbserver(argc, argv);
 llgs::terminate_debugger();
-break;
   case 'p':
 llgs::initialize();
-main_platform(argc, argv);
+ret = main_platform(argc, argv);
 llgs::terminate_debugger();
-break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+ret = 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+ret = 1;
   }
+  return ret;
 }
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,13 +1,13 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL 
%s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck 
--check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck 
--check-prefixes=FD2,ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck 
--check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck 
--check-prefixes=BOGUS,ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
 CONN: error: no connection arguments
 
 ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.


Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -52,29 +52,29 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
+  int ret = 0;
   switch (argv[1][0]) {
   case 'g':
 llgs::initialize();
-main_gdbserver(argc, argv);
+ret = main_gdbserver(argc, argv);
 llgs::terminate_debugger();
-break;
   case 'p':
 llgs::initialize();
-main_platform(argc, argv);
+ret = main_platform(argc, argv);
 llgs::terminate_debugger();
-break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+ret = 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+ret = 1;
   }
+  return ret;
 }
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,13 +1,13 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
 CONN: error: no connection arguments
 
 ALL: Use '{{.*}} g[dbserver] --help' for a 

[Lldb-commits] [PATCH] D106035: Remove conditional compilation for WCHAR support in libedit

[Neal via Phabricator via lldb-commits]

nealsid added a comment.

Sorry, I haven't forgotten about this.  Haven't had time to sit down for a 
proper debugging session this or last week.  I definitely still plan to update 
and resend for review.  Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106035

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

[Lldb-commits] [PATCH] D108078: [lldb] Support gdbserver signals

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

Didn't mean to accept yet (originally I thought the native-signals check was 
inverted)


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

https://reviews.llvm.org/D108078

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

[Lldb-commits] [PATCH] D108078: [lldb] Support gdbserver signals

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1344
   "qEcho+",
+  "native-signals+",
   };

What about `debugserver`, I assume that would need to be updated as well? 


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

https://reviews.llvm.org/D108078

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

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Greg Clayton 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 rG82507f179876: [LLDB][GUI] Add Process Launch form (authored 
by OmarEmaraDev, committed by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

Files:
  lldb/include/lldb/Target/Target.h
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3985,6 +3985,45 @@
   return ComputeEnvironment();
 }
 
+Environment TargetProperties::GetInheritedEnvironment() const {
+  Environment environment;
+
+  if (m_target == nullptr)
+return environment;
+
+  if (!m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, ePropertyInheritEnv,
+  g_target_properties[ePropertyInheritEnv].default_uint_value != 0))
+return environment;
+
+  PlatformSP platform_sp = m_target->GetPlatform();
+  if (platform_sp == nullptr)
+return environment;
+
+  Environment platform_environment = platform_sp->GetEnvironment();
+  for (const auto  : platform_environment)
+environment[KV.first()] = KV.second;
+
+  Args property_unset_environment;
+  m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyUnsetEnvVars,
+property_unset_environment);
+  for (const auto  : property_unset_environment)
+environment.erase(var.ref());
+
+  return environment;
+}
+
+Environment TargetProperties::GetTargetEnvironment() const {
+  Args property_environment;
+  m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyEnvVars,
+property_environment);
+  Environment environment;
+  for (const auto  : Environment(property_environment))
+environment[KV.first()] = KV.second;
+
+  return environment;
+}
+
 void TargetProperties::SetEnvironment(Environment env) {
   // TODO: Get rid of the Args intermediate step
   const uint32_t idx = ePropertyEnvVars;
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1259,6 +1259,14 @@
 
   const std::string () { return m_content; }
 
+  void SetText(const char *text) {
+if (text == nullptr) {
+  m_content.clear();
+  return;
+}
+m_content = text;
+  }
+
 protected:
   std::string m_label;
   bool m_required;
@@ -1618,6 +1626,33 @@
   }
 };
 
+class LazyBooleanFieldDelegate : public ChoicesFieldDelegate {
+public:
+  LazyBooleanFieldDelegate(const char *label, const char *calculate_label)
+  : ChoicesFieldDelegate(label, 3, GetPossibleOptions(calculate_label)) {}
+
+  static constexpr const char *kNo = "No";
+  static constexpr const char *kYes = "Yes";
+
+  std::vector GetPossibleOptions(const char *calculate_label) {
+std::vector options;
+options.push_back(calculate_label);
+options.push_back(kYes);
+options.push_back(kNo);
+return options;
+  }
+
+  LazyBool GetLazyBoolean() {
+std::string choice = GetChoiceContent();
+if (choice == kNo)
+  return eLazyBoolNo;
+else if (choice == kYes)
+  return eLazyBoolYes;
+else
+  return eLazyBoolCalculate;
+  }
+};
+
 template  class ListFieldDelegate : public FieldDelegate {
 public:
   ListFieldDelegate(const char *label, T default_field)
@@ -1908,6 +1943,29 @@
   SelectionType m_selection_type;
 };
 
+class ArgumentsFieldDelegate : public ListFieldDelegate {
+public:
+  ArgumentsFieldDelegate()
+  : ListFieldDelegate("Arguments",
+  TextFieldDelegate("Argument", "", false)) {}
+
+  Args GetArguments() {
+Args arguments;
+for (int i = 0; i < GetNumberOfFields(); i++) {
+  arguments.AppendArgument(GetField(i).GetText());
+}
+return arguments;
+  }
+
+  void AddArguments(const Args ) {
+for (size_t i = 0; i < arguments.GetArgumentCount(); i++) {
+  AddNewField();
+  TextFieldDelegate  = GetField(GetNumberOfFields() - 1);
+  field.SetText(arguments.GetArgumentAtIndex(i));
+}
+  }
+};
+
 template 
 class MappingFieldDelegate : public FieldDelegate {
 public:
@@ -2068,14 +2126,36 @@
   const std::string () { return GetKeyField().GetName(); }
 
   const std::string () { return GetValueField().GetText(); }
+
+  void SetName(const char *name) { return GetKeyField().SetText(name); }
+
+  void SetValue(const char *value) { return GetValueField().SetText(value); }
 };
 
 class EnvironmentVariableListFieldDelegate
 : public ListFieldDelegate {
 public:
-  EnvironmentVariableListFieldDelegate()
-  : ListFieldDelegate("Environment Variables",
-  EnvironmentVariableFieldDelegate()) {}
+  

[Lldb-commits] [lldb] 82507f1 - [LLDB][GUI] Add Process Launch form

[Greg Clayton via lldb-commits]

Author: Omar Emara
Date: 2021-08-18T15:43:30-07:00
New Revision: 82507f1798768280cf5d5aab95caaafbc7fe6f47

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

LOG: [LLDB][GUI] Add Process Launch form

This patch adds a process launch form. Additionally, a LazyBoolean field
was implemented and numerous utility methods were added to various
fields to get the launch form working.

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

Added: 


Modified: 
lldb/include/lldb/Target/Target.h
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index ac8d002b09a12..fc9dd97515aa9 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -122,7 +122,16 @@ class TargetProperties : public Properties {
 
   void SetRunArguments(const Args );
 
+  // Get the whole environment including the platform inherited environment and
+  // the target specific environment, excluding the unset environment 
variables.
   Environment GetEnvironment() const;
+  // Get the platform inherited environment, excluding the unset environment
+  // variables.
+  Environment GetInheritedEnvironment() const;
+  // Get the target specific environment only, without the platform inherited
+  // environment.
+  Environment GetTargetEnvironment() const;
+  // Set the target specific environment.
   void SetEnvironment(Environment env);
 
   bool GetSkipPrologue() const;

diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index 6e68c1d757c09..791e5edde250e 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1259,6 +1259,14 @@ class TextFieldDelegate : public FieldDelegate {
 
   const std::string () { return m_content; }
 
+  void SetText(const char *text) {
+if (text == nullptr) {
+  m_content.clear();
+  return;
+}
+m_content = text;
+  }
+
 protected:
   std::string m_label;
   bool m_required;
@@ -1618,6 +1626,33 @@ class ProcessPluginFieldDelegate : public 
ChoicesFieldDelegate {
   }
 };
 
+class LazyBooleanFieldDelegate : public ChoicesFieldDelegate {
+public:
+  LazyBooleanFieldDelegate(const char *label, const char *calculate_label)
+  : ChoicesFieldDelegate(label, 3, GetPossibleOptions(calculate_label)) {}
+
+  static constexpr const char *kNo = "No";
+  static constexpr const char *kYes = "Yes";
+
+  std::vector GetPossibleOptions(const char *calculate_label) {
+std::vector options;
+options.push_back(calculate_label);
+options.push_back(kYes);
+options.push_back(kNo);
+return options;
+  }
+
+  LazyBool GetLazyBoolean() {
+std::string choice = GetChoiceContent();
+if (choice == kNo)
+  return eLazyBoolNo;
+else if (choice == kYes)
+  return eLazyBoolYes;
+else
+  return eLazyBoolCalculate;
+  }
+};
+
 template  class ListFieldDelegate : public FieldDelegate {
 public:
   ListFieldDelegate(const char *label, T default_field)
@@ -1908,6 +1943,29 @@ template  class ListFieldDelegate : public 
FieldDelegate {
   SelectionType m_selection_type;
 };
 
+class ArgumentsFieldDelegate : public ListFieldDelegate {
+public:
+  ArgumentsFieldDelegate()
+  : ListFieldDelegate("Arguments",
+  TextFieldDelegate("Argument", "", false)) {}
+
+  Args GetArguments() {
+Args arguments;
+for (int i = 0; i < GetNumberOfFields(); i++) {
+  arguments.AppendArgument(GetField(i).GetText());
+}
+return arguments;
+  }
+
+  void AddArguments(const Args ) {
+for (size_t i = 0; i < arguments.GetArgumentCount(); i++) {
+  AddNewField();
+  TextFieldDelegate  = GetField(GetNumberOfFields() - 1);
+  field.SetText(arguments.GetArgumentAtIndex(i));
+}
+  }
+};
+
 template 
 class MappingFieldDelegate : public FieldDelegate {
 public:
@@ -2068,14 +2126,36 @@ class EnvironmentVariableFieldDelegate
   const std::string () { return GetKeyField().GetName(); }
 
   const std::string () { return GetValueField().GetText(); }
+
+  void SetName(const char *name) { return GetKeyField().SetText(name); }
+
+  void SetValue(const char *value) { return GetValueField().SetText(value); }
 };
 
 class EnvironmentVariableListFieldDelegate
 : public ListFieldDelegate {
 public:
-  EnvironmentVariableListFieldDelegate()
-  : ListFieldDelegate("Environment Variables",
-  EnvironmentVariableFieldDelegate()) {}
+  EnvironmentVariableListFieldDelegate(const char *label)
+  : ListFieldDelegate(label, EnvironmentVariableFieldDelegate()) {}
+
+  Environment GetEnvironment() {
+Environment environment;
+for (int i = 0; i < GetNumberOfFields(); 

[Lldb-commits] [PATCH] D108335: [lldb] Move UnixSignals subclasses to lldbTarget

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: LLDB, clayborg, jingham, teemperor, JDevlieghere.
Herald added subscribers: atanasyan, jrtc27, mgorny, sdardis, emaste.
bulbazord requested review of this revision.
Herald added a project: LLDB.

I have created this change as a way to solicit feedback and foster
discussion.  My goal is to sever lldbTarget's dependence on
lldbPluginProcessUtility as a part of my larger goal of more cleanly
setting boundaries between lldb's core libraries and lldb's plugin
libraries.

For the purposes of concretely demonstrating what severing the
dependence would look like, I made the easiest change possible (moving
files). I don't think this is the right kind of change because
lldbTarget should ideally be platform-independent. I considered turning
UnixSignals into a Plugin, but to me this felt like overkill. I think
the best change may be somewhere else (possibly creating a new non-plugin
library?) but I'm not sure what that should look like.

Feedback is appreciated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108335

Files:
  lldb/include/lldb/Target/FreeBSDSignals.h
  lldb/include/lldb/Target/GDBRemoteSignals.h
  lldb/include/lldb/Target/LinuxSignals.h
  lldb/include/lldb/Target/MipsLinuxSignals.h
  lldb/include/lldb/Target/NetBSDSignals.h
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
  lldb/source/Plugins/Process/Utility/FreeBSDSignals.h
  lldb/source/Plugins/Process/Utility/GDBRemoteSignals.cpp
  lldb/source/Plugins/Process/Utility/GDBRemoteSignals.h
  lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
  lldb/source/Plugins/Process/Utility/LinuxSignals.h
  lldb/source/Plugins/Process/Utility/MipsLinuxSignals.cpp
  lldb/source/Plugins/Process/Utility/MipsLinuxSignals.h
  lldb/source/Plugins/Process/Utility/NetBSDSignals.cpp
  lldb/source/Plugins/Process/Utility/NetBSDSignals.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/FreeBSDSignals.cpp
  lldb/source/Target/GDBRemoteSignals.cpp
  lldb/source/Target/LinuxSignals.cpp
  lldb/source/Target/MipsLinuxSignals.cpp
  lldb/source/Target/NetBSDSignals.cpp
  lldb/source/Target/UnixSignals.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -9,9 +9,9 @@
 
 #include "GDBRemoteTestUtils.h"
 
-#include "Plugins/Process/Utility/LinuxSignals.h"
 #include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h"
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Target/LinuxSignals.h"
 #include "lldb/Utility/GDBRemote.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Testing/Support/Error.h"
Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -7,12 +7,12 @@
 //===--===//
 
 #include "lldb/Target/UnixSignals.h"
-#include "Plugins/Process/Utility/FreeBSDSignals.h"
-#include "Plugins/Process/Utility/LinuxSignals.h"
-#include "Plugins/Process/Utility/MipsLinuxSignals.h"
-#include "Plugins/Process/Utility/NetBSDSignals.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/StringConvert.h"
+#include "lldb/Target/FreeBSDSignals.h"
+#include "lldb/Target/LinuxSignals.h"
+#include "lldb/Target/MipsLinuxSignals.h"
+#include "lldb/Target/NetBSDSignals.h"
 #include "lldb/Utility/ArchSpec.h"
 
 using namespace lldb_private;
Index: lldb/source/Target/NetBSDSignals.cpp
===
--- lldb/source/Target/NetBSDSignals.cpp
+++ lldb/source/Target/NetBSDSignals.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "NetBSDSignals.h"
+#include "lldb/Target/NetBSDSignals.h"
 
 using namespace lldb_private;
 
Index: lldb/source/Target/MipsLinuxSignals.cpp
===
--- lldb/source/Target/MipsLinuxSignals.cpp
+++ lldb/source/Target/MipsLinuxSignals.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "MipsLinuxSignals.h"
+#include "lldb/Target/MipsLinuxSignals.h"
 
 using namespace lldb_private;
 
Index: lldb/source/Target/LinuxSignals.cpp
===
--- 

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Work well now on macOS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

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

[Lldb-commits] [PATCH] D108327: [LLDB][GUI] Fix text field incorrect key handling

[Greg Clayton 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 rG698e2106362a: [LLDB][GUI] Fix text field incorrect key 
handling (authored by OmarEmaraDev, committed by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108327

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1208,7 +1208,13 @@
 
   // True if the key represents a char that can be inserted in the field
   // content, false otherwise.
-  virtual bool IsAcceptableChar(int key) { return isprint(key); }
+  virtual bool IsAcceptableChar(int key) {
+// The behavior of isprint is undefined when the value is not representable
+// as an unsigned char. So explicitly check for non-ascii key codes.
+if (key > 127)
+  return false;
+return isprint(key);
+  }
 
   HandleCharResult FieldDelegateHandleChar(int key) override {
 if (IsAcceptableChar(key)) {


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1208,7 +1208,13 @@
 
   // True if the key represents a char that can be inserted in the field
   // content, false otherwise.
-  virtual bool IsAcceptableChar(int key) { return isprint(key); }
+  virtual bool IsAcceptableChar(int key) {
+// The behavior of isprint is undefined when the value is not representable
+// as an unsigned char. So explicitly check for non-ascii key codes.
+if (key > 127)
+  return false;
+return isprint(key);
+  }
 
   HandleCharResult FieldDelegateHandleChar(int key) override {
 if (IsAcceptableChar(key)) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 698e210 - [LLDB][GUI] Fix text field incorrect key handling

[Greg Clayton via lldb-commits]

Author: Omar Emara
Date: 2021-08-18T15:06:27-07:00
New Revision: 698e2106362add2bd3aca6f8ffebe9dd90a50529

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

LOG: [LLDB][GUI] Fix text field incorrect key handling

The isprint libc function was used to determine if the key code
represents a printable character. The problem is that the specification
leaves the behavior undefined if the key is not representable as an
unsigned char, which is the case for many ncurses keys. This patch adds
and explicit check for this undefined behavior and make it consistent.

The llvm::isPrint function didn't work correctly for some reason, most
likely because it takes a char instead of an int, which I guess makes it
unsuitable for checking ncurses key codes.

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/source/Core/IOHandlerCursesGUI.cpp

Removed: 




diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index 0ae41dee7f9d..6e68c1d757c0 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1208,7 +1208,13 @@ class TextFieldDelegate : public FieldDelegate {
 
   // True if the key represents a char that can be inserted in the field
   // content, false otherwise.
-  virtual bool IsAcceptableChar(int key) { return isprint(key); }
+  virtual bool IsAcceptableChar(int key) {
+// The behavior of isprint is undefined when the value is not representable
+// as an unsigned char. So explicitly check for non-ascii key codes.
+if (key > 127)
+  return false;
+return isprint(key);
+  }
 
   HandleCharResult FieldDelegateHandleChar(int key) override {
 if (IsAcceptableChar(key)) {



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

[Lldb-commits] [PATCH] D108327: [LLDB][GUI] Fix text field incorrect key handling

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

This works much better now!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108327

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

[Lldb-commits] [PATCH] D108228: Fix error handling in the libcxx std::string formatter

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp:102
+std::string *not_a_string = (std::string *) 0x0;
+touch_string(*not_a_string);
 return 0;

teemperor wrote:
> dblaikie wrote:
> > JDevlieghere wrote:
> > > shafik wrote:
> > > > This is undefined behavior and I AFAICT this won't pass a sanitized 
> > > > build, amazingly even if I use `__attribute__((no_sanitize("address", 
> > > > "undefined")))` : https://godbolt.org/z/4TGPbrYhq
> > > Definitely UB, but the sanitized bot builds LLDB with the sanitizers, not 
> > > the test cases, so this should be "fine". 
> > Seems best avoided if possible though, yeah? What's trying to be 
> > demonstrated by this test?
> > 
> > What if the function took a std::string* instead of std::string&, and the 
> > caller doesn't need to dereference that pointer - it could call some other, 
> > unrelated function to act as a stop-point for the debugger?
> > 
> > & then the "printing a bad string" Could be tested by printing an 
> > expression, like "p *str" that dereferences in the expression?
> > 
> > Or is the issue only present through the auto-printing of variables in 
> > parameters in a stack trace, and not present when using the user-defined 
> > expression evaluator?
> > This is undefined behavior and I AFAICT this won't pass a sanitized build, 
> > amazingly even if I use __attribute__((no_sanitize("address", 
> > "undefined"))) : https://godbolt.org/z/4TGPbrYhq
> 
> This is just a segfault unrelated to sanitizers.
> 
> > & then the "printing a bad string" Could be tested by printing an 
> > expression, like "p *str" that dereferences in the expression?
> > Or is the issue only present through the auto-printing of variables in 
> > parameters in a stack trace, and not present when using the user-defined 
> > expression evaluator?
> 
> The expression evaluator is already erroring out when printing that variable 
> (or doing the `*str` equivalent) so I believe this is just about directly 
> accessing the variables. The same goes for situations like `frame var *str` 
> which already handle the error that we now also check for here. I *believe* 
> we really need a the null reference as in the current test so that we end up 
> in a situation where the formatter has to handle the "parent is null" error 
> itself in this code path because of a null reference.
Ah, that's a pity. Oh well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108228

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

[Lldb-commits] [PATCH] D108331: [LLDB][GUI] Handle return key for compound fields

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev created this revision.
OmarEmaraDev added a reviewer: clayborg.
Herald added a reviewer: teemperor.
OmarEmaraDev requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch handles the return key for compound fields like lists and
mapping fields. The return key, if not handled by the field will select
the next primary element, skipping secondary elements like remove
buttons and the like.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108331

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1816,6 +1816,31 @@
 return eKeyHandled;
   }
 
+  // If the last element of the field is selected and it didn't handle the key.
+  // Select the next field or new button if the selected field is the last one.
+  HandleCharResult SelectNextInList(int key) {
+assert(m_selection_type == SelectionType::Field);
+
+FieldDelegate  = m_fields[m_selection_index];
+if (field.FieldDelegateHandleChar(key) == eKeyHandled)
+  return eKeyHandled;
+
+if (!field.FieldDelegateOnLastOrOnlyElement())
+  return eKeyNotHandled;
+
+field.FieldDelegateExitCallback();
+
+if (m_selection_index == GetNumberOfFields() - 1) {
+  m_selection_type = SelectionType::NewButton;
+  return eKeyHandled;
+}
+
+m_selection_index++;
+FieldDelegate _field = m_fields[m_selection_index];
+next_field.FieldDelegateSelectFirstElement();
+return eKeyHandled;
+  }
+
   HandleCharResult FieldDelegateHandleChar(int key) override {
 switch (key) {
 case '\r':
@@ -1828,16 +1853,14 @@
   case SelectionType::RemoveButton:
 RemoveField();
 return eKeyHandled;
-  default:
-break;
+  case SelectionType::Field:
+return SelectNextInList(key);
   }
   break;
 case '\t':
-  SelectNext(key);
-  return eKeyHandled;
+  return SelectNext(key);
 case KEY_SHIFT_TAB:
-  SelectPrevious(key);
-  return eKeyHandled;
+  return SelectPrevious(key);
 default:
   break;
 }
@@ -1984,14 +2007,34 @@
 return eKeyHandled;
   }
 
+  // If the value field is selected, pass the key to it. If the key field is
+  // selected, its last element is selected, and it didn't handle the key, then
+  // select its corresponding value field.
+  HandleCharResult SelectNextField(int key) {
+if (m_selection_type == SelectionType::Value) {
+  return m_value_field.FieldDelegateHandleChar(key);
+}
+
+if (m_key_field.FieldDelegateHandleChar(key) == eKeyHandled)
+  return eKeyHandled;
+
+if (!m_key_field.FieldDelegateOnLastOrOnlyElement())
+  return eKeyNotHandled;
+
+m_key_field.FieldDelegateExitCallback();
+m_selection_type = SelectionType::Value;
+m_value_field.FieldDelegateSelectFirstElement();
+return eKeyHandled;
+  }
+
   HandleCharResult FieldDelegateHandleChar(int key) override {
 switch (key) {
+case KEY_RETURN:
+  return SelectNextField(key);
 case '\t':
-  SelectNext(key);
-  return eKeyHandled;
+  return SelectNext(key);
 case KEY_SHIFT_TAB:
-  SelectPrevious(key);
-  return eKeyHandled;
+  return SelectPrevious(key);
 default:
   break;
 }


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1816,6 +1816,31 @@
 return eKeyHandled;
   }
 
+  // If the last element of the field is selected and it didn't handle the key.
+  // Select the next field or new button if the selected field is the last one.
+  HandleCharResult SelectNextInList(int key) {
+assert(m_selection_type == SelectionType::Field);
+
+FieldDelegate  = m_fields[m_selection_index];
+if (field.FieldDelegateHandleChar(key) == eKeyHandled)
+  return eKeyHandled;
+
+if (!field.FieldDelegateOnLastOrOnlyElement())
+  return eKeyNotHandled;
+
+field.FieldDelegateExitCallback();
+
+if (m_selection_index == GetNumberOfFields() - 1) {
+  m_selection_type = SelectionType::NewButton;
+  return eKeyHandled;
+}
+
+m_selection_index++;
+FieldDelegate _field = m_fields[m_selection_index];
+next_field.FieldDelegateSelectFirstElement();
+return eKeyHandled;
+  }
+
   HandleCharResult FieldDelegateHandleChar(int key) override {
 switch (key) {
 case '\r':
@@ -1828,16 +1853,14 @@
   case SelectionType::RemoveButton:
 RemoveField();
 return eKeyHandled;
-  default:
-break;
+  case SelectionType::Field:
+return SelectNextInList(key);
   }
   break;
 case '\t':
-  SelectNext(key);
-  return eKeyHandled;
+  return 

[Lldb-commits] [PATCH] D108327: [LLDB][GUI] Fix text field incorrect key handling

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev created this revision.
OmarEmaraDev added a reviewer: clayborg.
Herald added a reviewer: teemperor.
OmarEmaraDev requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The isprint libc function was used to determine if the key code
represents a printable character. The problem is that the specification
leaves the behavior undefined if the key is not representable as an
unsigned char, which is the case for many ncurses keys. This patch adds
and explicit check for this undefined behavior and make it consistent.

The llvm::isPrint function didn't work correctly for some reason, most
likely because it takes a char instead of an int, which I guess makes it
unsuitable for checking ncurses key codes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108327

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1208,7 +1208,13 @@
 
   // True if the key represents a char that can be inserted in the field
   // content, false otherwise.
-  virtual bool IsAcceptableChar(int key) { return isprint(key); }
+  virtual bool IsAcceptableChar(int key) {
+// The behavior of isprint is undefined when the value is not representable
+// as an unsigned char. So explicitly check for non-ascii key codes.
+if (key > 127)
+  return false;
+return isprint(key);
+  }
 
   HandleCharResult FieldDelegateHandleChar(int key) override {
 if (IsAcceptableChar(key)) {


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1208,7 +1208,13 @@
 
   // True if the key represents a char that can be inserted in the field
   // content, false otherwise.
-  virtual bool IsAcceptableChar(int key) { return isprint(key); }
+  virtual bool IsAcceptableChar(int key) {
+// The behavior of isprint is undefined when the value is not representable
+// as an unsigned char. So explicitly check for non-ascii key codes.
+if (key > 127)
+  return false;
+return isprint(key);
+  }
 
   HandleCharResult FieldDelegateHandleChar(int key) override {
 if (IsAcceptableChar(key)) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

[Rumeet Dhindsa via Phabricator via lldb-commits]

rdhindsa updated this revision to Diff 367311.
rdhindsa added a comment.

Updated comments as suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105732

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


Index: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
===
--- lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/Support/Errno.h"
+#include "llvm/Support/FileSystem.h"
 
 #include 
 #include 
@@ -143,9 +144,32 @@
 // Close everything besides stdin, stdout, and stderr that has no file
 // action to avoid leaking. Only do this when debugging, as elsewhere we
 // actually rely on passing open descriptors to child processes.
-for (int fd = 3; fd < sysconf(_SC_OPEN_MAX); ++fd)
-  if (!info.GetFileActionForFD(fd) && fd != error_fd)
-close(fd);
+
+const llvm::StringRef proc_fd_path = "/proc/self/fd";
+std::error_code ec;
+bool result;
+ec = llvm::sys::fs::is_directory(proc_fd_path, result);
+if (result) {
+  std::vector files_to_close;
+  // Directory iterator doesn't ensure any sequence.
+  for (llvm::sys::fs::directory_iterator iter(proc_fd_path, ec), file_end;
+   iter != file_end && !ec; iter.increment(ec)) {
+int fd = std::stoi(iter->path().substr(proc_fd_path.size() + 1));
+
+// Don't close first three entries since they are stdin, stdout and
+// stderr.
+if (fd > 2 && !info.GetFileActionForFD(fd) && fd != error_fd)
+  files_to_close.push_back(fd);
+  }
+  for (int file_to_close : files_to_close)
+close(file_to_close);
+} else {
+  // Since /proc/self/fd didn't work, trying the slow way instead.
+  int max_fd = sysconf(_SC_OPEN_MAX);
+  for (int fd = 3; fd < max_fd; ++fd)
+if (!info.GetFileActionForFD(fd) && fd != error_fd)
+  close(fd);
+}
 
 // Start tracing this child that is about to exec.
 if (ptrace(PT_TRACE_ME, 0, nullptr, 0) == -1)


Index: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
===
--- lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/Support/Errno.h"
+#include "llvm/Support/FileSystem.h"
 
 #include 
 #include 
@@ -143,9 +144,32 @@
 // Close everything besides stdin, stdout, and stderr that has no file
 // action to avoid leaking. Only do this when debugging, as elsewhere we
 // actually rely on passing open descriptors to child processes.
-for (int fd = 3; fd < sysconf(_SC_OPEN_MAX); ++fd)
-  if (!info.GetFileActionForFD(fd) && fd != error_fd)
-close(fd);
+
+const llvm::StringRef proc_fd_path = "/proc/self/fd";
+std::error_code ec;
+bool result;
+ec = llvm::sys::fs::is_directory(proc_fd_path, result);
+if (result) {
+  std::vector files_to_close;
+  // Directory iterator doesn't ensure any sequence.
+  for (llvm::sys::fs::directory_iterator iter(proc_fd_path, ec), file_end;
+   iter != file_end && !ec; iter.increment(ec)) {
+int fd = std::stoi(iter->path().substr(proc_fd_path.size() + 1));
+
+// Don't close first three entries since they are stdin, stdout and
+// stderr.
+if (fd > 2 && !info.GetFileActionForFD(fd) && fd != error_fd)
+  files_to_close.push_back(fd);
+  }
+  for (int file_to_close : files_to_close)
+close(file_to_close);
+} else {
+  // Since /proc/self/fd didn't work, trying the slow way instead.
+  int max_fd = sysconf(_SC_OPEN_MAX);
+  for (int fd = 3; fd < max_fd; ++fd)
+if (!info.GetFileActionForFD(fd) && fd != error_fd)
+  close(fd);
+}
 
 // Start tracing this child that is about to exec.
 if (ptrace(PT_TRACE_ME, 0, nullptr, 0) == -1)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

Small nit about the comments, but otherwise LGTM




Comment at: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp:160
+// Don't close first three entries since they are
+// stdin/stdout/stderr
+if (fd > 2 && !info.GetFileActionForFD(fd) && fd != error_fd)

nit: Comments should be sentences ending with a period.



Comment at: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp:167
+} else {
+  // /proc/self/fd didn't work - trying the slow way instead
+  int max_fd = sysconf(_SC_OPEN_MAX);

nit: Same as above. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105732

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

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Rumeet Dhindsa via Phabricator via lldb-commits]

rdhindsa added a comment.

I checked the status with both gdb and lldb using test case added here. It 
looks like breakpoint on main doesn't get hit even for gdb when called through 
ld.so. However, breakpoint on shared library functions is supported by gdb. 
With this patch, we are now able to set breakpoint on functions in shared 
libraries as supported by gdb.

  bin/lldb -- /lib64/ld-linux-x86-64.so.2 --library-path $PWD ./a.out
  (lldb) target create "/lib64/ld-linux-x86-64.so.2"
   Current executable set to '/lib64/ld-linux-x86-64.so.2' (x86_64).
  (lldb) settings set -- target.run-args  "--library-path" "" "./a.out"
  (lldb) b main
  Breakpoint 1: no locations (pending).
  WARNING:  Unable to resolve breakpoint to any actual locations.
  (lldb) b get_signal_crash
  Breakpoint 2: no locations (pending).
  WARNING:  Unable to resolve breakpoint to any actual locations.
  (lldb) run
  Process 526132 launched: '/lib64/ld-linux-x86-64.so.2' (x86_64)
  1 location added to breakpoint 2
  Process 526132 stopped
  * thread #1, name = 'ld-linux-x86-64', stop reason = breakpoint 2.1
  frame #0: 0x77fa1110 libsignal_file.so`get_signal_crash()
  
  gdb --args /lib64/ld-linux-x86-64.so.2 --library-path $PWD ./a.out
  (gdb) b main
  Function "main" not defined.
  Make breakpoint pending on future shared library load? (y or [n]) y
  Breakpoint 1 (main) pending.
  (gdb) b get_signal_crash
  Function "get_signal_crash" not defined.
  Make breakpoint pending on future shared library load? (y or [n]) y
  Breakpoint 2 (get_signal_crash) pending.
  (gdb) run
  Starting program: /lib64/ld-linux-x86-64.so.2 --library-path .. ./a.out
  Breakpoint 2, 0x77fa1114 in get_signal_crash()

This patch adds the support for shared library load. Do you think if it is okay 
if the functionality to be able to breakpoint on the main executable is added 
as next step?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

[Rumeet Dhindsa via Phabricator via lldb-commits]

rdhindsa updated this revision to Diff 367286.
rdhindsa marked 7 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105732

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


Index: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
===
--- lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/Support/Errno.h"
+#include "llvm/Support/FileSystem.h"
 
 #include 
 #include 
@@ -143,9 +144,32 @@
 // Close everything besides stdin, stdout, and stderr that has no file
 // action to avoid leaking. Only do this when debugging, as elsewhere we
 // actually rely on passing open descriptors to child processes.
-for (int fd = 3; fd < sysconf(_SC_OPEN_MAX); ++fd)
-  if (!info.GetFileActionForFD(fd) && fd != error_fd)
-close(fd);
+
+const llvm::StringRef proc_fd_path = "/proc/self/fd";
+std::error_code ec;
+bool result;
+ec = llvm::sys::fs::is_directory(proc_fd_path, result);
+if (result) {
+  std::vector files_to_close;
+  // Directory iterator doesn't ensure any sequence.
+  for (llvm::sys::fs::directory_iterator iter(proc_fd_path, ec), file_end;
+   iter != file_end && !ec; iter.increment(ec)) {
+int fd = std::stoi(iter->path().substr(proc_fd_path.size() + 1));
+
+// Don't close first three entries since they are
+// stdin/stdout/stderr
+if (fd > 2 && !info.GetFileActionForFD(fd) && fd != error_fd)
+  files_to_close.push_back(fd);
+  }
+  for (int file_to_close : files_to_close)
+close(file_to_close);
+} else {
+  // /proc/self/fd didn't work - trying the slow way instead
+  int max_fd = sysconf(_SC_OPEN_MAX);
+  for (int fd = 3; fd < max_fd; ++fd)
+if (!info.GetFileActionForFD(fd) && fd != error_fd)
+  close(fd);
+}
 
 // Start tracing this child that is about to exec.
 if (ptrace(PT_TRACE_ME, 0, nullptr, 0) == -1)


Index: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
===
--- lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/Support/Errno.h"
+#include "llvm/Support/FileSystem.h"
 
 #include 
 #include 
@@ -143,9 +144,32 @@
 // Close everything besides stdin, stdout, and stderr that has no file
 // action to avoid leaking. Only do this when debugging, as elsewhere we
 // actually rely on passing open descriptors to child processes.
-for (int fd = 3; fd < sysconf(_SC_OPEN_MAX); ++fd)
-  if (!info.GetFileActionForFD(fd) && fd != error_fd)
-close(fd);
+
+const llvm::StringRef proc_fd_path = "/proc/self/fd";
+std::error_code ec;
+bool result;
+ec = llvm::sys::fs::is_directory(proc_fd_path, result);
+if (result) {
+  std::vector files_to_close;
+  // Directory iterator doesn't ensure any sequence.
+  for (llvm::sys::fs::directory_iterator iter(proc_fd_path, ec), file_end;
+   iter != file_end && !ec; iter.increment(ec)) {
+int fd = std::stoi(iter->path().substr(proc_fd_path.size() + 1));
+
+// Don't close first three entries since they are
+// stdin/stdout/stderr
+if (fd > 2 && !info.GetFileActionForFD(fd) && fd != error_fd)
+  files_to_close.push_back(fd);
+  }
+  for (int file_to_close : files_to_close)
+close(file_to_close);
+} else {
+  // /proc/self/fd didn't work - trying the slow way instead
+  int max_fd = sysconf(_SC_OPEN_MAX);
+  for (int fd = 3; fd < max_fd; ++fd)
+if (!info.GetFileActionForFD(fd) && fd != error_fd)
+  close(fd);
+}
 
 // Start tracing this child that is about to exec.
 if (ptrace(PT_TRACE_ME, 0, nullptr, 0) == -1)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 367244.
bulbazord added a comment.

Delete now unused variable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108229

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Target/Language.h
  lldb/source/Core/CMakeLists.txt
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h

Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -104,6 +104,9 @@
   std::vector
   GetMethodNameVariants(ConstString method_name) const override;
 
+  Language::FunctionNameInfo
+  GetFunctionNameInfo(ConstString name) const override;
+
   bool SymbolNameFitsToLanguage(Mangled mangled) const override;
 
   lldb::TypeCategoryImplSP GetFormatters() override;
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -275,6 +275,22 @@
   return variant_names;
 }
 
+Language::FunctionNameInfo
+ObjCLanguage::GetFunctionNameInfo(ConstString name) const {
+  Language::FunctionNameInfo info;
+  info.func_name_type = lldb::eFunctionNameTypeNone;
+
+  if (ObjCLanguage::IsPossibleObjCMethodName(name.GetCString())) {
+info.func_name_type = lldb::eFunctionNameTypeFull;
+  }
+
+  if (ObjCLanguage::IsPossibleObjCSelector(name.GetCString())) {
+info.func_name_type |= lldb::eFunctionNameTypeSelector;
+  }
+
+  return info;
+}
+
 bool ObjCLanguage::SymbolNameFitsToLanguage(Mangled mangled) const {
   ConstString demangled_name = mangled.GetDemangledName();
   if (!demangled_name)
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -104,6 +104,9 @@
 
   static lldb_private::ConstString GetPluginNameStatic();
 
+  Language::FunctionNameInfo
+  GetFunctionNameInfo(ConstString name) const override;
+
   bool SymbolNameFitsToLanguage(Mangled mangled) const override;
 
   ConstString
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -59,6 +59,39 @@
   return g_name;
 }
 
+Language::FunctionNameInfo
+CPlusPlusLanguage::GetFunctionNameInfo(ConstString name) const {
+  FunctionNameInfo info;
+  info.func_name_type = lldb::eFunctionNameTypeNone;
+
+  if (IsCPPMangledName(name.GetCString())) {
+info.func_name_type = lldb::eFunctionNameTypeFull;
+  }
+
+  CPlusPlusLanguage::MethodName method(name);
+  llvm::StringRef basename = method.GetBasename();
+  if (basename.empty()) {
+if (CPlusPlusLanguage::ExtractContextAndIdentifier(
+name.GetCString(), info.context, info.basename)) {
+  info.func_name_type |=
+  (lldb::eFunctionNameTypeMethod | eFunctionNameTypeBase);
+} else {
+  info.func_name_type |= lldb::eFunctionNameTypeFull;
+}
+  } else {
+info.func_name_type |=
+(lldb::eFunctionNameTypeMethod | eFunctionNameTypeBase);
+  }
+
+  if (!method.GetQualifiers().empty()) {
+// There is a 'const' or other qualifier following the end of the function
+// parens, this can't be a eFunctionNameTypeBase.
+info.func_name_type &= ~(lldb::eFunctionNameTypeBase);
+  }
+
+  return info;
+}
+
 bool CPlusPlusLanguage::SymbolNameFitsToLanguage(Mangled mangled) const {
   const char *mangled_name = mangled.GetMangledName().GetCString();
   return mangled_name && CPlusPlusLanguage::IsCPPMangledName(mangled_name);
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -51,7 +51,6 @@
 #endif
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
@@ -635,101 +634,81 @@
 
 Module::LookupInfo::LookupInfo(ConstString name,
FunctionNameType name_type_mask,
-   LanguageType language)
-: m_name(name), m_lookup_name(), m_language(language),
+   LanguageType language_type)
+: m_name(name), m_lookup_name(name), m_language_type(language_type),
   

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D104413#2952057 , @PatriosTheGreat 
wrote:

> Should I send this somehow back to review?

Nope, this is (as it should be) in the review queue but I just didn't get 
around to this yet. Sorry for the delay, I'll try to take a look at this today. 
Feel free to ping sooner in the future, LLVM policy says a week delay justifies 
a ping :)


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

https://reviews.llvm.org/D104413

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

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Should I send this somehow back to review?


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

https://reviews.llvm.org/D104413

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

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev updated this revision to Diff 367171.
OmarEmaraDev added a comment.

- Address review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107869

Files:
  lldb/include/lldb/Target/Target.h
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3985,6 +3985,45 @@
   return ComputeEnvironment();
 }
 
+Environment TargetProperties::GetInheritedEnvironment() const {
+  Environment environment;
+
+  if (m_target == nullptr)
+return environment;
+
+  if (!m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, ePropertyInheritEnv,
+  g_target_properties[ePropertyInheritEnv].default_uint_value != 0))
+return environment;
+
+  PlatformSP platform_sp = m_target->GetPlatform();
+  if (platform_sp == nullptr)
+return environment;
+
+  Environment platform_environment = platform_sp->GetEnvironment();
+  for (const auto  : platform_environment)
+environment[KV.first()] = KV.second;
+
+  Args property_unset_environment;
+  m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyUnsetEnvVars,
+property_unset_environment);
+  for (const auto  : property_unset_environment)
+environment.erase(var.ref());
+
+  return environment;
+}
+
+Environment TargetProperties::GetTargetEnvironment() const {
+  Args property_environment;
+  m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyEnvVars,
+property_environment);
+  Environment environment;
+  for (const auto  : Environment(property_environment))
+environment[KV.first()] = KV.second;
+
+  return environment;
+}
+
 void TargetProperties::SetEnvironment(Environment env) {
   // TODO: Get rid of the Args intermediate step
   const uint32_t idx = ePropertyEnvVars;
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1260,6 +1260,14 @@
 
   const std::string () { return m_content; }
 
+  void SetText(const char *text) {
+if (text == nullptr) {
+  m_content.clear();
+  return;
+}
+m_content = text;
+  }
+
 protected:
   std::string m_label;
   bool m_required;
@@ -1619,6 +1627,33 @@
   }
 };
 
+class LazyBooleanFieldDelegate : public ChoicesFieldDelegate {
+public:
+  LazyBooleanFieldDelegate(const char *label, const char *calculate_label)
+  : ChoicesFieldDelegate(label, 3, GetPossibleOptions(calculate_label)) {}
+
+  static constexpr const char *kNo = "No";
+  static constexpr const char *kYes = "Yes";
+
+  std::vector GetPossibleOptions(const char *calculate_label) {
+std::vector options;
+options.push_back(calculate_label);
+options.push_back(kYes);
+options.push_back(kNo);
+return options;
+  }
+
+  LazyBool GetLazyBoolean() {
+std::string choice = GetChoiceContent();
+if (choice == kNo)
+  return eLazyBoolNo;
+else if (choice == kYes)
+  return eLazyBoolYes;
+else
+  return eLazyBoolCalculate;
+  }
+};
+
 template  class ListFieldDelegate : public FieldDelegate {
 public:
   ListFieldDelegate(const char *label, T default_field)
@@ -1909,6 +1944,29 @@
   SelectionType m_selection_type;
 };
 
+class ArgumentsFieldDelegate : public ListFieldDelegate {
+public:
+  ArgumentsFieldDelegate()
+  : ListFieldDelegate("Arguments",
+  TextFieldDelegate("Argument", "", false)) {}
+
+  Args GetArguments() {
+Args arguments;
+for (int i = 0; i < GetNumberOfFields(); i++) {
+  arguments.AppendArgument(GetField(i).GetText());
+}
+return arguments;
+  }
+
+  void AddArguments(const Args ) {
+for (size_t i = 0; i < arguments.GetArgumentCount(); i++) {
+  AddNewField();
+  TextFieldDelegate  = GetField(GetNumberOfFields() - 1);
+  field.SetText(arguments.GetArgumentAtIndex(i));
+}
+  }
+};
+
 template 
 class MappingFieldDelegate : public FieldDelegate {
 public:
@@ -2069,14 +2127,36 @@
   const std::string () { return GetKeyField().GetName(); }
 
   const std::string () { return GetValueField().GetText(); }
+
+  void SetName(const char *name) { return GetKeyField().SetText(name); }
+
+  void SetValue(const char *value) { return GetValueField().SetText(value); }
 };
 
 class EnvironmentVariableListFieldDelegate
 : public ListFieldDelegate {
 public:
-  EnvironmentVariableListFieldDelegate()
-  : ListFieldDelegate("Environment Variables",
-  EnvironmentVariableFieldDelegate()) {}
+  EnvironmentVariableListFieldDelegate(const char *label)
+  : ListFieldDelegate(label, EnvironmentVariableFieldDelegate()) {}
+
+  Environment GetEnvironment() {
+

[Lldb-commits] [PATCH] D108228: Fix error handling in the libcxx std::string formatter

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

You could land all the formatting changes as its own commit just to make it 
clear that this is adding one `if (error)` and the other line changes are 
formatting updates. But I don't have a problem with keeping the formatting 
changes in this commit.

Could we minimize the test changes to:

  std::string _str_ref = *null_str; // (null_str is already a variable in 
the test).

  # No summary for null reference
  self.expect_var_path("null_str_ref", summary='"Summary Unavailable"')

There is no `expect_var` yet, so we have to stick to the expr/var path variant 
at the moment. But this would avoid all the breakpoint setting, continuing, 
variable fetching and manual error handling (it would also print the error when 
fetching the variable if we run into one),

Beside that this LGTM.




Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py:134
+var = frame.FindVariable("in_str")
+self.assertTrue(var.GetError().Success(), "Made variable")
+summary = var.GetSummary()

Side note as this could all be replaced with the one line check below, but 
Pavel added is `assertSuccess` that checks the same thing but also prints out 
the actual error message.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp:102
+std::string *not_a_string = (std::string *) 0x0;
+touch_string(*not_a_string);
 return 0;

dblaikie wrote:
> JDevlieghere wrote:
> > shafik wrote:
> > > This is undefined behavior and I AFAICT this won't pass a sanitized 
> > > build, amazingly even if I use `__attribute__((no_sanitize("address", 
> > > "undefined")))` : https://godbolt.org/z/4TGPbrYhq
> > Definitely UB, but the sanitized bot builds LLDB with the sanitizers, not 
> > the test cases, so this should be "fine". 
> Seems best avoided if possible though, yeah? What's trying to be demonstrated 
> by this test?
> 
> What if the function took a std::string* instead of std::string&, and the 
> caller doesn't need to dereference that pointer - it could call some other, 
> unrelated function to act as a stop-point for the debugger?
> 
> & then the "printing a bad string" Could be tested by printing an expression, 
> like "p *str" that dereferences in the expression?
> 
> Or is the issue only present through the auto-printing of variables in 
> parameters in a stack trace, and not present when using the user-defined 
> expression evaluator?
> This is undefined behavior and I AFAICT this won't pass a sanitized build, 
> amazingly even if I use __attribute__((no_sanitize("address", "undefined"))) 
> : https://godbolt.org/z/4TGPbrYhq

This is just a segfault unrelated to sanitizers.

> & then the "printing a bad string" Could be tested by printing an expression, 
> like "p *str" that dereferences in the expression?
> Or is the issue only present through the auto-printing of variables in 
> parameters in a stack trace, and not present when using the user-defined 
> expression evaluator?

The expression evaluator is already erroring out when printing that variable 
(or doing the `*str` equivalent) so I believe this is just about directly 
accessing the variables. The same goes for situations like `frame var *str` 
which already handle the error that we now also check for here. I *believe* we 
really need a the null reference as in the current test so that we end up in a 
situation where the formatter has to handle the "parent is null" error itself 
in this code path because of a null reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108228

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

[Lldb-commits] [PATCH] D108228: Fix error handling in the libcxx std::string formatter

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp:101
 S.assign(L"!"); // Set break point at this line.
+std::string *not_a_string = (std::string *) 0x0;
+touch_string(*not_a_string);

This cast isn't needed, right? This could be rewritten more traditionally as:
```
std::string _a_string = nullptr;
```
?



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp:102
+std::string *not_a_string = (std::string *) 0x0;
+touch_string(*not_a_string);
 return 0;

JDevlieghere wrote:
> shafik wrote:
> > This is undefined behavior and I AFAICT this won't pass a sanitized build, 
> > amazingly even if I use `__attribute__((no_sanitize("address", 
> > "undefined")))` : https://godbolt.org/z/4TGPbrYhq
> Definitely UB, but the sanitized bot builds LLDB with the sanitizers, not the 
> test cases, so this should be "fine". 
Seems best avoided if possible though, yeah? What's trying to be demonstrated 
by this test?

What if the function took a std::string* instead of std::string&, and the 
caller doesn't need to dereference that pointer - it could call some other, 
unrelated function to act as a stop-point for the debugger?

& then the "printing a bad string" Could be tested by printing an expression, 
like "p *str" that dereferences in the expression?

Or is the issue only present through the auto-printing of variables in 
parameters in a stack trace, and not present when using the user-defined 
expression evaluator?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108228

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