[Lldb-commits] [PATCH] D56237: Implement GetLoadAddress for the Windows process plugin

[Aaron Smith via Phabricator via lldb-commits]

asmith updated this revision to Diff 186799.
Herald added a subscriber: mgorny.

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

https://reviews.llvm.org/D56237

Files:
  source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
  source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h
  source/Plugins/Process/Windows/Common/CMakeLists.txt
  source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  source/Plugins/Process/Windows/Common/ProcessWindows.h

Index: source/Plugins/Process/Windows/Common/ProcessWindows.h
===
--- source/Plugins/Process/Windows/Common/ProcessWindows.h
+++ source/Plugins/Process/Windows/Common/ProcessWindows.h
@@ -16,6 +16,7 @@
 #include "llvm/Support/Mutex.h"
 
 #include "IDebugDelegate.h"
+#include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
 
 namespace lldb_private {
 
@@ -90,6 +91,8 @@
 
   lldb::addr_t GetImageInfoAddress() override;
 
+  DynamicLoaderWindowsDYLD *GetDynamicLoader() override;
+
   // IDebugDelegate overrides.
   void OnExitProcess(uint32_t exit_code) override;
   void OnDebuggerConnected(lldb::addr_t image_base) override;
Index: source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -864,6 +864,13 @@
 return LLDB_INVALID_ADDRESS;
 }
 
+DynamicLoaderWindowsDYLD *ProcessWindows::GetDynamicLoader() {
+  if (m_dyld_ap.get() == NULL)
+m_dyld_ap.reset(DynamicLoader::FindPlugin(
+this, DynamicLoaderWindowsDYLD::GetPluginNameStatic().GetCString()));
+  return static_cast(m_dyld_ap.get());
+}
+
 void ProcessWindows::OnExitProcess(uint32_t exit_code) {
   // No need to acquire the lock since m_session_data isn't accessed.
   Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_PROCESS);
@@ -916,12 +923,10 @@
 GetTarget().SetExecutableModule(module, eLoadDependentsNo);
   }
 
-  bool load_addr_changed;
-  module->SetLoadAddress(GetTarget(), image_base, false, load_addr_changed);
-
-  ModuleList loaded_modules;
-  loaded_modules.Append(module);
-  GetTarget().ModulesDidLoad(loaded_modules);
+  if (auto dyld = GetDynamicLoader())
+dyld->OnLoadModule(
+ModuleSpec(module->GetFileSpec(), module->GetArchitecture()),
+image_base);
 
   // Add the main executable module to the list of pending module loads.  We
   // can't call GetTarget().ModulesDidLoad() here because we still haven't
@@ -1027,29 +1032,13 @@
 
 void ProcessWindows::OnLoadDll(const ModuleSpec &module_spec,
lldb::addr_t module_addr) {
-  // Confusingly, there is no Target::AddSharedModule.  Instead, calling
-  // GetSharedModule() with a new module will add it to the module list and
-  // return a corresponding ModuleSP.
-  Status error;
-  ModuleSP module = GetTarget().GetSharedModule(module_spec, &error);
-  bool load_addr_changed = false;
-  module->SetLoadAddress(GetTarget(), module_addr, false, load_addr_changed);
-
-  ModuleList loaded_modules;
-  loaded_modules.Append(module);
-  GetTarget().ModulesDidLoad(loaded_modules);
+  if (auto dyld = GetDynamicLoader())
+dyld->OnLoadModule(module_spec, module_addr);
 }
 
 void ProcessWindows::OnUnloadDll(lldb::addr_t module_addr) {
-  Address resolved_addr;
-  if (GetTarget().ResolveLoadAddress(module_addr, resolved_addr)) {
-ModuleSP module = resolved_addr.GetModule();
-if (module) {
-  ModuleList unloaded_modules;
-  unloaded_modules.Append(module);
-  GetTarget().ModulesDidUnload(unloaded_modules, false);
-}
-  }
+  if (auto dyld = GetDynamicLoader())
+dyld->OnUnloadModule(module_addr);
 }
 
 void ProcessWindows::OnDebugString(const std::string &string) {}
Index: source/Plugins/Process/Windows/Common/CMakeLists.txt
===
--- source/Plugins/Process/Windows/Common/CMakeLists.txt
+++ source/Plugins/Process/Windows/Common/CMakeLists.txt
@@ -24,6 +24,7 @@
 lldbCore
 lldbHost
 lldbInterpreter
+lldbPluginDynamicLoaderWindowsDYLD
 lldbSymbol
 lldbTarget
 ws2_32
Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h
===
--- source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h
+++ source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h
@@ -12,6 +12,8 @@
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/lldb-forward.h"
 
+#include 
+
 namespace lldb_private {
 
 class DynamicLoaderWindowsDYLD : public DynamicLoader {
@@ -27,6 +29,9 @@
 
   static DynamicLoader *CreateInstance(Process *process, bool force);
 
+  void OnLoadModule(const ModuleSpec &module_spec, lldb::addr_t module_addr);
+  void OnUnloadModule(lldb::addr_t module_addr);
+
   void Di

[Lldb-commits] [lldb] r354012 - [gdb-remote] Sanity check platform pointer

[Aaron Smith via lldb-commits]

Author: asmith
Date: Thu Feb 14 00:59:04 2019
New Revision: 354012

URL: http://llvm.org/viewvc/llvm-project?rev=354012&view=rev
Log:
[gdb-remote] Sanity check platform pointer

Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp?rev=354012&r1=354011&r2=354012&view=diff
==
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Thu 
Feb 14 00:59:04 2019
@@ -979,8 +979,11 @@ Status GDBRemoteCommunication::StartDebu
 
 g_debugserver_file_spec = debugserver_file_spec;
   } else {
-debugserver_file_spec =
-platform->LocateExecutable(DEBUGSERVER_BASENAME);
+if (platform)
+  debugserver_file_spec =
+  platform->LocateExecutable(DEBUGSERVER_BASENAME);
+else
+  debugserver_file_spec.Clear();
 if (debugserver_file_spec) {
   // Platform::LocateExecutable() wouldn't return a path if it doesn't
   // exist


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

[Lldb-commits] [PATCH] D58223: [lldb] [unittests] XFAIL two unittests failing on NetBSD

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, labath, zturner.
mgorny added a project: LLDB.

The two following LLDBServerTests fail reliably on NetBSD:

StandardStartupTest.TestStopReplyContainsThreadPcs
 TestBase.LaunchModePreservesEnvironment

Establish an XFAIL behavior to let buildbots test for regressions
while we are still working on fixing them.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58223

Files:
  lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
  lldb/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp


Index: lldb/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
===
--- lldb/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
+++ lldb/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
@@ -29,6 +29,10 @@
   Succeeded());
 
   ASSERT_THAT_ERROR(Client->ListThreadsInStopReply(), Succeeded());
+#if defined(__NetBSD__)
+  // XFAIL: this test currently fails on NetBSD
+  ASSERT_THAT_ERROR(Client->ContinueAll(), Failed());
+#else
   ASSERT_THAT_ERROR(Client->ContinueAll(), Succeeded());
   unsigned int pc_reg = Client->GetPcRegisterId();
   ASSERT_NE(pc_reg, UINT_MAX);
@@ -50,4 +54,5 @@
 EXPECT_THAT(thread_infos[tid].ReadRegister(pc_reg),
 Pointee(Eq(stop_reply_pc.second)));
   }
+#endif
 }
Index: lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
===
--- lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -22,11 +22,16 @@
   ASSERT_THAT_EXPECTED(ClientOr, Succeeded());
   auto &Client = **ClientOr;
 
+#if defined(__NetBSD__)
+  // XFAIL: this test currently fails on NetBSD
+  ASSERT_THAT_ERROR(Client.ContinueAll(), Failed());
+#else
   ASSERT_THAT_ERROR(Client.ContinueAll(), Succeeded());
   ASSERT_THAT_EXPECTED(
   Client.GetLatestStopReplyAs(),
   HasValue(testing::Property(&StopReply::getKind,
  WaitStatus{WaitStatus::Exit, 0})));
+#endif
 }
 
 TEST_F(TestBase, DS_TEST(DebugserverEnv)) {


Index: lldb/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
===
--- lldb/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
+++ lldb/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
@@ -29,6 +29,10 @@
   Succeeded());
 
   ASSERT_THAT_ERROR(Client->ListThreadsInStopReply(), Succeeded());
+#if defined(__NetBSD__)
+  // XFAIL: this test currently fails on NetBSD
+  ASSERT_THAT_ERROR(Client->ContinueAll(), Failed());
+#else
   ASSERT_THAT_ERROR(Client->ContinueAll(), Succeeded());
   unsigned int pc_reg = Client->GetPcRegisterId();
   ASSERT_NE(pc_reg, UINT_MAX);
@@ -50,4 +54,5 @@
 EXPECT_THAT(thread_infos[tid].ReadRegister(pc_reg),
 Pointee(Eq(stop_reply_pc.second)));
   }
+#endif
 }
Index: lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
===
--- lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -22,11 +22,16 @@
   ASSERT_THAT_EXPECTED(ClientOr, Succeeded());
   auto &Client = **ClientOr;
 
+#if defined(__NetBSD__)
+  // XFAIL: this test currently fails on NetBSD
+  ASSERT_THAT_ERROR(Client.ContinueAll(), Failed());
+#else
   ASSERT_THAT_ERROR(Client.ContinueAll(), Succeeded());
   ASSERT_THAT_EXPECTED(
   Client.GetLatestStopReplyAs(),
   HasValue(testing::Property(&StopReply::getKind,
  WaitStatus{WaitStatus::Exit, 0})));
+#endif
 }
 
 TEST_F(TestBase, DS_TEST(DebugserverEnv)) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58223: [lldb] [unittests] XFAIL two unittests failing on NetBSD

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

FTR, I've tried both @zturner's and @labath's XFAIL suggestions and the former 
doesn't work because we're not using exeptions, and the latter just gives some 
creepy C++ errors. Then I figured out this is probably the simplest and most 
readable possibility.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58223



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

[Lldb-commits] [PATCH] D58222: [ClangExpressionParser] Reuse the FileManager from the compiler instance.

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM.


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

https://reviews.llvm.org/D58222



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

[Lldb-commits] [PATCH] D58227: [lldb] [MainLoop] Remove redundant termination clause (NFCI)

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, labath.
mgorny added a project: LLDB.
Herald added a subscriber: abidh.

Remove the redundant termination clause from within the loop.  Since
the check is done at the end of the loop, it's entirely redundant
to the 'while' condition.  If termination was requested, the latter
will become false and the 'while' loop will terminate, resulting
in the 'return' statement below the loop being executed (which is
equivalent to the one used inside 'if').


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58227

Files:
  lldb/source/Host/common/MainLoop.cpp


Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -380,9 +380,6 @@
   return error;
 
 impl.ProcessEvents();
-
-if (m_terminate_request)
-  return Status();
   }
   return Status();
 }


Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -380,9 +380,6 @@
   return error;
 
 impl.ProcessEvents();
-
-if (m_terminate_request)
-  return Status();
   }
   return Status();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58228: [lldb] [lldb-server] Catch and report errors from main loop

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, krytarowski.
mgorny added a project: LLDB.
Herald added a subscriber: abidh.

Catch the possible error from lldb-gdbserver's main loop, and report
it verbosely.  Currently, if the loop fails the server exits normally,
rendering the problem indistinguishable from regular termination.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58228

Files:
  lldb/tools/lldb-server/lldb-gdbserver.cpp


Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -533,7 +533,12 @@
 return 1;
   }
 
-  mainloop.Run();
+  Status ret = mainloop.Run();
+  if (ret.Fail()) {
+fprintf(stderr, "lldb-server terminating due to error: %s\n",
+ret.AsCString());
+return 1;
+  }
   fprintf(stderr, "lldb-server exiting...\n");
 
   return 0;


Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -533,7 +533,12 @@
 return 1;
   }
 
-  mainloop.Run();
+  Status ret = mainloop.Run();
+  if (ret.Fail()) {
+fprintf(stderr, "lldb-server terminating due to error: %s\n",
+ret.AsCString());
+return 1;
+  }
   fprintf(stderr, "lldb-server exiting...\n");
 
   return 0;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58229: [lldb] [MainLoop] Report errno for failed kevent()

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, krytarowski.
mgorny added a project: LLDB.
Herald added a subscriber: emaste.

Modify the kevent() error reporting to use errno rather than returning
the return value.  At least on FreeBSD and NetBSD, kevent() always
returns -1 in case of error, and the actual error is returned via errno.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58229

Files:
  lldb/source/Host/common/MainLoop.cpp


Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -108,7 +108,7 @@
   out_events, llvm::array_lengthof(out_events), nullptr);
 
   if (num_events < 0)
-return Status("kevent() failed with error %d\n", num_events);
+return Status(errno, eErrorTypePOSIX);
   return Status();
 }
 


Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -108,7 +108,7 @@
   out_events, llvm::array_lengthof(out_events), nullptr);
 
   if (num_events < 0)
-return Status("kevent() failed with error %d\n", num_events);
+return Status(errno, eErrorTypePOSIX);
   return Status();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58230: [lldb] [MainLoop] Add kevent() EINTR handling

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, labath.
mgorny added a project: LLDB.
Herald added a subscriber: abidh.

Add missing EINTR handling for kevent() calls.  If the call is
interrupted, return from Poll() as if zero events were returned and let
the polling resume on next iteration.  This fixes test flakiness
on NetBSD.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58230

Files:
  lldb/source/Host/common/MainLoop.cpp


Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -107,8 +107,14 @@
   num_events = kevent(loop.m_kqueue, in_events.data(), in_events.size(),
   out_events, llvm::array_lengthof(out_events), nullptr);
 
-  if (num_events < 0)
-return Status(errno, eErrorTypePOSIX);
+  if (num_events < 0) {
+if (errno == EINTR) {
+  // in case of EINTR, let the main loop run one iteration
+  // we need to zero num_events to avoid assertions failing
+  num_events = 0;
+} else
+  return Status(errno, eErrorTypePOSIX);
+  }
   return Status();
 }
 


Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -107,8 +107,14 @@
   num_events = kevent(loop.m_kqueue, in_events.data(), in_events.size(),
   out_events, llvm::array_lengthof(out_events), nullptr);
 
-  if (num_events < 0)
-return Status(errno, eErrorTypePOSIX);
+  if (num_events < 0) {
+if (errno == EINTR) {
+  // in case of EINTR, let the main loop run one iteration
+  // we need to zero num_events to avoid assertions failing
+  num_events = 0;
+} else
+  return Status(errno, eErrorTypePOSIX);
+  }
   return Status();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58177: Fix lldb-server test suite for python3

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for the review Stella. I was hoping someone would step in and tell me 
that there is a better way to do that. However :), I see two problems with your 
proposal:

