[Lldb-commits] [PATCH] D36046: Improve the posix core file triple detection

2017-07-29 Thread Tamas Berghammer via Phabricator via lldb-commits
tberghammer created this revision.
Herald added subscribers: kristof.beyls, arichardson, sdardis, aemerson.

Posix core files sometime don't contain enough information to correctly
detect the OS. If that is the case we should use the OS from the target
instead as it will contain usable information in more cases and if the
target and the core contain different OS-es then we are already in a
pretty bad state so moving from an unknown OS to a known (but possibly
incorrect) OS will do no harm.

We already had similar code in place for MIPS. This change tries to make
it more generic by using ArchSpec::MergeFrom and extends it to all
architectures but some MIPS specific issue prevent us from getting rid
of special casing MIPS.


https://reviews.llvm.org/D36046

Files:
  source/Core/ArchSpec.cpp
  source/Plugins/Process/elf-core/ProcessElfCore.cpp


Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -724,15 +724,15 @@
 }
 
 ArchSpec ProcessElfCore::GetArchitecture() {
-  ObjectFileELF *core_file =
-  (ObjectFileELF *)(m_core_module_sp->GetObjectFile());
   ArchSpec arch;
-  core_file->GetArchitecture(arch);
+  m_core_module_sp->GetObjectFile()->GetArchitecture(arch);
 
   ArchSpec target_arch = GetTarget().GetArchitecture();
-  
-  if (target_arch.IsMIPS())
+  arch.MergeFrom(target_arch);
+
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }
 
   return arch;
 }
Index: source/Core/ArchSpec.cpp
===
--- source/Core/ArchSpec.cpp
+++ source/Core/ArchSpec.cpp
@@ -1002,6 +1002,9 @@
 m_core = other.GetCore();
 CoreUpdated(true);
   }
+  if (GetFlags() == 0) {
+SetFlags(other.GetFlags());
+  }
 }
 
 bool ArchSpec::SetArchitecture(ArchitectureType arch_type, uint32_t cpu,


Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -724,15 +724,15 @@
 }
 
 ArchSpec ProcessElfCore::GetArchitecture() {
-  ObjectFileELF *core_file =
-  (ObjectFileELF *)(m_core_module_sp->GetObjectFile());
   ArchSpec arch;
-  core_file->GetArchitecture(arch);
+  m_core_module_sp->GetObjectFile()->GetArchitecture(arch);
 
   ArchSpec target_arch = GetTarget().GetArchitecture();
-  
-  if (target_arch.IsMIPS())
+  arch.MergeFrom(target_arch);
+
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }
 
   return arch;
 }
Index: source/Core/ArchSpec.cpp
===
--- source/Core/ArchSpec.cpp
+++ source/Core/ArchSpec.cpp
@@ -1002,6 +1002,9 @@
 m_core = other.GetCore();
 CoreUpdated(true);
   }
+  if (GetFlags() == 0) {
+SetFlags(other.GetFlags());
+  }
 }
 
 bool ArchSpec::SetArchitecture(ArchitectureType arch_type, uint32_t cpu,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D36046: Improve the posix core file triple detection

2017-07-29 Thread Tamas Berghammer via Phabricator via lldb-commits
tberghammer added inline comments.



Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:733-735
+  if (target_arch.IsMIPS()) {
 return target_arch;
+  }

Hi Nitesh,

I tried to remove this MIPS specific code as it shouldn't be necessary if I add 
the above MergeFrom for all architecture but if I do it fails LinuxCore
TestCase.test_mips_n32.

The issue is that in that case the tripe we get from the core file is 
"mipsel--" ("mipsel--linux" after the merge) and the one we get from the binary 
is "mips64el--linux". Is it normal to have a seemingly 32bit core file with a 
64bit binary on mips? If not then can I ask you to help me figure out which one 
is incorrect?

If it is then I don't see any better short term solution then leaving this 
condition here but it feels like a quite big hack what might backfire when 
somebody tries to use a core file with a completely incompatible binary.

Thanks,
Tamas


https://reviews.llvm.org/D36046



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


[Lldb-commits] [PATCH] D33213: Use socketpair on all Unix platforms

2017-07-29 Thread Demi Marie Obenour via Phabricator via lldb-commits
DemiMarie updated this revision to Diff 108796.
DemiMarie added a comment.

Set FD_CLOEXEC on the communication fd


https://reviews.llvm.org/D33213

Files:
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  tools/lldb-server/lldb-gdbserver.cpp

Index: tools/lldb-server/lldb-gdbserver.cpp
===
--- tools/lldb-server/lldb-gdbserver.cpp
+++ tools/lldb-server/lldb-gdbserver.cpp
@@ -106,6 +106,7 @@
// than llgs listening for a connection from address on port.
 {"setsid", no_argument, NULL,
  'S'}, // Call setsid() to make llgs run in its own session.
+{"fd", required_argument, NULL, 'F'},
 {NULL, 0, NULL, 0}};
 
 //--