- build.py is a standalone file, and so it is not easy for it to share code 
with other stuff. This can be solved with some use_lldb_suite magic or moving 
the script some place else, but then comes the second problem:
- I am not sure these two use cases are actually compatible. Here I 
specifically want to "reinterpret_cast" binary bytes without doing any sort of 
encoding. The only place where `to_string` is used in `build.py` currently is 
on a value retrieved from the windows registry. I'm not 100% sure, but it seems 
to me that using utf8 encoding (which is what `to_string` is doing) is 
precisely the right thing to do there.

With that in mind, I propose to do the following:

- leave build.py alone for now
- rename the new functions I'm introducing here to something less generic. 
Maybe `bitcast_to_string` and `bitcast_to_bytes`?

WDYT?


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

https://reviews.llvm.org/D58177



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

[Lldb-commits] [PATCH] D58177: Fix lldb-server test suite for python3

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py:415
 # Add zero-fill to the right/end (MSB side) of the value.
-retval += "00" * (byte_size - len(retval) / 2)
+retval += "00" * (byte_size - len(retval) // 2)
 return retval

serge-sans-paille wrote:
> Maybe `from __future__ import division` to make sure the `/` operator behaves 
> consistently across versions? That way it may uncover missed `/` to `//` 
> conversion?
That sounds like a good idea. I didn't know about that. Thanks.


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

https://reviews.llvm.org/D58177



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

[Lldb-commits] [PATCH] D58177: Fix lldb-server test suite for python3

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 186823.
labath marked an inline comment as done.
labath added a comment.

- rename the string conversion functions and explain their operation in more 
detail
- add "from __future__ import division" to ensure consistency between python 
versions


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

https://reviews.llvm.org/D58177

Files:
  packages/Python/lldbsuite/support/seven.py
  packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteModuleInfo.py
  packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
  packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
  packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py

Index: packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py
===
--- packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py
+++ packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py
@@ -9,6 +9,7 @@
 import codecs
 
 from six.moves import queue
+from lldbsuite.support import seven
 
 
 def _handle_output_packet_string(packet_contents):
@@ -19,7 +20,7 @@
 elif packet_contents == "OK":
 return None
 else:
-return packet_contents[1:].decode("hex")
+return seven.unhexlify(packet_contents[1:])
 
 
 def _dump_queue(the_queue):
@@ -174,7 +175,7 @@
 can_read, _, _ = select.select([self._socket], [], [], 0)
 if can_read and self._socket in can_read:
 try:
-new_bytes = self._socket.recv(4096)
+new_bytes = seven.bitcast_to_string(self._socket.recv(4096))
 if self._logger and new_bytes and len(new_bytes) > 0:
 self._logger.debug(
 "pump received bytes: {}".format(new_bytes))
Index: packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
===
--- packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
+++ packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
@@ -1,7 +1,7 @@
 """Module for supporting unit testing of the lldb-server debug monitor exe.
 """
 
-from __future__ import print_function
+from __future__ import division, print_function
 
 
 import os
@@ -412,7 +412,7 @@
 value = value >> 8
 if byte_size:
 # Add zero-fill to the right/end (MSB side) of the value.
-retval += "00" * (byte_size - len(retval) / 2)
+retval += "00" * (byte_size - len(retval) // 2)
 return retval
 
 elif endian == 'big':
@@ -422,7 +422,7 @@
 value = value >> 8
 if byte_size:
 # Add zero-fill to the left/front (MSB side) of the value.
-retval = ("00" * (byte_size - len(retval) / 2)) + retval
+retval = ("00" * (byte_size - len(retval) // 2)) + retval
 return retval
 
 else:
Index: packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
===
--- packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -2,7 +2,7 @@
 Base class for gdb-remote test cases.
 """
 
-from __future__ import print_function
+from __future__ import division, print_function
 
 
 import errno
@@ -20,6 +20,7 @@
 import time
 from lldbsuite.test import configuration
 from lldbsuite.test.lldbtest import *
+from lldbsuite.support import seven
 from lldbgdbserverutils import *
 import logging
 
@@ -589,7 +590,7 @@
 if can_read and sock in can_read:
 recv_bytes = sock.recv(4096)
 if recv_bytes:
-response += recv_bytes.decode("utf-8")
+response += seven.bitcast_to_string(recv_bytes)
 
 self.assertTrue(expected_content_regex.match(response))
 
@@ -1235,7 +1236,7 @@
 reg_index = reg_info["lldb_register_index"]
 self.assertIsNotNone(reg_index)
 
-reg_byte_size = int(reg_info["bitsize"]) / 8
+reg_byte_size = int(reg_info["bitsize"]) // 8
 self.assertTrue(reg_byte_size > 0)
 
 # Handle thread suffix.
@@ -1261,7 +1262,7 @@
 endian, p_response)
 
 # Flip the value by xoring with all 1s
-all_one_bits_raw = "ff" * (int(reg_info["bitsize"]) / 8)
+all_one_bits_raw = "ff" * (int(reg_info["bitsize"]) // 8)
 flipped_bits_int = initial_reg_value ^ int(all_one_bits_raw, 16)
 # print("reg (index={}, name={}): val={}, flipped bits (int={}, hex={:x})".format(reg_index, reg_info["name"], initial_reg_value, flipped_bits_int, flipped_bits_int))
 
@@ -1476,8 +1477,8 @@

[Lldb-commits] [PATCH] D58230: [lldb] [MainLoop] Add kevent() EINTR handling

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

We already had a patch for this, but it kind of got forgotten while we were 
figuring out how to test it. I had made a test for the behavior here 
https://reviews.llvm.org/D42206#980045. Could you please try it out and 
incorporate it into the patch (or write some other kind of test for this fix)?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58230



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

[Lldb-commits] [PATCH] D58229: [lldb] [MainLoop] Report errno for failed kevent()

[Pavel Labath via Phabricator via lldb-commits]

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

That seems right. Even the macos manpage for kevent says the error is returned 
through errno.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58229



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

[Lldb-commits] [lldb] r354029 - [lldb] [MainLoop] Report errno for failed kevent()

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Thu Feb 14 05:52:31 2019
New Revision: 354029

URL: http://llvm.org/viewvc/llvm-project?rev=354029&view=rev
Log:
[lldb] [MainLoop] Report errno for failed kevent()

Modify the kevent() error reporting to use errno rather than returning
the return value.  At least on FreeBSD and NetBSD, kevent() always
returns -1 in case of error, and the actual error is returned via errno.

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

Modified:
lldb/trunk/source/Host/common/MainLoop.cpp

Modified: lldb/trunk/source/Host/common/MainLoop.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/MainLoop.cpp?rev=354029&r1=354028&r2=354029&view=diff
==
--- lldb/trunk/source/Host/common/MainLoop.cpp (original)
+++ lldb/trunk/source/Host/common/MainLoop.cpp Thu Feb 14 05:52:31 2019
@@ -108,7 +108,7 @@ Status MainLoop::RunImpl::Poll() {
   out_events, llvm::array_lengthof(out_events), nullptr);
 
   if (num_events < 0)
-return Status("kevent() failed with error %d\n", num_events);
+return Status(errno, eErrorTypePOSIX);
   return Status();
 }
 


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

[Lldb-commits] [PATCH] D58229: [lldb] [MainLoop] Report errno for failed kevent()

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354029: [lldb] [MainLoop] Report errno for failed kevent() 
(authored by mgorny, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58229?vs=186816&id=186829#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58229

Files:
  lldb/trunk/source/Host/common/MainLoop.cpp


Index: lldb/trunk/source/Host/common/MainLoop.cpp
===
--- lldb/trunk/source/Host/common/MainLoop.cpp
+++ lldb/trunk/source/Host/common/MainLoop.cpp
@@ -108,7 +108,7 @@
   out_events, llvm::array_lengthof(out_events), nullptr);
 
   if (num_events < 0)
-return Status("kevent() failed with error %d\n", num_events);
+return Status(errno, eErrorTypePOSIX);
   return Status();
 }
 


Index: lldb/trunk/source/Host/common/MainLoop.cpp
===
--- lldb/trunk/source/Host/common/MainLoop.cpp
+++ lldb/trunk/source/Host/common/MainLoop.cpp
@@ -108,7 +108,7 @@
   out_events, llvm::array_lengthof(out_events), nullptr);
 
   if (num_events < 0)
-return Status("kevent() failed with error %d\n", num_events);
+return Status(errno, eErrorTypePOSIX);
   return Status();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58228: [lldb] [lldb-server] Catch and report errors from main loop

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354030: [lldb] [lldb-server] Catch and report errors from 
main loop (authored by mgorny, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58228?vs=186812&id=186830#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58228

Files:
  lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp


Index: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
@@ -533,7 +533,12 @@
 return 1;
   }
 
-  mainloop.Run();
+  Status ret = mainloop.Run();
+  if (ret.Fail()) {
+fprintf(stderr, "lldb-server terminating due to error: %s\n",
+ret.AsCString());
+return 1;
+  }
   fprintf(stderr, "lldb-server exiting...\n");
 
   return 0;


Index: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
@@ -533,7 +533,12 @@
 return 1;
   }
 
-  mainloop.Run();
+  Status ret = mainloop.Run();
+  if (ret.Fail()) {
+fprintf(stderr, "lldb-server terminating due to error: %s\n",
+ret.AsCString());
+return 1;
+  }
   fprintf(stderr, "lldb-server exiting...\n");
 
   return 0;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r354030 - [lldb] [lldb-server] Catch and report errors from main loop

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Thu Feb 14 05:52:37 2019
New Revision: 354030

URL: http://llvm.org/viewvc/llvm-project?rev=354030&view=rev
Log:
[lldb] [lldb-server] Catch and report errors from main loop

Catch the possible error from lldb-gdbserver's main loop, and report
it verbosely.  Currently, if the loop fails the server exits normally,
rendering the problem indistinguishable from regular termination.

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

Modified:
lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp

Modified: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp?rev=354030&r1=354029&r2=354030&view=diff
==
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp (original)
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp Thu Feb 14 05:52:37 2019
@@ -533,7 +533,12 @@ int main_gdbserver(int argc, char *argv[
 return 1;
   }
 
-  mainloop.Run();
+  Status ret = mainloop.Run();
+  if (ret.Fail()) {
+fprintf(stderr, "lldb-server terminating due to error: %s\n",
+ret.AsCString());
+return 1;
+  }
   fprintf(stderr, "lldb-server exiting...\n");
 
   return 0;


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

[Lldb-commits] [PATCH] D58129: Move UnwindTable from ObjectFile to Module

[Pavel Labath via Phabricator via lldb-commits]

labath marked 3 inline comments as done.
labath added inline comments.



Comment at: include/lldb/Core/Module.h:1108-1110
+  llvm::Optional m_unwind_table; /// < Table of FuncUnwinders
+  /// objects created for this
+  /// Module's functions

clayborg wrote:
> Any reason to not just have a UnwindTable instance here? The accessor can't 
> fail, so one must be created anyway right?
The difference is in when it gets created. The regular instance would have to 
be created together with the Module. However, that shouldn't really matter, as 
UnwindTable is internally lazily initialized as well, so I'll just remove the 
optional. We may need to do something smarter here anyway once we start 
re-initializing the unwind info due to symbol file changes.


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

https://reviews.llvm.org/D58129



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

[Lldb-commits] [lldb] r354033 - Move UnwindTable from ObjectFile to Module

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu Feb 14 06:40:10 2019
New Revision: 354033

URL: http://llvm.org/viewvc/llvm-project?rev=354033&view=rev
Log:
Move UnwindTable from ObjectFile to Module

Summary:
This is a preparatory step to enable adding extra unwind strategies by
symbol file plugins. This has been discussed on the lldb-dev mailing
list: .

Reviewers: jasonmolenda, clayborg, espindola

Subscribers: lemo, emaste, lldb-commits, arichardson

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

Modified:
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/include/lldb/Symbol/ObjectFile.h
lldb/trunk/include/lldb/Symbol/UnwindTable.h
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
lldb/trunk/source/Symbol/ObjectFile.cpp
lldb/trunk/source/Symbol/UnwindTable.cpp

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=354033&r1=354032&r2=354033&view=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Thu Feb 14 06:40:10 2019
@@ -693,6 +693,21 @@ public:
   //--
   virtual void SectionFileAddressesChanged();
 
+  //--
+  /// Returns a reference to the UnwindTable for this Module
+  ///
+  /// The UnwindTable contains FuncUnwinders objects for any function in this
+  /// Module.  If a FuncUnwinders object hasn't been created yet (i.e. the
+  /// function has yet to be unwound in a stack walk), it will be created when
+  /// requested.  Specifically, we do not create FuncUnwinders objects for
+  /// functions until they are needed.
+  ///
+  /// @return
+  /// Returns the unwind table for this module. If this object has no
+  /// associated object file, an empty UnwindTable is returned.
+  //--
+  UnwindTable &GetUnwindTable() { return m_unwind_table; }
+
   llvm::VersionTuple GetVersion();
 
   //--
@@ -1090,6 +1105,8 @@ protected:
   lldb::ObjectFileSP m_objfile_sp; ///< A shared pointer to the object file
///parser for this module as it may or may
///not be shared with the SymbolFile
+  UnwindTable m_unwind_table{*this}; ///< Table of FuncUnwinders objects 
created
+ /// for this Module's functions
   lldb::SymbolVendorUP
   m_symfile_up; ///< A pointer to the symbol vendor for this module.
   std::vector

Modified: lldb/trunk/include/lldb/Symbol/ObjectFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ObjectFile.h?rev=354033&r1=354032&r2=354033&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ObjectFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/ObjectFile.h Thu Feb 14 06:40:10 2019
@@ -478,20 +478,6 @@ public:
   virtual bool ParseHeader() = 0;
 
   //--
-  /// Returns a reference to the UnwindTable for this ObjectFile
-  ///
-  /// The UnwindTable contains FuncUnwinders objects for any function in this
-  /// ObjectFile.  If a FuncUnwinders object hasn't been created yet (i.e. the
-  /// function has yet to be unwound in a stack walk), it will be created when
-  /// requested.  Specifically, we do not create FuncUnwinders objects for
-  /// functions until they are needed.
-  ///
-  /// @return
-  /// Returns the unwind table for this object file.
-  //--
-  virtual lldb_private::UnwindTable &GetUnwindTable() { return m_unwind_table; 
}
-
-  //--
   /// Returns if the function bounds for symbols in this symbol file are
   /// likely accurate.
   ///
@@ -774,9 +760,6 @@ protected:
  ///determined).
   DataExtractor
   m_data; ///< The data for this object file so things can be parsed 
lazily.
-  lldb_private::UnwindTable m_unwind_table; /// < Table of FuncUnwinders 
objects
-/// created for this ObjectFile's
-/// functions
   lldb::ProcessWP m_process_wp;
   const lldb::addr_t m_memory_addr;
   std::unique_ptr m_sections_up;

Modified: lldb/trunk/include/lldb/Symbol/UnwindTable.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/UnwindTable.h?rev=354033&r1=354032&r2=354033&view=diff
=

[Lldb-commits] [PATCH] D58129: Move UnwindTable from ObjectFile to Module

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rL354033: Move UnwindTable from ObjectFile to Module (authored 
by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58129?vs=186599&id=186836#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58129

Files:
  lldb/trunk/include/lldb/Core/Module.h
  lldb/trunk/include/lldb/Symbol/ObjectFile.h
  lldb/trunk/include/lldb/Symbol/UnwindTable.h
  lldb/trunk/source/Commands/CommandObjectTarget.cpp
  lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  lldb/trunk/source/Symbol/ObjectFile.cpp
  lldb/trunk/source/Symbol/UnwindTable.cpp

Index: lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
===
--- lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
@@ -239,9 +239,8 @@
 
 if (m_sym_ctx_valid) {
   func_unwinders_sp =
-  pc_module_sp->GetObjectFile()
-  ->GetUnwindTable()
-  .GetFuncUnwindersContainingAddress(m_current_pc, m_sym_ctx);
+  pc_module_sp->GetUnwindTable().GetFuncUnwindersContainingAddress(
+  m_current_pc, m_sym_ctx);
 }
 
 if (func_unwinders_sp.get() != nullptr)
@@ -672,9 +671,8 @@
 return unwind_plan_sp;
 
   FuncUnwindersSP func_unwinders_sp(
-  pc_module_sp->GetObjectFile()
-  ->GetUnwindTable()
-  .GetFuncUnwindersContainingAddress(m_current_pc, m_sym_ctx));
+  pc_module_sp->GetUnwindTable().GetFuncUnwindersContainingAddress(
+  m_current_pc, m_sym_ctx));
   if (!func_unwinders_sp)
 return unwind_plan_sp;
 
@@ -775,9 +773,8 @@
   FuncUnwindersSP func_unwinders_sp;
   if (m_sym_ctx_valid) {
 func_unwinders_sp =
-pc_module_sp->GetObjectFile()
-->GetUnwindTable()
-.GetFuncUnwindersContainingAddress(m_current_pc, m_sym_ctx);
+pc_module_sp->GetUnwindTable().GetFuncUnwindersContainingAddress(
+m_current_pc, m_sym_ctx);
   }
 
   // No FuncUnwinders available for this pc (stripped function symbols, lldb
@@ -795,7 +792,7 @@
 // Even with -fomit-frame-pointer, we can try eh_frame to get back on
 // track.
 DWARFCallFrameInfo *eh_frame =
-pc_module_sp->GetObjectFile()->GetUnwindTable().GetEHFrameInfo();
+pc_module_sp->GetUnwindTable().GetEHFrameInfo();
 if (eh_frame) {
   unwind_plan_sp = std::make_shared(lldb::eRegisterKindGeneric);
   if (eh_frame->GetUnwindPlan(m_current_pc, *unwind_plan_sp))
@@ -805,7 +802,7 @@
 }
 
 ArmUnwindInfo *arm_exidx =
-pc_module_sp->GetObjectFile()->GetUnwindTable().GetArmUnwindInfo();
+pc_module_sp->GetUnwindTable().GetArmUnwindInfo();
 if (arm_exidx) {
   unwind_plan_sp = std::make_shared(lldb::eRegisterKindGeneric);
   if (arm_exidx->GetUnwindPlan(exe_ctx.GetTargetRef(), m_current_pc,
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2862,8 +2862,8 @@
   }
 }
 
-DWARFCallFrameInfo *eh_frame = GetUnwindTable().GetEHFrameInfo();
-if (eh_frame) {
+if (DWARFCallFrameInfo *eh_frame =
+GetModule()->GetUnwindTable().GetEHFrameInfo()) {
   if (m_symtab_up == nullptr)
 m_symtab_up.reset(new Symtab(this));
   ParseUnwindSymbols(m_symtab_up.get(), eh_frame);
Index: lldb/trunk/source/Commands/CommandObjectTarget.cpp
===
--- lldb/trunk/source/Commands/CommandObjectTarget.cpp
+++ lldb/trunk/source/Commands/CommandObjectTarget.cpp
@@ -3533,8 +3533,7 @@
 start_addr = abi->FixCodeAddress(start_addr);
 
   FuncUnwindersSP func_unwinders_sp(
-  sc.module_sp->GetObjectFile()
-  ->GetUnwindTable()
+  sc.module_sp->GetUnwindTable()
   .GetUncachedFuncUnwindersContainingAddress(start_addr, sc));
   if (!func_unwinders_sp)
 continue;
Index: lldb/trunk/source/Symbol/ObjectFile.cpp
===
--- lldb/trunk/source/Symbol/ObjectFile.cpp
+++ lldb/trunk/source/Symbol/ObjectFile.cpp
@@ -263,8 +263,7 @@
 : ModuleChild(module_sp),
   m_file(), // This file could be different from the original module's file
   m_type(eTypeInvalid), m_strata(eStrataInvalid),
-  m_file_offset(file_offset), m_length(length), m_data(),
-  m_unwind_table(*this), m_process_wp(),
+  m_file_offset(file_of

[Lldb-commits] [PATCH] D58235: Don't source local .lldbinit in the test suite

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: labath.
Herald added subscribers: lldb-commits, abidh.
Herald added a reviewer: serge-sans-paille.
Herald added a project: LLDB.

As suggested by Pavel, we shouldn't let our tests parse the local .lldbinit to 
prevent random test failures
due to changed settings.

Fixes Minidump/Windows/Sigsegv/sigsegv.test (and probably others too).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D58235

Files:
  lldb/lit/helper/toolchain.py


Index: lldb/lit/helper/toolchain.py
===
--- lldb/lit/helper/toolchain.py
+++ lldb/lit/helper/toolchain.py
@@ -18,7 +18,7 @@
 dsargs = [] if platform.system() in ['Darwin'] else ['gdbserver']
 lldbmi = ToolSubst('%lldbmi',
command=FindTool('lldb-mi'),
-   extra_args=['--synchronous'],
+   extra_args=['--synchronous', '--no-lldbinit'],
unresolved='ignore')
 
 


Index: lldb/lit/helper/toolchain.py
===
--- lldb/lit/helper/toolchain.py
+++ lldb/lit/helper/toolchain.py
@@ -18,7 +18,7 @@
 dsargs = [] if platform.system() in ['Darwin'] else ['gdbserver']
 lldbmi = ToolSubst('%lldbmi',
command=FindTool('lldb-mi'),
-   extra_args=['--synchronous'],
+   extra_args=['--synchronous', '--no-lldbinit'],
unresolved='ignore')
 
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58235: Don't source local .lldbinit in the test suite

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Are you sure you're adding this to the right place? This is modifying the 
substitution for lldb-mi and not lldb..


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58235



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

[Lldb-commits] [PATCH] D58230: [lldb] [MainLoop] Add kevent() EINTR handling

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

For EINTR we shall use `llvm::sys::RetryAfterSignal`


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58230



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

[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.
Herald added a project: LLVM.

'attaching a debugger produces an observable side-effect (EINTR) in the 
debugged process is considered a bug by the linux kernel folks'

There are differences between Linux and BSD.

On BSD we try hard to make return of such syscall peacefully valid input (like 
returning already received bytes int read(2)), while on Linux there is hard 
interruption of operation.

Also we can introduce EINTR easily on BSD in arbitrary moments with ctrl-T 
(SIGINFO) that is used in many programs to pass information and the unprepared 
ones can misbehave due to EINTR.

So even if this could be a bug to cause EINTR in some scenarios on BSD under a 
debugger, it's a program fault to not handle it.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D42206



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

[Lldb-commits] [PATCH] D58227: [lldb] [MainLoop] Remove redundant termination clause (NFCI)

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

It looks good to me.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58227



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

[Lldb-commits] [PATCH] D58235: Don't source local .lldbinit in the test suite

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 186850.
teemperor added a comment.

Ah yes, that also explains why the test suit failed. Fixed and the test now 
passes, thanks!


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

https://reviews.llvm.org/D58235

Files:
  lldb/lit/helper/toolchain.py


Index: lldb/lit/helper/toolchain.py
===
--- lldb/lit/helper/toolchain.py
+++ lldb/lit/helper/toolchain.py
@@ -35,7 +35,7 @@
 primary_tools = [
 ToolSubst('%lldb',
   command=FindTool('lldb'),
-  extra_args=['-S',
+  extra_args=['--no-lldbinit', '-S',
   os.path.join(config.test_source_root,
'lit-lldb-init')]),
 lldbmi,


Index: lldb/lit/helper/toolchain.py
===
--- lldb/lit/helper/toolchain.py
+++ lldb/lit/helper/toolchain.py
@@ -35,7 +35,7 @@
 primary_tools = [
 ToolSubst('%lldb',
   command=FindTool('lldb'),
-  extra_args=['-S',
+  extra_args=['--no-lldbinit', '-S',
   os.path.join(config.test_source_root,
'lit-lldb-init')]),
 lldbmi,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58227: [lldb] [MainLoop] Remove redundant termination clause (NFCI)

[Pavel Labath via Phabricator via lldb-commits]

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

That sounds correct. The extra check must have crept in because of all the 
refactors this class has gone through to support all possible event handling 
mechanisms...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58227



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

[Lldb-commits] [PATCH] D58235: Don't source local .lldbinit in the test suite

[Pavel Labath via Phabricator via lldb-commits]

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

lgtm


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

https://reviews.llvm.org/D58235



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

[Lldb-commits] [lldb] r354037 - [CMake] Fix RPATH handling for LLDB.framework

[Stefan Granitz via lldb-commits]

Author: stefan.graenitz
Date: Thu Feb 14 09:34:39 2019
New Revision: 354037

URL: http://llvm.org/viewvc/llvm-project?rev=354037&view=rev
Log:
[CMake] Fix RPATH handling for LLDB.framework

Summary:
Generator expressions are not supported in the `BUILD_RPATH` target property.
`BUILD_RPATH` is only supported in 3.8+ 
https://cliutils.gitlab.io/modern-cmake/chapters/intro/newcmake.html
`LLDB_FRAMEWORK_INSTALL_DIR` should not overwrite, but rather add an install 
RPATH (and it should be the first)

Reviewers: xiaobai, lanza

Reviewed By: xiaobai

Subscribers: mgorny

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

Modified:
lldb/trunk/cmake/modules/AddLLDB.cmake
lldb/trunk/cmake/modules/LLDBConfig.cmake

Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=354037&r1=354036&r2=354037&view=diff
==
--- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
+++ lldb/trunk/cmake/modules/AddLLDB.cmake Thu Feb 14 09:34:39 2019
@@ -175,23 +175,25 @@ endfunction()
 # added as an extra RPATH below.
 #
 function(lldb_setup_framework_rpaths_in_tool name)
-  # In the build-tree, we know the exact path to the binary in the framework.
-  set(rpath_build_tree "$")
-
   # The installed framework is relocatable and can be in different locations.
-  set(rpaths_install_tree "@loader_path/../../../SharedFrameworks")
-  list(APPEND rpaths_install_tree 
"@loader_path/../../System/Library/PrivateFrameworks")
-  list(APPEND rpaths_install_tree 
"@loader_path/../../Library/PrivateFrameworks")
+  set(rpaths_install_tree)
 
   if(LLDB_FRAMEWORK_INSTALL_DIR)
-set(rpaths_install_tree "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
+list(APPEND rpaths_install_tree 
"@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
   endif()
 
+  list(APPEND rpaths_install_tree "@loader_path/../../../SharedFrameworks")
+  list(APPEND rpaths_install_tree 
"@loader_path/../../System/Library/PrivateFrameworks")
+  list(APPEND rpaths_install_tree 
"@loader_path/../../Library/PrivateFrameworks")
+
+  # In the build-tree, we know the exact path to the framework directory.
+  get_target_property(framework_target_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
+
   # If LLDB_NO_INSTALL_DEFAULT_RPATH was NOT enabled (default), this overwrites
   # the default settings from llvm_setup_rpath().
   set_target_properties(${name} PROPERTIES
 BUILD_WITH_INSTALL_RPATH OFF
-BUILD_RPATH "${rpath_build_tree}"
+BUILD_RPATH "${framework_target_dir}"
 INSTALL_RPATH "${rpaths_install_tree}"
   )
 

Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=354037&r1=354036&r2=354037&view=diff
==
--- lldb/trunk/cmake/modules/LLDBConfig.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake Thu Feb 14 09:34:39 2019
@@ -55,8 +55,9 @@ if(LLDB_BUILD_FRAMEWORK)
 message(FATAL_ERROR "LLDB.framework can only be generated when targeting 
Apple platforms")
   endif()
   # CMake 3.6 did not correctly emit POST_BUILD commands for Apple Framework 
targets
-  if(CMAKE_VERSION VERSION_LESS 3.7)
-message(FATAL_ERROR "LLDB_BUILD_FRAMEWORK is not supported on CMake < 3.7")
+  # CMake < 3.8 did not have the BUILD_RPATH target property
+  if(CMAKE_VERSION VERSION_LESS 3.8)
+message(FATAL_ERROR "LLDB_BUILD_FRAMEWORK is not supported on CMake < 3.8")
   endif()
 
   set(LLDB_FRAMEWORK_VERSION A CACHE STRING "LLDB.framework version (default 
is A)")


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

[Lldb-commits] [PATCH] D57989: [CMake] Fix RPATH handling for LLDB.framework

[Stefan Gränitz via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdb85fdd11595: [CMake] Fix RPATH handling for LLDB.framework 
(authored by sgraenitz).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57989?vs=186634&id=186861#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57989

Files:
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -55,8 +55,9 @@
 message(FATAL_ERROR "LLDB.framework can only be generated when targeting 
Apple platforms")
   endif()
   # CMake 3.6 did not correctly emit POST_BUILD commands for Apple Framework 
targets
-  if(CMAKE_VERSION VERSION_LESS 3.7)
-message(FATAL_ERROR "LLDB_BUILD_FRAMEWORK is not supported on CMake < 3.7")
+  # CMake < 3.8 did not have the BUILD_RPATH target property
+  if(CMAKE_VERSION VERSION_LESS 3.8)
+message(FATAL_ERROR "LLDB_BUILD_FRAMEWORK is not supported on CMake < 3.8")
   endif()
 
   set(LLDB_FRAMEWORK_VERSION A CACHE STRING "LLDB.framework version (default 
is A)")
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -175,23 +175,25 @@
 # added as an extra RPATH below.
 #
 function(lldb_setup_framework_rpaths_in_tool name)
-  # In the build-tree, we know the exact path to the binary in the framework.
-  set(rpath_build_tree "$")
-
   # The installed framework is relocatable and can be in different locations.
-  set(rpaths_install_tree "@loader_path/../../../SharedFrameworks")
-  list(APPEND rpaths_install_tree 
"@loader_path/../../System/Library/PrivateFrameworks")
-  list(APPEND rpaths_install_tree 
"@loader_path/../../Library/PrivateFrameworks")
+  set(rpaths_install_tree)
 
   if(LLDB_FRAMEWORK_INSTALL_DIR)
-set(rpaths_install_tree "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
+list(APPEND rpaths_install_tree 
"@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
   endif()
 
+  list(APPEND rpaths_install_tree "@loader_path/../../../SharedFrameworks")
+  list(APPEND rpaths_install_tree 
"@loader_path/../../System/Library/PrivateFrameworks")
+  list(APPEND rpaths_install_tree 
"@loader_path/../../Library/PrivateFrameworks")
+
+  # In the build-tree, we know the exact path to the framework directory.
+  get_target_property(framework_target_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
+
   # If LLDB_NO_INSTALL_DEFAULT_RPATH was NOT enabled (default), this overwrites
   # the default settings from llvm_setup_rpath().
   set_target_properties(${name} PROPERTIES
 BUILD_WITH_INSTALL_RPATH OFF
-BUILD_RPATH "${rpath_build_tree}"
+BUILD_RPATH "${framework_target_dir}"
 INSTALL_RPATH "${rpaths_install_tree}"
   )
 


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -55,8 +55,9 @@
 message(FATAL_ERROR "LLDB.framework can only be generated when targeting Apple platforms")
   endif()
   # CMake 3.6 did not correctly emit POST_BUILD commands for Apple Framework targets
-  if(CMAKE_VERSION VERSION_LESS 3.7)
-message(FATAL_ERROR "LLDB_BUILD_FRAMEWORK is not supported on CMake < 3.7")
+  # CMake < 3.8 did not have the BUILD_RPATH target property
+  if(CMAKE_VERSION VERSION_LESS 3.8)
+message(FATAL_ERROR "LLDB_BUILD_FRAMEWORK is not supported on CMake < 3.8")
   endif()
 
   set(LLDB_FRAMEWORK_VERSION A CACHE STRING "LLDB.framework version (default is A)")
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -175,23 +175,25 @@
 # added as an extra RPATH below.
 #
 function(lldb_setup_framework_rpaths_in_tool name)
-  # In the build-tree, we know the exact path to the binary in the framework.
-  set(rpath_build_tree "$")
-
   # The installed framework is relocatable and can be in different locations.
-  set(rpaths_install_tree "@loader_path/../../../SharedFrameworks")
-  list(APPEND rpaths_install_tree "@loader_path/../../System/Library/PrivateFrameworks")
-  list(APPEND rpaths_install_tree "@loader_path/../../Library/PrivateFrameworks")
+  set(rpaths_install_tree)
 
   if(LLDB_FRAMEWORK_INSTALL_DIR)
-set(rpaths_install_tree "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
+list(APPEND rpaths_install_tree "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
   endif()
 
+  list(APPEND rpaths_install_tree "@loader_path/../../../SharedFrameworks")
+  list(APPEND rpaths_install_tree "@loader_path/../../System/Library/Priva

[Lldb-commits] [lldb] r354038 - Don't source local .lldbinit in the test suite

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Thu Feb 14 09:39:19 2019
New Revision: 354038

URL: http://llvm.org/viewvc/llvm-project?rev=354038&view=rev
Log:
Don't source local .lldbinit in the test suite

Summary:
As suggested by Pavel, we shouldn't let our tests parse the local .lldbinit to 
prevent random test failures
due to changed settings.

Fixes Minidump/Windows/Sigsegv/sigsegv.test (and probably others too).

Reviewers: labath, serge-sans-paille

Reviewed By: labath

Subscribers: abidh, lldb-commits, zturner

Tags: #lldb

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

Modified:
lldb/trunk/lit/helper/toolchain.py

Modified: lldb/trunk/lit/helper/toolchain.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/helper/toolchain.py?rev=354038&r1=354037&r2=354038&view=diff
==
--- lldb/trunk/lit/helper/toolchain.py (original)
+++ lldb/trunk/lit/helper/toolchain.py Thu Feb 14 09:39:19 2019
@@ -35,7 +35,7 @@ def use_lldb_substitutions(config):
 primary_tools = [
 ToolSubst('%lldb',
   command=FindTool('lldb'),
-  extra_args=['-S',
+  extra_args=['--no-lldbinit', '-S',
   os.path.join(config.test_source_root,
'lit-lldb-init')]),
 lldbmi,


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

[Lldb-commits] [PATCH] D58235: Don't source local .lldbinit in the test suite

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB354038: Don't source local .lldbinit in the test 
suite (authored by teemperor, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58235?vs=186850&id=186865#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58235

Files:
  lit/helper/toolchain.py


Index: lit/helper/toolchain.py
===
--- lit/helper/toolchain.py
+++ lit/helper/toolchain.py
@@ -35,7 +35,7 @@
 primary_tools = [
 ToolSubst('%lldb',
   command=FindTool('lldb'),
-  extra_args=['-S',
+  extra_args=['--no-lldbinit', '-S',
   os.path.join(config.test_source_root,
'lit-lldb-init')]),
 lldbmi,


Index: lit/helper/toolchain.py
===
--- lit/helper/toolchain.py
+++ lit/helper/toolchain.py
@@ -35,7 +35,7 @@
 primary_tools = [
 ToolSubst('%lldb',
   command=FindTool('lldb'),
-  extra_args=['-S',
+  extra_args=['--no-lldbinit', '-S',
   os.path.join(config.test_source_root,
'lit-lldb-init')]),
 lldbmi,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58177: Fix lldb-server test suite for python3

[Stella Stamenova via Phabricator via lldb-commits]

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

In D58177#1397873 , @labath wrote:

> Thanks for the review Stella. I was hoping someone would step in and tell me 
> that there is a better way to do that. However :), I see two problems with 
> your proposal:
>
> - build.py is a standalone file, and so it is not easy for it to share code 
> with other stuff. This can be solved with some use_lldb_suite magic or moving 
> the script some place else, but then comes the second problem:
> - I am not sure these two use cases are actually compatible. Here I 
> specifically want to "reinterpret_cast" binary bytes without doing any sort 
> of encoding. The only place where `to_string` is used in `build.py` currently 
> is on a value retrieved from the windows registry. I'm not 100% sure, but it 
> seems to me that using utf8 encoding (which is what `to_string` is doing) is 
> precisely the right thing to do there.
>
>   With that in mind, I propose to do the following:
> - leave build.py alone for now
> - rename the new functions I'm introducing here to something less generic. 
> Maybe `bitcast_to_string` and `bitcast_to_bytes`?
>
>   WDYT?


Thanks, Pavel. That sounds good and it will make it clear that they two 
functions are distinct.


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

https://reviews.llvm.org/D58177



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

[Lldb-commits] [PATCH] D58222: [ClangExpressionParser] Reuse the FileManager from the compiler instance.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB354041: [ExpressionParser] Reuse the FileManager from the 
compiler instance. (authored by JDevlieghere, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58222?vs=186794&id=186866#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58222

Files:
  source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h


Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -11,7 +11,6 @@
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/DiagnosticIDs.h"
-#include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
@@ -490,14 +489,9 @@
   m_compiler->getDiagnostics().setClient(new ClangDiagnosticManagerAdapter);
 
   // 7. Set up the source management objects inside the compiler
-
-  clang::FileSystemOptions file_system_options;
-  m_file_manager.reset(new clang::FileManager(file_system_options));
-
-  if (!m_compiler->hasSourceManager())
-m_compiler->createSourceManager(*m_file_manager);
-
   m_compiler->createFileManager();
+  if (!m_compiler->hasSourceManager())
+m_compiler->createSourceManager(m_compiler->getFileManager());
   m_compiler->createPreprocessor(TU_Complete);
 
   if (ClangModulesDeclVendor *decl_vendor =
@@ -855,9 +849,9 @@
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(
-  source_mgr.createFileID(m_file_manager->getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
+  source_mgr.setMainFileID(source_mgr.createFileID(
+  m_compiler->getFileManager().getFile(result_path),
+  SourceLocation(), SrcMgr::C_User));
   created_main_file = true;
 }
   }
Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
@@ -178,8 +178,6 @@
 
   std::unique_ptr
   m_llvm_context; ///< The LLVM context to generate IR into
-  std::unique_ptr
-  m_file_manager; ///< The Clang file manager object used by the compiler
   std::unique_ptr
   m_compiler; ///< The Clang compiler used to parse expressions into IR
   std::unique_ptr


Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -11,7 +11,6 @@
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/DiagnosticIDs.h"
-#include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
@@ -490,14 +489,9 @@
   m_compiler->getDiagnostics().setClient(new ClangDiagnosticManagerAdapter);
 
   // 7. Set up the source management objects inside the compiler
-
-  clang::FileSystemOptions file_system_options;
-  m_file_manager.reset(new clang::FileManager(file_system_options));
-
-  if (!m_compiler->hasSourceManager())
-m_compiler->createSourceManager(*m_file_manager);
-
   m_compiler->createFileManager();
+  if (!m_compiler->hasSourceManager())
+m_compiler->createSourceManager(m_compiler->getFileManager());
   m_compiler->createPreprocessor(TU_Complete);
 
   if (ClangModulesDeclVendor *decl_vendor =
@@ -855,9 +849,9 @@
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(
-  source_mgr.createFileID(m_file_manager->getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
+  source_mgr.setMainFileID(source_mgr.createFileID(
+  m_compiler->getFileManager().getFile(result_path),
+  SourceLocation(), SrcMgr::C_User));
   created_main_file = true;
 }
   }
Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
@@ -178,8 +178,6 @@
 
   std::unique_ptr
 

[Lldb-commits] [lldb] r354041 - [ExpressionParser] Reuse the FileManager from the compiler instance.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu Feb 14 10:05:43 2019
New Revision: 354041

URL: http://llvm.org/viewvc/llvm-project?rev=354041&view=rev
Log:
[ExpressionParser] Reuse the FileManager from the compiler instance.

I was looking at the ClangExpressionParser and noticed that we have a
FileManager owned by the expression parser and later ask the compiler
instance to create a new FileManager, owned by the clang CI. Looking at
the code I don't see a good reason for having two instances. This patch
removes the one owned by LLDB.

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

Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h

Modified: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=354041&r1=354040&r2=354041&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
Thu Feb 14 10:05:43 2019
@@ -11,7 +11,6 @@
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/DiagnosticIDs.h"
-#include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
@@ -490,14 +489,9 @@ ClangExpressionParser::ClangExpressionPa
   m_compiler->getDiagnostics().setClient(new ClangDiagnosticManagerAdapter);
 
   // 7. Set up the source management objects inside the compiler
-
-  clang::FileSystemOptions file_system_options;
-  m_file_manager.reset(new clang::FileManager(file_system_options));
-
-  if (!m_compiler->hasSourceManager())
-m_compiler->createSourceManager(*m_file_manager);
-
   m_compiler->createFileManager();
+  if (!m_compiler->hasSourceManager())
+m_compiler->createSourceManager(m_compiler->getFileManager());
   m_compiler->createPreprocessor(TU_Complete);
 
   if (ClangModulesDeclVendor *decl_vendor =
@@ -855,9 +849,9 @@ ClangExpressionParser::ParseInternal(Dia
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(
-  source_mgr.createFileID(m_file_manager->getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
+  source_mgr.setMainFileID(source_mgr.createFileID(
+  m_compiler->getFileManager().getFile(result_path),
+  SourceLocation(), SrcMgr::C_User));
   created_main_file = true;
 }
   }

Modified: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h?rev=354041&r1=354040&r2=354041&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h 
Thu Feb 14 10:05:43 2019
@@ -178,8 +178,6 @@ private:
 
   std::unique_ptr
   m_llvm_context; ///< The LLVM context to generate IR into
-  std::unique_ptr
-  m_file_manager; ///< The Clang file manager object used by the compiler
   std::unique_ptr
   m_compiler; ///< The Clang compiler used to parse expressions into IR
   std::unique_ptr


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

[Lldb-commits] [PATCH] D57964: Fix potential UB when target_file directory is null

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.
Herald added a subscriber: jdoerfert.

Ping


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

https://reviews.llvm.org/D57964



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

[Lldb-commits] [PATCH] D58193: Do not explicitly depend on llvm tools during standalone build

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

All these targets are exported from the LLVM build-tree since D56606 
 and from the install-tree since D57383 
.

I did not revert or continue to work on D57233 
 yet, because it needs a good concept first. 
`LLDB_BUILT_STANDALONE` is not the right condition here. What we really want to 
know is:

- Have LLVM tools/utils been built (`LLVM_BUILD_TOOLS/UTILS`) when we have a 
build-tree.
- Have LLVM tools/utils been installed (`LLVM_INSTALL_TOOLS/UTILS`) when we 
have an install-tree.
- What tree do we have? I have seen hacks to determine it, but not sure whether 
there's a clean way.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58193



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

[Lldb-commits] [PATCH] D58193: Do not explicitly depend on llvm tools during standalone build

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

@xiaobai && @compnerd Do you know if `find_package` can tell us anything about 
it?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58193



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

[Lldb-commits] [PATCH] D58172: Embed swig version into lldb.py in a different way

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

Long term, I wonder if we can just delete this entire `modify-python-lldb.py` 
script.  it seems like the entire purpose is to make sure that the SB methods 
support iteration, equality, and some other basic stuff, and it does this by 
inserting lines of python code into the actual `__init__.py` file.  It seems 
like we could just have the swig file say `%pythoncode%{import sb_iteration}`, 
then write a file called `sb_iteration.py` which we ship alongside the 
`__init__.py`, and have it dynamically add all of the things it needs at 
runtime.  Then, we don't even need a a post processing step at all, which seems 
like a definite improvement.

As for this particular change, another option is to just say:

  %pythoncode%{
swig_version = VERSION
  }

There's no real reason we have to do the parsing here, we could do it in our 
dotest.py decorators.  I don't have a strong preference on which is better 
though.


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

https://reviews.llvm.org/D58172



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

[Lldb-commits] [PATCH] D58193: Do not explicitly depend on llvm tools during standalone build

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added a comment.

@sgraenitz - not really ... the `find_package` will load the `LLVMConfig.cmake` 
that is generated.  We only have what we keep in there.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58193



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

[Lldb-commits] [PATCH] D57402: build: remove custom variables

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

I don't want to play the nitpicker here, but did you test this? If so, please 
provide a sample configuration command line.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57402



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

[Lldb-commits] [PATCH] D58172: Embed swig version into lldb.py in a different way

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yes, my long term plan is to try to get rid of this file altogether. Doing the 
hotpatching at runtime is an interesting idea. I may resort to that, but I 
think it would be better to add all of these as regular blocks of inline 
%pythoncode%, as that's how swig is meant to be used. However, i haven't looked 
at that too closely yet. I am trying to dismantle this one step at a time.

In D58172#1398246 , @zturner wrote:

>   %pythoncode%{
> swig_version = VERSION
>   }
>
>
> There's no real reason we have to do the parsing here, we could do it in our 
> dotest.py decorators.  I don't have a strong preference on which is better 
> though.


Originally I thought this was a proper api that we deliberately wanted to 
export, and only later found out that this was added only to support test 
decorators. However, this gets exported as a public api nonetheless, and it 
sounds like the kind of thing that might be useful to somebody, so I think it's 
good to spend a bit of effort to present it in a nicer format. Besides, we're 
going to need to put the funny parsing code somewhere anyway, and putting it 
close to the source means we won't have to parse it twice.


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

https://reviews.llvm.org/D58172



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

[Lldb-commits] [lldb] r354047 - [dotest] Fix compiler version number comparison

[Frederic Riss via lldb-commits]

Author: friss
Date: Thu Feb 14 10:48:05 2019
New Revision: 354047

URL: http://llvm.org/viewvc/llvm-project?rev=354047&view=rev
Log:
[dotest] Fix compiler version number comparison

dotest's version comparision function is just a lexicographical compare
of the version strings. This is obviously wrong. This patch implements
the comparison using distutils.version.LooseVersion as suggested by
Zachary.

Reviewers: zturner, labath, serge-sans-paille

Subscribers: jdoerfert, lldb-commits

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

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py?rev=354047&r1=354046&r2=354047&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py Thu Feb 14 10:48:05 
2019
@@ -37,6 +37,7 @@ from __future__ import print_function
 # System modules
 import abc
 import collections
+from distutils.version import LooseVersion
 from functools import wraps
 import gc
 import glob
@@ -1352,13 +1353,13 @@ class Base(unittest2.TestCase):
 if (version is None):
 return True
 if (operator == '>'):
-return self.getCompilerVersion() > version
+return LooseVersion(self.getCompilerVersion()) > 
LooseVersion(version)
 if (operator == '>=' or operator == '=>'):
-return self.getCompilerVersion() >= version
+return LooseVersion(self.getCompilerVersion()) >= 
LooseVersion(version)
 if (operator == '<'):
-return self.getCompilerVersion() < version
+return LooseVersion(self.getCompilerVersion()) < 
LooseVersion(version)
 if (operator == '<=' or operator == '=<'):
-return self.getCompilerVersion() <= version
+return LooseVersion(self.getCompilerVersion()) <= 
LooseVersion(version)
 if (operator == '!=' or operator == '!' or operator == 'not'):
 return str(version) not in str(self.getCompilerVersion())
 return str(version) in str(self.getCompilerVersion())


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

[Lldb-commits] [lldb] r354048 - Add explicit language specifier to test.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Thu Feb 14 10:49:14 2019
New Revision: 354048

URL: http://llvm.org/viewvc/llvm-project?rev=354048&view=rev
Log:
Add explicit language specifier to test.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/modules-import/TestCXXModulesImport.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/modules-import/TestCXXModulesImport.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/modules-import/TestCXXModulesImport.py?rev=354048&r1=354047&r2=354048&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/modules-import/TestCXXModulesImport.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/modules-import/TestCXXModulesImport.py
 Thu Feb 14 10:49:14 2019
@@ -27,5 +27,5 @@ class CXXModulesImportTestCase(TestBase)
 target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
 self, 'break here', lldb.SBFileSpec('main.cpp'))
 
-self.expect("expr -- @import Bar")
+self.expect("expr -l Objective-C++ -- @import Bar")
 self.expect("expr -- Bar()", substrs = ["success"])


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

[Lldb-commits] [lldb] r354050 - [lldb] [MainLoop] Remove redundant termination clause (NFCI)

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Thu Feb 14 10:51:21 2019
New Revision: 354050

URL: http://llvm.org/viewvc/llvm-project?rev=354050&view=rev
Log:
[lldb] [MainLoop] Remove redundant termination clause (NFCI)

Remove the redundant termination clause from within the loop.  Since
the check is done at the end of the loop, it's entirely redundant
to the 'while' condition.  If termination was requested, the latter
will become false and the 'while' loop will terminate, resulting
in the 'return' statement below the loop being executed (which is
equivalent to the one used inside 'if').

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

Modified:
lldb/trunk/source/Host/common/MainLoop.cpp

Modified: lldb/trunk/source/Host/common/MainLoop.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/MainLoop.cpp?rev=354050&r1=354049&r2=354050&view=diff
==
--- lldb/trunk/source/Host/common/MainLoop.cpp (original)
+++ lldb/trunk/source/Host/common/MainLoop.cpp Thu Feb 14 10:51:21 2019
@@ -382,9 +382,6 @@ Status MainLoop::Run() {
   return error;
 
 impl.ProcessEvents();
-
-if (m_terminate_request)
-  return Status();
   }
   return Status();
 }


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

[Lldb-commits] [PATCH] D57964: Fix potential UB when target_file directory is null

[Jim Ingham via Phabricator via lldb-commits]

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

That code seems amazingly good to me ;-)


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

https://reviews.llvm.org/D57964



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

[Lldb-commits] [PATCH] D58219: [dotest] Fix compiler version number comparison

[Frederic Riss via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354047: [dotest] Fix compiler version number comparison 
(authored by friss, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58219?vs=186782&id=186873#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58219

Files:
  lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py


Index: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
@@ -37,6 +37,7 @@
 # System modules
 import abc
 import collections
+from distutils.version import LooseVersion
 from functools import wraps
 import gc
 import glob
@@ -1352,13 +1353,13 @@
 if (version is None):
 return True
 if (operator == '>'):
-return self.getCompilerVersion() > version
+return LooseVersion(self.getCompilerVersion()) > 
LooseVersion(version)
 if (operator == '>=' or operator == '=>'):
-return self.getCompilerVersion() >= version
+return LooseVersion(self.getCompilerVersion()) >= 
LooseVersion(version)
 if (operator == '<'):
-return self.getCompilerVersion() < version
+return LooseVersion(self.getCompilerVersion()) < 
LooseVersion(version)
 if (operator == '<=' or operator == '=<'):
-return self.getCompilerVersion() <= version
+return LooseVersion(self.getCompilerVersion()) <= 
LooseVersion(version)
 if (operator == '!=' or operator == '!' or operator == 'not'):
 return str(version) not in str(self.getCompilerVersion())
 return str(version) in str(self.getCompilerVersion())


Index: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
@@ -37,6 +37,7 @@
 # System modules
 import abc
 import collections
+from distutils.version import LooseVersion
 from functools import wraps
 import gc
 import glob
@@ -1352,13 +1353,13 @@
 if (version is None):
 return True
 if (operator == '>'):
-return self.getCompilerVersion() > version
+return LooseVersion(self.getCompilerVersion()) > LooseVersion(version)
 if (operator == '>=' or operator == '=>'):
-return self.getCompilerVersion() >= version
+return LooseVersion(self.getCompilerVersion()) >= LooseVersion(version)
 if (operator == '<'):
-return self.getCompilerVersion() < version
+return LooseVersion(self.getCompilerVersion()) < LooseVersion(version)
 if (operator == '<=' or operator == '=<'):
-return self.getCompilerVersion() <= version
+return LooseVersion(self.getCompilerVersion()) <= LooseVersion(version)
 if (operator == '!=' or operator == '!' or operator == 'not'):
 return str(version) not in str(self.getCompilerVersion())
 return str(version) in str(self.getCompilerVersion())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58227: [lldb] [MainLoop] Remove redundant termination clause (NFCI)

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354050: [lldb] [MainLoop] Remove redundant termination 
clause (NFCI) (authored by mgorny, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58227?vs=186806&id=186875#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58227

Files:
  lldb/trunk/source/Host/common/MainLoop.cpp


Index: lldb/trunk/source/Host/common/MainLoop.cpp
===
--- lldb/trunk/source/Host/common/MainLoop.cpp
+++ lldb/trunk/source/Host/common/MainLoop.cpp
@@ -382,9 +382,6 @@
   return error;
 
 impl.ProcessEvents();
-
-if (m_terminate_request)
-  return Status();
   }
   return Status();
 }


Index: lldb/trunk/source/Host/common/MainLoop.cpp
===
--- lldb/trunk/source/Host/common/MainLoop.cpp
+++ lldb/trunk/source/Host/common/MainLoop.cpp
@@ -382,9 +382,6 @@
   return error;
 
 impl.ProcessEvents();
-
-if (m_terminate_request)
-  return Status();
   }
   return Status();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58193: Do not explicitly depend on llvm tools during standalone build

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

@sgraenitz: I don't think `find_package` alone will help you here. LLVMConfig 
doesn't necessarily tell you what's actually been built and/or installed, but 
it can tell you what's exported. Here's what I would do:

1. List the tools you need for `LLDB_TEST_DEPS` from LLVM
2. For each tool, check to see if it's exported from the LLVM CMake package 
(this is as simple as checking if it's a target)
3. If it's not exported, proceed to the next tool
4. If it is exported, use `find_program` to check if it was built/installed 
(LLVMConfig should tell you where it is -- `LLVM_TOOLS_BINARY_DIR`)
5. If it is built, add it to `LLDB_TEST_DEPS`.
6. If it's not built, don't add it to `LLDB_TEST_DEPS`.

I think this should work regardless of if you have a build tree or an install 
tree. I don't see any logic in here that is specific to one or the other, but 
if you absolutely do need to check, probably the most reliable way is to check 
for the existince of the `LLVM_BUILD_*` variables. I think the install tree 
also has a variable `LLVM_INSTALL_PREFIX`, so that could be another way to 
check.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58193



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

[Lldb-commits] [PATCH] D58167: Refactor user/group name resolving code

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I didn't get around to address the style issues yet, but I wanted to respond to 
one high-level comment today:

> So we are currently only using the platform for the user ID resolving, but it 
> might be nice to keep the platform as the argument in case we need it for 
> something else in the future?

Doing this would defeat the purpose of this patch, as the `ProcessInstanceInfo` 
class would no longer be standalone. I don't think it's very likely that we'll 
need other platform capabilities to decode this class, but in case we do, I see 
two options:

- hide the details of this new thing behind an interface. This could either be 
a new interface, or an extension of the user id resolver interface, depending 
on what the thing is.
- Move the Dumping code out of the ProcessInstanceInfo class. If interpreting 
the ProcessInstanceInfo data is so complicated that we need access to full 
platform class, then I don't think the ProcessInstanceInfo is the right place 
for that code. In this case, we could move the code to the platform class 
(`platform->Dump(stream, info)`), or a completely separate class altogether 
(like we did with the DumpDataExtractor classes).


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

https://reviews.llvm.org/D58167



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

[Lldb-commits] [PATCH] D58193: Do not explicitly depend on llvm tools during standalone build

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

BTW, what's the reason for having super-high-fidelity list of dependencies in a 
standalone build? I know that they're important for in-tree builds, as that 
makes sure everything is built when you run "check-lldb". However, that's isn't 
going to help you with standalone builds. The binaries are already there, or 
they aren't, and there's nothing we can do to change that. I'd consider just 
removing all non-lldb targets from the deps list when in standalone mode.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58193



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

[Lldb-commits] [PATCH] D55653: Check pointer results on nullptr before using them

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha updated this revision to Diff 186892.
tatyana-krasnukha added a comment.

Fixed misspelling


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

https://reviews.llvm.org/D55653

Files:
  MICmdCmdGdbInfo.cpp
  MICmdCmdMiscellanous.cpp
  MICmnLLDBDebugSessionInfo.cpp
  MIDataTypes.h

Index: MIDataTypes.h
===
--- MIDataTypes.h
+++ MIDataTypes.h
@@ -62,3 +62,18 @@
 // Fundamentals:
 typedef long long MIint64;   // 64bit signed integer.
 typedef unsigned long long MIuint64; // 64bit unsigned integer.
+
+template 
+class MIDereferenceable {
+public:
+  MIDereferenceable(T *ptr) : m_ptr(ptr) {}
+
+  T *Or(T &alt_value) { return m_ptr ? m_ptr : &alt_value; }
+private:
+  T* m_ptr = nullptr;
+};
+
+template
+MIDereferenceable GetDereferenceable(T *ptr) {
+  return MIDereferenceable(ptr);
+}
Index: MICmnLLDBDebugSessionInfo.cpp
===
--- MICmnLLDBDebugSessionInfo.cpp
+++ MICmnLLDBDebugSessionInfo.cpp
@@ -639,11 +639,10 @@
 
   vwPc = rFrame.GetPC();
 
-  const char *pFnName = rFrame.GetFunctionName();
-  vwFnName = (pFnName != nullptr) ? pFnName : pUnkwn;
+  vwFnName = GetDereferenceable(rFrame.GetFunctionName()).Or(*pUnkwn);
 
-  const char *pFileName = rFrame.GetLineEntry().GetFileSpec().GetFilename();
-  vwFileName = (pFileName != nullptr) ? pFileName : pUnkwn;
+  vwFileName = GetDereferenceable(
+  rFrame.GetLineEntry().GetFileSpec().GetFilename()).Or(*pUnkwn);
 
   vwnLine = rFrame.GetLineEntry().GetLine();
 
@@ -829,9 +828,9 @@
   vrwBrkPtInfo.m_id = vBrkPt.GetID();
   vrwBrkPtInfo.m_strType = "breakpoint";
   vrwBrkPtInfo.m_pc = nAddr;
-  vrwBrkPtInfo.m_fnName = pFn;
-  vrwBrkPtInfo.m_fileName = pFile;
-  vrwBrkPtInfo.m_path = pFilePath;
+  vrwBrkPtInfo.m_fnName = GetDereferenceable(pFn).Or(*pUnkwn);
+  vrwBrkPtInfo.m_fileName = GetDereferenceable(pFile).Or(*pUnkwn);
+  vrwBrkPtInfo.m_path = GetDereferenceable(pFilePath).Or(*pUnkwn);
   vrwBrkPtInfo.m_nLine = nLine;
   vrwBrkPtInfo.m_nTimes = vBrkPt.GetHitCount();
 
Index: MICmdCmdMiscellanous.cpp
===
--- MICmdCmdMiscellanous.cpp
+++ MICmdCmdMiscellanous.cpp
@@ -336,9 +336,12 @@
 }
 
 if (rSessionInfo.GetTarget().IsValid()) {
+  const char *pUnkwn = "??";
   lldb::SBTarget sbTrgt = rSessionInfo.GetTarget();
-  const char *pDir = sbTrgt.GetExecutable().GetDirectory();
-  const char *pFileName = sbTrgt.GetExecutable().GetFilename();
+  const char *pDir =
+  GetDereferenceable(sbTrgt.GetExecutable().GetDirectory()).Or(*pUnkwn);
+  const char *pFileName =
+  GetDereferenceable(sbTrgt.GetExecutable().GetFilename()).Or(*pUnkwn);
   const CMIUtilString strFile(
   CMIUtilString::Format("%s/%s", pDir, pFileName));
   const CMICmnMIValueConst miValueConst4(strFile);
Index: MICmdCmdGdbInfo.cpp
===
--- MICmdCmdGdbInfo.cpp
+++ MICmdCmdGdbInfo.cpp
@@ -192,6 +192,8 @@
   bool bOk = CMICmnStreamStdout::TextToStdout(
   "~\"FromTo  Syms Read   Shared Object Library\"");
 
+  const char *pUnknown = "??";
+
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
   lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
@@ -199,9 +201,11 @@
   for (MIuint i = 0; bOk && (i < nModules); i++) {
 lldb::SBModule module = sbTarget.GetModuleAtIndex(i);
 if (module.IsValid()) {
+  auto fileSpec = module.GetFileSpec();
   const CMIUtilString strModuleFilePath(
-  module.GetFileSpec().GetDirectory());
-  const CMIUtilString strModuleFileName(module.GetFileSpec().GetFilename());
+  GetDereferenceable(fileSpec.GetDirectory()).Or(*pUnknown));
+  const CMIUtilString strModuleFileName(
+  GetDereferenceable(fileSpec.GetFilename()).Or(*pUnknown));
   const CMIUtilString strModuleFullPath(CMIUtilString::Format(
   "%s/%s", strModuleFilePath.c_str(), strModuleFileName.c_str()));
   const CMIUtilString strHasSymbols =
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55653: Check pointer results on nullptr before using them

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Looks like a great fix!




Comment at: MIDataTypes.h:67
+template 
+class MIDereferenceable {
+public:

Can we use `llvm::Optional` instead? Then we can use `getValueOr()`.



Comment at: MIDataTypes.h:71
+
+  T *Or(T &alt_value) { return m_ptr ? m_ptr : &alt_value; }
+private:

If we can not use `llvm::Optional` then why not just take `T *`?


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

https://reviews.llvm.org/D55653



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

[Lldb-commits] [PATCH] D55653: Check pointer results on nullptr before using them

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

Thanks for the review!




Comment at: MIDataTypes.h:67
+template 
+class MIDereferenceable {
+public:

shafik wrote:
> Can we use `llvm::Optional` instead? Then we can use `getValueOr()`.
Unfortunately no, with `Optional` getValueOr doesn't have the constraint 
that alternative value is not nullptr.
In case of `Optional`  getValueOr returns T by value which is not what we 
need.



Comment at: MIDataTypes.h:71
+
+  T *Or(T &alt_value) { return m_ptr ? m_ptr : &alt_value; }
+private:

shafik wrote:
> If we can not use `llvm::Optional` then why not just take `T *`?
It is easier to read:

```
  const char *pFileName =
  GetDereferenceable(fileSpec.GetFilename()).Or(*pUnknown);
```
vs

```
  const char *pFileName = fileSpec.GetFilename();
  pFileName = (pFileName != nullptr) ? pFileName : pUnknown;
```
isn't it?


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

https://reviews.llvm.org/D55653



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

[Lldb-commits] [PATCH] D58193: Do not explicitly depend on llvm tools during standalone build

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D58193#1398376 , @labath wrote:

> BTW, what's the reason for having super-high-fidelity list of dependencies in 
> a standalone build? I know that they're important for in-tree builds, as that 
> makes sure everything is built when you run "check-lldb". However, that's 
> isn't going to help you with standalone builds. The binaries are already 
> there, or they aren't, and there's nothing we can do to change that. I'd 
> consider just removing all non-lldb targets from the deps list when in 
> standalone mode.


I think it's useful to be able to fully test lldb if you have all the available 
tools even if it's built standalone. This is especially relevant for 
swift-lldb, which more often than not is built standalone because swift is 
often built standalone. I'm generally not in favor of keeping things around 
only because somebody downstream uses it, but I can see the usefulness for 
people who work on upstream LLDB as well. I'm not sure if other people feel the 
same way though.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58193



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

[Lldb-commits] [PATCH] D58230: [lldb] [MainLoop] Add kevent() EINTR handling

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D58230#1398020 , @krytarowski wrote:

> For EINTR we shall use `llvm::sys::RetryAfterSignal`


`kevent()` man page indicates:

> All changes contained in the changelist are applied before any pending events 
> are read from the queue.

Also:

> [EINTR]A signal was delivered before the timeout expired and 
> before any events were placed on the kqueue for return.

So while it's not stated explicitly, I think `in_events` is always consumed, 
even if EINTR is returned. In which case, `llvm::sys::RetryAfterSignal` would 
be wrong whenever `in_events` is not empty.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58230



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

[Lldb-commits] [PATCH] D58223: [lldb] [unittests] XFAIL two unittests failing on NetBSD

[Michał Górny via Phabricator via lldb-commits]

mgorny planned changes to this revision.
mgorny added a comment.

@labath suggested another implementation possibility on IRC, I'll be trying it 
tomorrow.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58223



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

[Lldb-commits] [PATCH] D58230: [lldb] [MainLoop] Add kevent() EINTR handling

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 186915.
mgorny edited the summary of this revision.
mgorny added a comment.
Herald added a subscriber: jfb.

Added the test case.


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

https://reviews.llvm.org/D58230

Files:
  lldb/source/Host/common/MainLoop.cpp
  lldb/unittests/Host/MainLoopTest.cpp


Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -141,4 +141,28 @@
   ASSERT_TRUE(loop.Run().Success());
   ASSERT_EQ(1u, callback_count);
 }
+
+// Test that a signal which is not monitored by the MainLoop does not
+// cause a premature exit.
+TEST_F(MainLoopTest, UnmonitoredSignal) {
+  MainLoop loop;
+  Status error;
+  struct sigaction sa;
+  sa.sa_sigaction = [](int, siginfo_t *, void *) { };
+  sa.sa_flags = SA_SIGINFO; // important: no SA_RESTART
+  sigemptyset(&sa.sa_mask);
+  ASSERT_EQ(0, sigaction(SIGUSR2, &sa, nullptr));
+
+  auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
+  ASSERT_TRUE(error.Success());
+  std::thread killer([]() {
+sleep(1);
+kill(getpid(), SIGUSR2);
+sleep(1);
+kill(getpid(), SIGUSR1);
+  });
+  ASSERT_TRUE(loop.Run().Success());
+  killer.join();
+  ASSERT_EQ(1u, callback_count);
+}
 #endif
Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -107,8 +107,14 @@
   num_events = kevent(loop.m_kqueue, in_events.data(), in_events.size(),
   out_events, llvm::array_lengthof(out_events), nullptr);
 
-  if (num_events < 0)
-return Status(errno, eErrorTypePOSIX);
+  if (num_events < 0) {
+if (errno == EINTR) {
+  // in case of EINTR, let the main loop run one iteration
+  // we need to zero num_events to avoid assertions failing
+  num_events = 0;
+} else
+  return Status(errno, eErrorTypePOSIX);
+  }
   return Status();
 }
 


Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -141,4 +141,28 @@
   ASSERT_TRUE(loop.Run().Success());
   ASSERT_EQ(1u, callback_count);
 }
+
+// Test that a signal which is not monitored by the MainLoop does not
+// cause a premature exit.
+TEST_F(MainLoopTest, UnmonitoredSignal) {
+  MainLoop loop;
+  Status error;
+  struct sigaction sa;
+  sa.sa_sigaction = [](int, siginfo_t *, void *) { };
+  sa.sa_flags = SA_SIGINFO; // important: no SA_RESTART
+  sigemptyset(&sa.sa_mask);
+  ASSERT_EQ(0, sigaction(SIGUSR2, &sa, nullptr));
+
+  auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
+  ASSERT_TRUE(error.Success());
+  std::thread killer([]() {
+sleep(1);
+kill(getpid(), SIGUSR2);
+sleep(1);
+kill(getpid(), SIGUSR1);
+  });
+  ASSERT_TRUE(loop.Run().Success());
+  killer.join();
+  ASSERT_EQ(1u, callback_count);
+}
 #endif
Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -107,8 +107,14 @@
   num_events = kevent(loop.m_kqueue, in_events.data(), in_events.size(),
   out_events, llvm::array_lengthof(out_events), nullptr);
 
-  if (num_events < 0)
-return Status(errno, eErrorTypePOSIX);
+  if (num_events < 0) {
+if (errno == EINTR) {
+  // in case of EINTR, let the main loop run one iteration
+  // we need to zero num_events to avoid assertions failing
+  num_events = 0;
+} else
+  return Status(errno, eErrorTypePOSIX);
+  }
   return Status();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58222: [ClangExpressionParser] Reuse the FileManager from the compiler instance.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

This change was probably due to things changing over time and there may be more 
of these kinds of cleanups to be had. nice catch.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58222



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

[Lldb-commits] [PATCH] D55653: Check pointer results on nullptr before using them

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In the future please include context with the diff (-U should do the 
trick), it makes reviewing a lot easier.

Have you considered wrapping raw strings in llvm's `StringRef`s? It looks like 
lldb-mi is already using some ADT classes from llvm. 
Unfortunately we cannot use those in the SB interface, but they're a lot safer. 
They have a `.str()` which does the right thing and returns an empty string 
when the raw string is a `nullptr`. Personally I would very much prefer that 
over the `This.or(That)` pattern, but if you really wanted that you could have 
an Optional and do `getValueOr()` as Shafik suggested.


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

https://reviews.llvm.org/D55653



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

[Lldb-commits] [PATCH] D58230: [lldb] [MainLoop] Add kevent() EINTR handling

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D58230#1398529 , @mgorny wrote:

> In D58230#1398020 , @krytarowski 
> wrote:
>
> > For EINTR we shall use `llvm::sys::RetryAfterSignal`
>
>
> `kevent()` man page indicates:
>
> > All changes contained in the changelist are applied before any pending 
> > events are read from the queue.
>
> Also:
>
> > [EINTR]A signal was delivered before the timeout expired and 
> > before any events were placed on the kqueue for return.
>
> So while it's not stated explicitly, I think `in_events` is always consumed, 
> even if EINTR is returned. In which case, `llvm::sys::RetryAfterSignal` would 
> be wrong whenever `in_events` is not empty.


Please bring it to tech-kern@ to clear this and improve documentation. I think 
that we need to use `llvm::sys::RetryAfterSignal`.


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

https://reviews.llvm.org/D58230



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

[Lldb-commits] [PATCH] D58257: Disable ExecControl/StopHook/stop-hook-threads.test on Linux

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe created this revision.
jgorbe added reviewers: labath, zturner.
Herald added a subscriber: jdoerfert.

ExecControl/StopHook/stop-hook-threads.test is flaky on Linux (it's 
consistently failing on my machine, but doesn't fail on a co-worker's). I'm 
seeing the following assertion failure:

  CommandObject.cpp:145: bool 
lldb_private::CommandObject::CheckRequirements(lldb_private::CommandReturnObject&):
 Assertion `m_exe_ctx.GetTargetPtr() == NULL' failed.

Interestingly, this doesn't happen when typing the same commands in interactive 
mode. The cause seems to be that, in synchronous execution mode `continue` 
waits until the process stops again, and that includes running any stop-hooks 
for that later stop, so we end with a stack trace like this (lots of frames 
omitted for clarity):

  abort()
  CommandObject::CheckRequirements() <-- this is again the same instance of 
CommandObjectProcessContinue, fails assertion because the previous continue 
command hasn't finished.
  Target::RunStopHooks()
  CommandObjectProcessContinue::DoExecute()
  Target::RunStopHooks()

In general, it seems like using process control commands inside stop-hooks does 
not have very well defined semantics. You don't even need multiple threads to 
make that assertion fail, you can build

  #include 
  int main() {
printf("1\n");  // break1
printf("2\n");  // break2
  }

and then on lldb

  target stop-hook add -o continue
  break set -f stop-hook-simple.cpp -p "break1"
  break set -f stop-hook-simple.cpp -p "break2"
  run

In this case it's even worse because the presence of multiple threads makes it 
prone to race conditions. In some tests I ran with a simpler version of this 
test case, I was hitting either the previous assertion failure or the following 
issue:

1. Two threads reach a breakpoint
2. First stop-hook does a `process continue`
3. Threads end
4. Second stop-hook runs, complains about process not existing.

This change disables the test on Linux. It's already marked as XFAIL on 
Windows, so maybe we should just delete it until it's clear what should be the 
expected behavior in these cases. Or maybe try to come up with a way to write a 
similar multithreaded test without calling `continue` from a stop hook, I don't 
know.


https://reviews.llvm.org/D58257

Files:
  lldb/lit/ExecControl/StopHook/stop-hook-threads.test


Index: lldb/lit/ExecControl/StopHook/stop-hook-threads.test
===
--- lldb/lit/ExecControl/StopHook/stop-hook-threads.test
+++ lldb/lit/ExecControl/StopHook/stop-hook-threads.test
@@ -4,6 +4,7 @@
 # RUN: %lldb -b -s %p/Inputs/stop-hook-threads-2.lldbinit -s %s -f %t \
 # RUN: | FileCheck --check-prefix=CHECK --check-prefix=CHECK-FILTER %s
 # XFAIL: system-windows
+# UNSUPPORTED: linux
 
 thread list
 break set -f stop-hook-threads.cpp -p "Set break point at this line"


Index: lldb/lit/ExecControl/StopHook/stop-hook-threads.test
===
--- lldb/lit/ExecControl/StopHook/stop-hook-threads.test
+++ lldb/lit/ExecControl/StopHook/stop-hook-threads.test
@@ -4,6 +4,7 @@
 # RUN: %lldb -b -s %p/Inputs/stop-hook-threads-2.lldbinit -s %s -f %t \
 # RUN: | FileCheck --check-prefix=CHECK --check-prefix=CHECK-FILTER %s
 # XFAIL: system-windows
+# UNSUPPORTED: linux
 
 thread list
 break set -f stop-hook-threads.cpp -p "Set break point at this line"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58257: Disable ExecControl/StopHook/stop-hook-threads.test on Linux

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I haven't gotten a chance to revisit the stop hooks in a long time.  They 
definitely need some love.

IMO, stop hooks should have the same semantics as breakpoint commands, the 
first command that causes the target to run should exit the stop hook.  That 
way you would never get into a situation where you hit another stop hook while 
handing previous stop's hit.

And we should also add a --continue (maybe also --step & --next) option to the 
stop hook.  Then we could just disallow step/next/continue in stop hooks.  It 
is really hard to reason about what to do when handling stops if the contents 
of the stop hook can restart the target out from under you.  For instance, if 
you have multiple stop hooks, you have to abort running all the other stop 
hooks after the first one continues.


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

https://reviews.llvm.org/D58257



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

[Lldb-commits] [lldb] r354099 - Implement GetLoadAddress for the Windows process plugin

[Aaron Smith via lldb-commits]

Author: asmith
Date: Thu Feb 14 20:32:50 2019
New Revision: 354099

URL: http://llvm.org/viewvc/llvm-project?rev=354099&view=rev
Log:
Implement GetLoadAddress for the Windows process plugin

Summary:
When a process is loaded, update its sections with the load address to resolve 
any created breakpoints. For the remote debugging case, the debugged process is 
launched remotely so GetLoadAddress is intended to pass the load address from 
remote to LLDB (client).


Reviewers: zturner, llvm-commits, clayborg, labath

Reviewed By: labath

Subscribers: mgorny, sas, Hui, clayborg, labath, lldb-commits

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

Modified:

lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp

lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h
lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.h

Modified: 
lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp?rev=354099&r1=354098&r2=354099&view=diff
==
--- 
lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
 Thu Feb 14 20:32:50 2019
@@ -12,6 +12,7 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
@@ -61,6 +62,56 @@ DynamicLoader *DynamicLoaderWindowsDYLD:
   return nullptr;
 }
 
+void DynamicLoaderWindowsDYLD::OnLoadModule(const ModuleSpec &module_spec,
+lldb::addr_t module_addr) {
+  // Confusingly, there is no Target::AddSharedModule.  Instead, calling
+  // GetSharedModule() with a new module will add it to the module list and
+  // return a corresponding ModuleSP.
+  Status error;
+  ModuleSP module_sp =
+  m_process->GetTarget().GetSharedModule(module_spec, &error);
+  if (error.Fail())
+return;
+
+  m_loaded_modules[module_sp] = module_addr;
+  UpdateLoadedSectionsCommon(module_sp, module_addr, false);
+}
+
+void DynamicLoaderWindowsDYLD::OnUnloadModule(lldb::addr_t module_addr) {
+  Address resolved_addr;
+  if (!m_process->GetTarget().ResolveLoadAddress(module_addr, resolved_addr))
+return;
+
+  ModuleSP module_sp = resolved_addr.GetModule();
+  if (module_sp) {
+m_loaded_modules.erase(module_sp);
+UnloadSectionsCommon(module_sp);
+  }
+}
+
+lldb::addr_t DynamicLoaderWindowsDYLD::GetLoadAddress(ModuleSP executable) {
+  // First, see if the load address is already cached.
+  auto it = m_loaded_modules.find(executable);
+  if (it != m_loaded_modules.end() && it->second != LLDB_INVALID_ADDRESS)
+return it->second;
+
+  lldb::addr_t load_addr = LLDB_INVALID_ADDRESS;
+
+  // Second, try to get it through the process plugins.  For a remote process,
+  // the remote platform will be responsible for providing it.
+  FileSpec file_spec(executable->GetPlatformFileSpec());
+  bool is_loaded = false;
+  Status status =
+  m_process->GetFileLoadAddress(file_spec, is_loaded, load_addr);
+  // Servers other than lldb server could respond with a bogus address.
+  if (status.Success() && is_loaded && load_addr != LLDB_INVALID_ADDRESS) {
+m_loaded_modules[executable] = load_addr;
+return load_addr;
+  }
+
+  return LLDB_INVALID_ADDRESS;
+}
+
 void DynamicLoaderWindowsDYLD::DidAttach() {
 Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
   if (log)
@@ -73,21 +124,17 @@ void DynamicLoaderWindowsDYLD::DidAttach
 
   // Try to fetch the load address of the file from the process, since there
   // could be randomization of the load address.
+  lldb::addr_t load_addr = GetLoadAddress(executable);
+  if (load_addr == LLDB_INVALID_ADDRESS)
+return;
 
-  // It might happen that the remote has a different dir for the file, so we
-  // only send the basename of the executable in the query. I think this is 
safe
-  // because I doubt that two executables with the same basenames are loaded in
-  // memory...
-  FileSpec file_spec(
-  executable->GetPlatformFileSpec().GetFilename().GetCString());
-  bool is_loaded;
-  addr_t base_addr = 0;
-  lldb::addr_t load_addr;
-  Status error = m_process->GetFileLoadAddress(file_spec, is_loaded, 
load_addr);
-  if (error.Success() && is_loaded) {
-base_addr = load_addr;
-UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, base_addr, false);
-  }
+  // Request the process base address.
+  lldb::addr_t image_base = m_process->Get

[Lldb-commits] [PATCH] D56237: Implement GetLoadAddress for the Windows process plugin

[Aaron Smith via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL354099: Implement GetLoadAddress for the Windows process 
plugin (authored by asmith, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D56237?vs=186799&id=186958#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56237

Files:
  
lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
  
lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h
  lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
  lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.h

Index: lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.h
===
--- lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.h
+++ lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.h
@@ -16,6 +16,7 @@
 #include "llvm/Support/Mutex.h"
 
 #include "IDebugDelegate.h"
+#include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
 
 namespace lldb_private {
 
@@ -90,6 +91,8 @@
 
   lldb::addr_t GetImageInfoAddress() override;
 
+  DynamicLoaderWindowsDYLD *GetDynamicLoader() override;
+
   // IDebugDelegate overrides.
   void OnExitProcess(uint32_t exit_code) override;
   void OnDebuggerConnected(lldb::addr_t image_base) override;
Index: lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
===
--- lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
+++ lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
@@ -24,6 +24,7 @@
 lldbCore
 lldbHost
 lldbInterpreter
+lldbPluginDynamicLoaderWindowsDYLD
 lldbSymbol
 lldbTarget
 ws2_32
Index: lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -864,6 +864,13 @@
 return LLDB_INVALID_ADDRESS;
 }
 
+DynamicLoaderWindowsDYLD *ProcessWindows::GetDynamicLoader() {
+  if (m_dyld_ap.get() == NULL)
+m_dyld_ap.reset(DynamicLoader::FindPlugin(
+this, DynamicLoaderWindowsDYLD::GetPluginNameStatic().GetCString()));
+  return static_cast(m_dyld_ap.get());
+}
+
 void ProcessWindows::OnExitProcess(uint32_t exit_code) {
   // No need to acquire the lock since m_session_data isn't accessed.
   Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_PROCESS);
@@ -916,12 +923,10 @@
 GetTarget().SetExecutableModule(module, eLoadDependentsNo);
   }
 
-  bool load_addr_changed;
-  module->SetLoadAddress(GetTarget(), image_base, false, load_addr_changed);
-
-  ModuleList loaded_modules;
-  loaded_modules.Append(module);
-  GetTarget().ModulesDidLoad(loaded_modules);
+  if (auto dyld = GetDynamicLoader())
+dyld->OnLoadModule(
+ModuleSpec(module->GetFileSpec(), module->GetArchitecture()),
+image_base);
 
   // Add the main executable module to the list of pending module loads.  We
   // can't call GetTarget().ModulesDidLoad() here because we still haven't
@@ -1027,29 +1032,13 @@
 
 void ProcessWindows::OnLoadDll(const ModuleSpec &module_spec,
lldb::addr_t module_addr) {
-  // Confusingly, there is no Target::AddSharedModule.  Instead, calling
-  // GetSharedModule() with a new module will add it to the module list and
-  // return a corresponding ModuleSP.
-  Status error;
-  ModuleSP module = GetTarget().GetSharedModule(module_spec, &error);
-  bool load_addr_changed = false;
-  module->SetLoadAddress(GetTarget(), module_addr, false, load_addr_changed);
-
-  ModuleList loaded_modules;
-  loaded_modules.Append(module);
-  GetTarget().ModulesDidLoad(loaded_modules);
+  if (auto dyld = GetDynamicLoader())
+dyld->OnLoadModule(module_spec, module_addr);
 }
 
 void ProcessWindows::OnUnloadDll(lldb::addr_t module_addr) {
-  Address resolved_addr;
-  if (GetTarget().ResolveLoadAddress(module_addr, resolved_addr)) {
-ModuleSP module = resolved_addr.GetModule();
-if (module) {
-  ModuleList unloaded_modules;
-  unloaded_modules.Append(module);
-  GetTarget().ModulesDidUnload(unloaded_modules, false);
-}
-  }
+  if (auto dyld = GetDynamicLoader())
+dyld->OnUnloadModule(module_addr);
 }
 
 void ProcessWindows::OnDebugString(const std::string &string) {}
Index: lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h
===
--- lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoad

[Lldb-commits] [lldb] r354100 - Fix for build bot problem from last change

[Aaron Smith via lldb-commits]

Author: asmith
Date: Thu Feb 14 22:13:59 2019
New Revision: 354100

URL: http://llvm.org/viewvc/llvm-project?rev=354100&view=rev
Log:
Fix for build bot problem from last change

Modified:
lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp

Modified: lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp?rev=354100&r1=354099&r2=354100&view=diff
==
--- lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp Thu Feb 
14 22:13:59 2019
@@ -865,10 +865,10 @@ lldb::addr_t ProcessWindows::GetImageInf
 }
 
 DynamicLoaderWindowsDYLD *ProcessWindows::GetDynamicLoader() {
-  if (m_dyld_ap.get() == NULL)
-m_dyld_ap.reset(DynamicLoader::FindPlugin(
+  if (m_dyld_up.get() == NULL)
+m_dyld_up.reset(DynamicLoader::FindPlugin(
 this, DynamicLoaderWindowsDYLD::GetPluginNameStatic().GetCString()));
-  return static_cast(m_dyld_ap.get());
+  return static_cast(m_dyld_up.get());
 }
 
 void ProcessWindows::OnExitProcess(uint32_t exit_code) {


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

[Lldb-commits] [PATCH] D58230: [lldb] [MainLoop] Add kevent() EINTR handling

[Pavel Labath via Phabricator via lldb-commits]

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

The change looks fine to me. I don't understand all the intricacies of the 
RetryAfterSignal discussion (AFAICT, it does not matter what you do there, 
since `RunImpl::Poll` is called in a loop anyway), so I'll let you sort that 
out with Kamil.


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

https://reviews.llvm.org/D58230



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

[Lldb-commits] [PATCH] D58257: Disable ExecControl/StopHook/stop-hook-threads.test on Linux

[Pavel Labath via Phabricator via lldb-commits]

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

lgtm. I think the current behavior of stop hooks with process control commands 
is so under-defined that it does not make sense to run this test, especially 
given that we're considering disallowing that altogether. If we wanted to just 
test that stop hooks do run in the presence of multiple threads, then it should 
be possible to replace the "continue" command in the hook with something else 
which e.g. prints a variable. Then we can just check that the variable gets 
displayed correctly.

In D58257#1398766 , @jingham wrote:

> And we should also add a --continue (maybe also --step & --next) option to 
> the stop hook.  Then we could just disallow step/next/continue in stop hooks. 
>  It is really hard to reason about what to do when handling stops if the 
> contents of the stop hook can restart the target out from under you.  For 
> instance, if you have multiple stop hooks, you have to abort running all the 
> other stop hooks after the first one continues.


I like that a lot. In fact, I was proposing something very similar when we were 
discussing this internally. :)


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

https://reviews.llvm.org/D58257



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

[Lldb-commits] [lldb] r354103 - Remove redundant semicolon after namespace-closing '}'

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu Feb 14 23:33:37 2019
New Revision: 354103

URL: http://llvm.org/viewvc/llvm-project?rev=354103&view=rev
Log:
Remove redundant semicolon after namespace-closing '}'

Modified:
lldb/trunk/include/lldb/Symbol/SourceModule.h

Modified: lldb/trunk/include/lldb/Symbol/SourceModule.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SourceModule.h?rev=354103&r1=354102&r2=354103&view=diff
==
--- lldb/trunk/include/lldb/Symbol/SourceModule.h (original)
+++ lldb/trunk/include/lldb/Symbol/SourceModule.h Thu Feb 14 23:33:37 2019
@@ -22,6 +22,6 @@ struct SourceModule {
   ConstString sysroot;
 };
 
-}; // namespace lldb_private
+} // namespace lldb_private
 
 #endif


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

[Lldb-commits] [PATCH] D58193: Do not explicitly depend on llvm tools during standalone build

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D58193#1398458 , @xiaobai wrote:

> I think it's useful to be able to fully test lldb if you have all the 
> available tools even if it's built standalone. This is especially relevant 
> for swift-lldb, which more often than not is built standalone because swift 
> is often built standalone. I'm generally not in favor of keeping things 
> around only because somebody downstream uses it, but I can see the usefulness 
> for people who work on upstream LLDB as well. I'm not sure if other people 
> feel the same way though.


I certainly agree with that, but I don't see how fiddling with LLDB_TEST_DEPS 
is going to help testing at all. AFAICT, all it does it set's up the 
dependencies in the cmake graph. That is not going to help you run the tests. 
If the tools are there, the tests will run fine; if they aren't the tests will 
fail.

Now if we were talking about detecting the available tools and automatically 
warning about their absence and/or skipping tests, that would be a different 
story, but I don't see anyone proposing that...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58193



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

[Lldb-commits] [lldb] r354104 - Embed swig version into lldb.py in a different way

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu Feb 14 23:41:12 2019
New Revision: 354104

URL: http://llvm.org/viewvc/llvm-project?rev=354104&view=rev
Log:
Embed swig version into lldb.py in a different way

Summary:
Instead of doing string chopping on the resulting python file, get swig
to output the version for us. The two things which make slightly
non-trivial are:
- in order to get swig to expand SWIG_VERSION for us, we cannot use
  %pythoncode directly, but we have to go through an intermediate macro.
- SWIG_VERSION is a hex number, but it's components are supposed to be
  interpreted decimally, so there is a bit of integer magic needed to
  get the right number to come out.

I've tested that this approach works both with the latest (3.0.12) and
oldest (1.3.40) supported swig.

Reviewers: zturner, jingham, serge-sans-paille

Subscribers: jdoerfert, lldb-commits

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

Added:

lldb/trunk/packages/Python/lldbsuite/test/python_api/lldbutil/TestSwigVersion.py
Modified:
lldb/trunk/scripts/Python/modify-python-lldb.py
lldb/trunk/scripts/lldb.swig

Added: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/lldbutil/TestSwigVersion.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/lldbutil/TestSwigVersion.py?rev=354104&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/lldbutil/TestSwigVersion.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/lldbutil/TestSwigVersion.py
 Thu Feb 14 23:41:12 2019
@@ -0,0 +1,28 @@
+"""
+Test that we embed the swig version into the lldb module
+"""
+
+from __future__ import print_function
+
+"""
+import os
+import time
+import re
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+"""
+from lldbsuite.test.lldbtest import *
+
+class SwigVersionTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test(self):
+self.assertTrue(getattr(lldb, "swig_version"))
+self.assertIsInstance(lldb.swig_version, tuple)
+self.assertEqual(len(lldb.swig_version), 3)
+self.assertGreaterEqual(lldb.swig_version[0], 1)
+for v in lldb.swig_version:
+self.assertGreaterEqual(v, 0)

Modified: lldb/trunk/scripts/Python/modify-python-lldb.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/modify-python-lldb.py?rev=354104&r1=354103&r2=354104&view=diff
==
--- lldb/trunk/scripts/Python/modify-python-lldb.py (original)
+++ lldb/trunk/scripts/Python/modify-python-lldb.py Thu Feb 14 23:41:12 2019
@@ -45,11 +45,6 @@ else:
 # print "output_name is '" + output_name + "'"
 
 #
-# Version string
-#
-version_line = "swig_version = %s"
-
-#
 # Residues to be removed.
 #
 c_endif_swig = "#endif"
@@ -338,7 +333,6 @@ init_pattern = re.compile("^def __in
 isvalid_pattern = re.compile("^def IsValid\(")
 
 # These define the states of our finite state machine.
-EXPECTING_VERSION = 0
 NORMAL = 1
 DEFINING_ITERATOR = 2
 DEFINING_EQUALITY = 4
@@ -364,9 +358,8 @@ lldb_iter_defined = False
 # The FSM, in all possible states, also checks the current input for IsValid()
 # definition, and inserts a __nonzero__() method definition to implement truth
 # value testing and the built-in operation bool().
-state = EXPECTING_VERSION
+state = NORMAL
 
-swig_version_tuple = None
 for line in content.splitlines():
 # Handle the state transition into CLEANUP_DOCSTRING state as it is 
possible
 # to enter this state from either NORMAL or DEFINING_ITERATOR/EQUALITY.
@@ -383,20 +376,6 @@ for line in content.splitlines():
 else:
 state |= CLEANUP_DOCSTRING
 
-if state == EXPECTING_VERSION:
-# We haven't read the version yet, read it now.
-if swig_version_tuple is None:
-match = version_pattern.search(line)
-if match:
-v = match.group(1)
-swig_version_tuple = tuple(map(int, (v.split("."
-elif not line.startswith('#'):
-# This is the first non-comment line after the header.  Inject the
-# version
-new_line = version_line % str(swig_version_tuple)
-new_content.add_line(new_line)
-state = NORMAL
-
 if state == NORMAL:
 match = class_pattern.search(line)
 # Inserts lldb_helpers and the lldb_iter() definition before the first

Modified: lldb/trunk/scripts/lldb.swig
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/lldb.swig?rev=354104&r1=354103&r2=354104&view=diff
==
--- lldb/trunk/scripts/lldb.swig (original)
+++ lldb/trunk/scripts/lldb.swig Thu Feb 14 23:41:12 2019
@@ -66,6 +66,21 @@ import os
 
 import six
 %}
+
+// Incl

[Lldb-commits] [lldb] r354105 - Use sys.executable in lldb-dotest

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu Feb 14 23:41:17 2019
New Revision: 354105

URL: http://llvm.org/viewvc/llvm-project?rev=354105&view=rev
Log:
Use sys.executable in lldb-dotest

Without that, dotest.py would be executed with the default python
interpreter, which may not be the same one that lldb is built with.

This still requires the user do know which python interpreter to use
when running lldb-dotest, but now he is at least able to choose it, if
he knows which one to use.

Modified:
lldb/trunk/utils/lldb-dotest/lldb-dotest.in

Modified: lldb/trunk/utils/lldb-dotest/lldb-dotest.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/utils/lldb-dotest/lldb-dotest.in?rev=354105&r1=354104&r2=354105&view=diff
==
--- lldb/trunk/utils/lldb-dotest/lldb-dotest.in (original)
+++ lldb/trunk/utils/lldb-dotest/lldb-dotest.in Thu Feb 14 23:41:17 2019
@@ -9,7 +9,7 @@ if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
 dotest_args = dotest_args_str.split(';')
 # Build dotest.py command.
-cmd = [dotest_path, '-q']
+cmd = [sys.executable, dotest_path, '-q']
 cmd.extend(dotest_args)
 cmd.extend(wrapper_args)
 # Invoke dotest.py and return exit code.


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

[Lldb-commits] [PATCH] D58172: Embed swig version into lldb.py in a different way

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB354104: Embed swig version into lldb.py in a different 
way (authored by labath, committed by ).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D58172?vs=186676&id=186968#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58172

Files:
  packages/Python/lldbsuite/test/python_api/lldbutil/TestSwigVersion.py
  scripts/Python/modify-python-lldb.py
  scripts/lldb.swig

Index: packages/Python/lldbsuite/test/python_api/lldbutil/TestSwigVersion.py
===
--- packages/Python/lldbsuite/test/python_api/lldbutil/TestSwigVersion.py
+++ packages/Python/lldbsuite/test/python_api/lldbutil/TestSwigVersion.py
@@ -0,0 +1,28 @@
+"""
+Test that we embed the swig version into the lldb module
+"""
+
+from __future__ import print_function
+
+"""
+import os
+import time
+import re
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+"""
+from lldbsuite.test.lldbtest import *
+
+class SwigVersionTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test(self):
+self.assertTrue(getattr(lldb, "swig_version"))
+self.assertIsInstance(lldb.swig_version, tuple)
+self.assertEqual(len(lldb.swig_version), 3)
+self.assertGreaterEqual(lldb.swig_version[0], 1)
+for v in lldb.swig_version:
+self.assertGreaterEqual(v, 0)
Index: scripts/lldb.swig
===
--- scripts/lldb.swig
+++ scripts/lldb.swig
@@ -66,6 +66,21 @@
 
 import six
 %}
+
+// Include the version of swig that was used to generate this interface.
+%define EMBED_VERSION(VERSION)
+%pythoncode%{
+# SWIG_VERSION is written as a single hex number, but the components of it are
+# meant to be interpreted in decimal. So, 0x030012 is swig 3.0.12, and not
+# 3.0.18.
+def _to_int(hex):
+return hex // 0x10 % 0x10 * 10 + hex % 0x10
+swig_version = (_to_int(VERSION // 0x1), _to_int(VERSION // 0x100), _to_int(VERSION))
+del _to_int
+%}
+%enddef
+EMBED_VERSION(SWIG_VERSION)
+
 %include "./Python/python-typemaps.swig"
 
 /* C++ headers to be included. */
Index: scripts/Python/modify-python-lldb.py
===
--- scripts/Python/modify-python-lldb.py
+++ scripts/Python/modify-python-lldb.py
@@ -45,11 +45,6 @@
 # print "output_name is '" + output_name + "'"
 
 #
-# Version string
-#
-version_line = "swig_version = %s"
-
-#
 # Residues to be removed.
 #
 c_endif_swig = "#endif"
@@ -338,7 +333,6 @@
 isvalid_pattern = re.compile("^def IsValid\(")
 
 # These define the states of our finite state machine.
-EXPECTING_VERSION = 0
 NORMAL = 1
 DEFINING_ITERATOR = 2
 DEFINING_EQUALITY = 4
@@ -364,9 +358,8 @@
 # The FSM, in all possible states, also checks the current input for IsValid()
 # definition, and inserts a __nonzero__() method definition to implement truth
 # value testing and the built-in operation bool().
-state = EXPECTING_VERSION
+state = NORMAL
 
-swig_version_tuple = None
 for line in content.splitlines():
 # Handle the state transition into CLEANUP_DOCSTRING state as it is possible
 # to enter this state from either NORMAL or DEFINING_ITERATOR/EQUALITY.
@@ -383,20 +376,6 @@
 else:
 state |= CLEANUP_DOCSTRING
 
-if state == EXPECTING_VERSION:
-# We haven't read the version yet, read it now.
-if swig_version_tuple is None:
-match = version_pattern.search(line)
-if match:
-v = match.group(1)
-swig_version_tuple = tuple(map(int, (v.split("."
-elif not line.startswith('#'):
-# This is the first non-comment line after the header.  Inject the
-# version
-new_line = version_line % str(swig_version_tuple)
-new_content.add_line(new_line)
-state = NORMAL
-
 if state == NORMAL:
 match = class_pattern.search(line)
 # Inserts lldb_helpers and the lldb_iter() definition before the first
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58177: Fix lldb-server test suite for python3

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354106: Fix lldb-server test suite for python3 (authored by 
labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58177

Files:
  lldb/trunk/packages/Python/lldbsuite/support/seven.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteModuleInfo.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py

Index: lldb/trunk/packages/Python/lldbsuite/support/seven.py
===
--- lldb/trunk/packages/Python/lldbsuite/support/seven.py
+++ lldb/trunk/packages/Python/lldbsuite/support/seven.py
@@ -1,3 +1,4 @@
+import binascii
 import six
 
 if six.PY2:
@@ -23,3 +24,28 @@
 return get_command_status_output(command)[1]
 
 cmp_ = lambda x, y: (x > y) - (x < y)
+
+def bitcast_to_string(b):
+"""
+Take a string(PY2) or a bytes(PY3) object and return a string. The returned
+string contains the exact same bytes as the input object (latin1 <-> unicode
+transformation is an identity operation for the first 256 code points).
+"""
+return b if six.PY2 else b.decode("latin1")
+
+def bitcast_to_bytes(s):
+"""
+Take a string and return a string(PY2) or a bytes(PY3) object. The returned
+object contains the exact same bytes as the input string. (latin1 <->
+unicode transformation is an identity operation for the first 256 code
+points).
+"""
+return s if six.PY2 else s.encode("latin1")
+
+def unhexlify(hexstr):
+"""Hex-decode a string. The result is always a string."""
+return bitcast_to_string(binascii.unhexlify(hexstr))
+
+def hexlify(data):
+"""Hex-encode string data. The result if always a string."""
+return bitcast_to_string(binascii.hexlify(bitcast_to_bytes(data)))
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
@@ -10,7 +10,7 @@
 the initial set of tests implemented.
 """
 
-from __future__ import print_function
+from __future__ import division, print_function
 
 
 import unittest2
@@ -18,6 +18,7 @@
 import lldbgdbserverutils
 import platform
 import signal
+from lldbsuite.support import seven
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.lldbdwarf import *
@@ -868,7 +869,7 @@
 
 # Ensure what we read from inferior memory is what we wrote.
 self.assertIsNotNone(context.get("read_contents"))
-read_contents = context.get("read_contents").decode("hex")
+read_contents = seven.unhexlify(context.get("read_contents"))
 self.assertEqual(read_contents, MEMORY_CONTENTS)
 
 @debugserver_test
@@ -1352,7 +1353,7 @@
 message_address = int(context.get("message_address"), 16)
 
 # Hex-encode the test message, adding null termination.
-hex_encoded_message = TEST_MESSAGE.encode("hex")
+hex_encoded_message = seven.hexlify(TEST_MESSAGE)
 
 # Write the message to the inferior. Verify that we can read it with the hex-encoded (m)
 # and binary (x) memory read packets.
@@ -1467,7 +1468,7 @@
 
 reg_index = self.select_modifiable_register(reg_infos)
 self.assertIsNotNone(reg_index)
-reg_byte_size = int(reg_infos[reg_index]["bitsize"]) / 8
+reg_byte_size = int(reg_infos[reg_index]["bitsize"]) // 8
 self.assertTrue(reg_byte_size > 0)
 
 # Run the process a bit so threads can start up, and collect register
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py
@@ -9,6 +9,7 @@
 import codecs
 
 from six.moves import queue
+from lldbsuite.support import seven
 
 
 def _handle_output_packet_string(packet_contents):
@@ -19,7 +20,7 @@
 elif packet_contents == "OK":
 return None
 else:
-return packet_contents[1:].decode("hex")
+return seven.unhexlify(packet_contents[1:])
 
 
 def _dump_queue(the_queue):
@@ -174,7 +175,7 @@
 can_read, _, _ = select.select([self._socket], [], [], 0)
 if 

[Lldb-commits] [PATCH] D58177: Fix lldb-server test suite for python3

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thank you for reviewing this.


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

https://reviews.llvm.org/D58177



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

[Lldb-commits] [lldb] r354106 - Fix lldb-server test suite for python3

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu Feb 14 23:47:06 2019
New Revision: 354106

URL: http://llvm.org/viewvc/llvm-project?rev=354106&view=rev
Log:
Fix lldb-server test suite for python3

Summary:
This patch finishes the python3-ification of the lldb-server test suite.
It reverts the partial attempt in r352709 to encode/decode the string
via utf8 before writing to the socket. This wasn't enough because the
gdb-remote protocol can sometimes (but not very often) carry binary
data, and the utf8 codec chokes on that. Instead I add utility functions
to the "seven" module for performing "identity" transformations on the
byte data. This basically drills back the hole in the python type system
that the string/bytes distinction was supposed to plug. That is not
ideal, but was the best solution of the alternatives I could come up
with. The options I considered were:
- make use of the type system to add type safety to the test suite: This
  required making a lot of changes to the test suite, since most of the
  strings would now become byte objects instead, and it was not even
  fully clear to me where to draw the line. One extreme solution would
  be to just use byte objects everywhere, as the protocol doesn't
  support non-ascii characters anyway. However, this appeared to be:
  a) weird, because most of the protocol actually deals with strings,
 but we would have to prefix everything with 'b'
  b) clunky, because the handling of the bytes objects is sufficiently
 different in PY2 and PY3 (e.g. b'a'[0] is a string in PY2, but an
 int in PY3).
- using the latin1 codec (which gives an identity transformation for the
  first 256 code points of unicode) instead of the custom
  bytes_to_string functions. This almost could work, but it was still
  slightly different between python 2 and 3, because in PY2 in would
  return a unicode object, which would then cause problems when
  combined with regular strings if it contained 8-bit chars.

With this in mind, I think the best solution for the time being is to
just coerce everything into the string type as early as possible, and
have things proceed indentically on both python versions. Once we stop
supporting python3, we can revisit the idea of using bytes objects more
prevasively.

Reviewers: davide, zturner, serge-sans-paille

Subscribers: lldb-commits

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

Modified:
lldb/trunk/packages/Python/lldbsuite/support/seven.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteModuleInfo.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py

Modified: lldb/trunk/packages/Python/lldbsuite/support/seven.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/support/seven.py?rev=354106&r1=354105&r2=354106&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/support/seven.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/support/seven.py Thu Feb 14 23:47:06 
2019
@@ -1,3 +1,4 @@
+import binascii
 import six
 
 if six.PY2:
@@ -23,3 +24,28 @@ else:
 return get_command_status_output(command)[1]
 
 cmp_ = lambda x, y: (x > y) - (x < y)
+
+def bitcast_to_string(b):
+"""
+Take a string(PY2) or a bytes(PY3) object and return a string. The returned
+string contains the exact same bytes as the input object (latin1 <-> 
unicode
+transformation is an identity operation for the first 256 code points).
+"""
+return b if six.PY2 else b.decode("latin1")
+
+def bitcast_to_bytes(s):
+"""
+Take a string and return a string(PY2) or a bytes(PY3) object. The returned
+object contains the exact same bytes as the input string. (latin1 <->
+unicode transformation is an identity operation for the first 256 code
+points).
+"""
+return s if six.PY2 else s.encode("latin1")
+
+def unhexlify(hexstr):
+"""Hex-decode a string. The result is always a string."""
+return bitcast_to_string(binascii.unhexlify(hexstr))
+
+def hexlify(data):
+"""Hex-encode string data. The result if always a string."""
+return bitcast_to_string(binascii.hexlify(bitcast_to_bytes(data)))

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteModuleInfo.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteModuleInfo.py?rev=354106&r1=354105&r2=354106&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteModuleInfo.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lld