@@ -132,13 +133,13 @@
   "[--log-file log-file-name] "
   "[--log-channels log-channel-list] "
   "[--setsid] "
+  "[--fd file-descriptor]"
   "[--named-pipe named-pipe-path] "
   "[--native-regs] "
   "[--attach pid] "
   "[[HOST]:PORT] "
   "[-- PROGRAM ARG1 ARG2 ...]\n",
   progname, subcommand);
-  exit(0);
 }
 
 void handle_attach_to_pid(GDBRemoteCommunicationServerLLGS &gdb_server,
@@ -232,10 +233,34 @@
  GDBRemoteCommunicationServerLLGS &gdb_server,
  bool reverse_connect, const char *const host_and_port,
  const char *const progname, const char *const subcommand,
- const char *const named_pipe_path, int unnamed_pipe_fd) {
+ const char *const named_pipe_path, int unnamed_pipe_fd,
+ int connection_fd) {
   Status error;
 
-  if (host_and_port && host_and_port[0]) {
+  std::unique_ptr connection_up;
+  if (connection_fd != -1) {
+// Build the connection string.
+char connection_url[512];
+snprintf(connection_url, sizeof(connection_url), "fd://%d", connection_fd);
+
+// Create the connection.
+#if !defined LLDB_DISABLE_POSIX && !defined _WIN32
+::fcntl(connection_fd, F_SETFD, FD_CLOEXEC);
+#endif
+connection_up.reset(new ConnectionFileDescriptor);
+auto connection_result = connection_up->Connect(connection_url, &error);
+if (connection_result != eConnectionStatusSuccess) {
+  fprintf(stderr, "error: failed to connect to client at '%s' "
+  "(connection status: %d)",
+  connection_url, static_cast(connection_result));
+  exit(-1);
+}
+if (error.Fail()) {
+  fprintf(stderr, "error: failed to connect to client at '%s': %s",
+  connection_url, error.AsCString());
+  exit(-1);
+}
+  } else if (host_and_port && host_and_port[0]) {
 // Parse out host and port.
 std::string final_host_and_port;
 std::string connection_host;
@@ -255,7 +280,6 @@
   connection_portno = StringConvert::ToUInt32(connection_port.c_str(), 0);
 }
 
-std::unique_ptr connection_up;
 
 if (reverse_connect) {
   // llgs will connect to the gdb-remote client.
@@ -328,14 +352,14 @@
   }
   connection_up.reset(conn);
 }
-error = gdb_server.InitializeConnection(std::move(connection_up));
-if (error.Fail()) {
-  fprintf(stderr, "Failed to initialize connection: %s\n",
-  error.AsCString());
-  exit(-1);
-}
-printf("Connection established.\n");
   }
+  error = gdb_server.InitializeConnection(std::move(connection_up));
+  if (error.Fail()) {
+fprintf(stderr, "Failed to initialize connection: %s\n",
+error.AsCString());
+exit(-1);
+  }
+  printf("Connection established.\n");
 }
 
 //--
@@ -364,6 +388,7 @@
   log_channels; // e.g. "lldb process threads:gdb-remote default:linux all"
   int unnamed_pipe_fd = -1;
   bool reverse_connect = false;
+  int connection_fd = -1;
 
   // ProcessLaunchInfo launch_info;
   ProcessAttachInfo attach_info;
@@ -413,6 +438,10 @@
   reverse_connect = true;
   break;
 
+case 'F':
+  connection_fd = StringConvert::ToUInt32(optarg, -1);
+  break;
+
 #ifndef _WIN32
 case 'S':
   // Put llgs into a new session. Terminals group processes
@@ -472,7 +501,8 @@
   argc -= optind;
   argv += optind;
 
-  if (argc == 0) {
+  if (argc == 0 && connection_fd == -1) {
+fputs("No arguments\n", stderr);
 display_usage(progname, subcommand);
 exit(255);
   }
@@ -501,7 +531,7 @@
 
   ConnectToRemote(mainloop, gdb_server, reverse_connect, host_and_port,
   progname, subcommand, named_pipe_path.c_str(),
-  unnamed_pipe_fd);
+  unnamed_pipe_fd, connection_fd);
 
   if (!gdb_server.IsConnected()) {
 fprintf(stderr, "no connection information provided, unable to run\n");
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
=