[Lldb-commits] [PATCH] D47228: Break dependency from Core to ObjectFileJIT

[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 sounds reasonable to me. I should just point out that this is the 
Core <-> ObjectFile**JIT** cycle you are breaking.




Comment at: lldb/include/lldb/Core/Module.h:165
+module_sp->m_objfile_sp =
+std::make_shared(module_sp, args...);
+

std::forward(args)...



Comment at: lldb/source/Expression/IRExecutionUnit.cpp:35-36
 
 #include "lldb/../../source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "lldb/../../source/Plugins/ObjectFile/JIT/ObjectFileJIT.h"
 

`#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"` should work here


https://reviews.llvm.org/D47228



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

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Both of the solutions sound plausible to me (extra struct vs. moving REPL to 
`Command`). Maybe that's because I don't know enough about the REPL to have 
formed an opinion on it.




Comment at: lldb/include/lldb/Expression/REPL.h:31
+bool TryAllThreads = true;
+uint32_t Timeout = 0;
+  };

If you stick with this solution, please make this `Timeout`


https://reviews.llvm.org/D47232



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In a way, this makes sense, but in a lot of other ways, it actually makes 
things worse :)

My long-term goal is to be able to build lldb-server without even having clang 
checked out (because it doesn't really need anything clang-related), and this 
would certainly make that impossible. Having Core depend on clang does not 
worry me much because it depends on so many things already, so the way I was 
thinking of resolving that is to move low-level things out of it (my tentative 
list includes things like Event/Listener/Broadcaster, State, Communication). 
That would leave Core with containing only the high-level stuff for which a 
clang dependency is not that surprising (although I agree that a ModuleList is 
not the best place for such a dependency).

I guess it would be nice to encapsulate this in some sort of a plugin (since 
the setting is used from the clang expression parser plugin, I guess this would 
be the natural home for it) , but I haven't looked in detail at could that 
work. What I do know is that we already have the ability to inject settings 
from within a plugin (see SymbolFileDWARF::DebuggerInitialize). Maybe that 
would work here too?

PS: This patch would make the clang modules path similar to the python 
directory computation, but this is also something that I've been meaning to 
change somehow (lldb-server does not need python either).




Comment at: source/Core/ModuleList.cpp:88
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  SetClangModulesCachePath(HostInfoBase::GetDefaultClangModuleCachePath());
 }

You should reference this through the `HostInfo` typedef to get the "static 
polymorphism" to work.


https://reviews.llvm.org/D47235



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

Re: [Lldb-commits] [lldb] r332111 - HostInfoMacOSX: Share the clang resource directory with Swift.

[Pavel Labath via lldb-commits]

Btw, r333032 gave me an idea. We already have unittests/Host/linux/*** to
contain linux-specific host tests (although one of them maybe shouldn't be
there). What would you say to moving this test to unittests/Host/macosx so
we can avoid the #ifdefs?
On Fri, 11 May 2018 at 18:57, Adrian Prantl via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: adrian
> Date: Fri May 11 10:54:09 2018
> New Revision: 332111

> URL: http://llvm.org/viewvc/llvm-project?rev=332111&view=rev
> Log:
> HostInfoMacOSX: Share the clang resource directory with Swift.

> Inside Xcode and in Xcode toolchains LLDB is always in lockstep
> with the Swift compiler, so it can reuse its Clang resource
> directory. This allows LLDB and the Swift compiler to share the
> same Clang module cache.

> rdar://problem/40039633

> Differential Revision: https://reviews.llvm.org/D46736

> Modified:
>  lldb/trunk/include/lldb/Host/macosx/HostInfoMacOSX.h
>  lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm
>  lldb/trunk/unittests/Host/CMakeLists.txt
>  lldb/trunk/unittests/Host/HostInfoTest.cpp

> Modified: lldb/trunk/include/lldb/Host/macosx/HostInfoMacOSX.h
> URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/macosx/HostInfoMacOSX.h?rev=332111&r1=332110&r2=332111&view=diff

==
> --- lldb/trunk/include/lldb/Host/macosx/HostInfoMacOSX.h (original)
> +++ lldb/trunk/include/lldb/Host/macosx/HostInfoMacOSX.h Fri May 11
10:54:09 2018
> @@ -38,6 +38,8 @@ protected:
> static bool ComputeHeaderDirectory(FileSpec &file_spec);
> static bool ComputePythonDirectory(FileSpec &file_spec);
> static bool ComputeClangDirectory(FileSpec &file_spec);
> +  static bool ComputeClangDirectory(FileSpec &lldb_shlib_spec,
> +FileSpec &file_spec, bool verify);
> static bool ComputeSystemPluginsDirectory(FileSpec &file_spec);
> static bool ComputeUserPluginsDirectory(FileSpec &file_spec);
>   };

> Modified: lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm
> URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm?rev=332111&r1=332110&r2=332111&view=diff

==
> --- lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm (original)
> +++ lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm Fri May 11 10:54:09
2018
> @@ -19,6 +19,7 @@

>   #include "llvm/ADT/SmallString.h"
>   #include "llvm/Support/FileSystem.h"
> +#include "llvm/Support/Path.h"
>   #include "llvm/Support/raw_ostream.h"

>   // C++ Includes
> @@ -226,21 +227,67 @@ bool HostInfoMacOSX::ComputePythonDirect
>   #endif
>   }

> +static bool VerifyClangPath(const llvm::Twine &clang_path) {
> +  if (llvm::sys::fs::is_directory(clang_path))
> +return true;
> +  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
> +  if (log)
> +log->Printf("HostInfoMacOSX::ComputeClangDirectory(): "
> +"failed to stat clang resource directory at \"%s\"",
> +clang_path.str().c_str());
> +  return false;
> +}
> +
>   bool HostInfoMacOSX::ComputeClangDirectory(FileSpec &file_spec) {
> FileSpec lldb_file_spec;
> if (!GetLLDBPath(lldb::ePathTypeLLDBShlibDir, lldb_file_spec))
>   return false;
> +  return ComputeClangDirectory(lldb_file_spec, file_spec, true);
> +}

> -  std::string raw_path = lldb_file_spec.GetPath();
> +bool HostInfoMacOSX::ComputeClangDirectory(FileSpec &lldb_shlib_spec,
> +   FileSpec &file_spec, bool
verify) {
> +  std::string raw_path = lldb_shlib_spec.GetPath();

> -  size_t framework_pos = raw_path.find("LLDB.framework");
> -  if (framework_pos == std::string::npos)
> +  auto rev_it = llvm::sys::path::rbegin(raw_path);
> +  auto r_end = llvm::sys::path::rend(raw_path);
> +
> +  // Check for a Posix-style build of LLDB.
> +  if (rev_it == r_end || *rev_it != "LLDB.framework")
>   return HostInfoPosix::ComputeClangDirectory(file_spec);
> -
> -  framework_pos += strlen("LLDB.framework");
> -  raw_path.resize(framework_pos);
> +
> +  // Inside Xcode and in Xcode toolchains LLDB is always in lockstep
> +  // with the Swift compiler, so it can reuse its Clang resource
> +  // directory. This allows LLDB and the Swift compiler to share the
> +  // same Clang module cache.
> +  llvm::SmallString<256> clang_path;
> +  const char *swift_clang_resource_dir = "usr/lib/swift/clang";
> +  ++rev_it;
> +  if (rev_it != r_end && *rev_it == "SharedFrameworks") {
> +// This is the top-level LLDB in the Xcode.app bundle.
> +raw_path.resize(rev_it - r_end);
> +llvm::sys::path::append(clang_path, raw_path,
> +
  "Developer/Toolchains/XcodeDefault.xctoolchain",
> +swift_clang_resource_dir);
> +if (!verify || VerifyClangPath(clang_path)) {
> +  file_spec.SetFile(clang_path.c_str(), true);
> +  return true;
> +   

[Lldb-commits] [lldb] r333073 - ProcessLauncherPosixFork: move setgid call into the if(debug) branch

[Pavel Labath via lldb-commits]

Author: labath
Date: Wed May 23 03:10:36 2018
New Revision: 333073

URL: http://llvm.org/viewvc/llvm-project?rev=333073&view=rev
Log:
ProcessLauncherPosixFork: move setgid call into the if(debug) branch

This call was originally being only made when launching for debug (as an
attempt to make sure we don't impart extra privileges on the launched
process), but after the debug and non-debug paths were merged, it made
it's way into generic code. This was causing problems in locked down
android environments which disallowed calling setgid even if it would be
a no-op. This prevented launching llgs from lldb-server platform.

Overall I'm not sure we should be calling setgid in the first place
(it seems random -- e.g. why don't we call setuid then as well).
However, all our other copies of launch code have it, so I choose to
keep it for now.

Modified:
lldb/trunk/source/Host/posix/ProcessLauncherPosixFork.cpp

Modified: lldb/trunk/source/Host/posix/ProcessLauncherPosixFork.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/ProcessLauncherPosixFork.cpp?rev=333073&r1=333072&r2=333073&view=diff
==
--- lldb/trunk/source/Host/posix/ProcessLauncherPosixFork.cpp (original)
+++ lldb/trunk/source/Host/posix/ProcessLauncherPosixFork.cpp Wed May 23 
03:10:36 2018
@@ -90,10 +90,6 @@ static void DupDescriptor(int error_fd,
 
 static void LLVM_ATTRIBUTE_NORETURN ChildFunc(int error_fd,
   const ProcessLaunchInfo &info) {
-  // Do not inherit setgid powers.
-  if (setgid(getgid()) != 0)
-ExitWithError(error_fd, "setgid");
-
   if (info.GetFlags().Test(eLaunchFlagLaunchInSeparateProcessGroup)) {
 if (setpgid(0, 0) != 0)
   ExitWithError(error_fd, "setpgid");
@@ -139,6 +135,10 @@ static void LLVM_ATTRIBUTE_NORETURN Chil
 ExitWithError(error_fd, "pthread_sigmask");
 
   if (info.GetFlags().Test(eLaunchFlagDebug)) {
+// Do not inherit setgid powers.
+if (setgid(getgid()) != 0)
+  ExitWithError(error_fd, "setgid");
+
 // HACK:
 // Close everything besides stdin, stdout, and stderr that has no file
 // action to avoid leaking. Only do this when debugging, as elsewhere we


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

[Lldb-commits] [lldb] r333074 - Fix PathMappingList tests on windows

[Pavel Labath via lldb-commits]

Author: labath
Date: Wed May 23 03:32:05 2018
New Revision: 333074

URL: http://llvm.org/viewvc/llvm-project?rev=333074&view=rev
Log:
Fix PathMappingList tests on windows

The tests added in r332842 don't work on windows, because they do path
comparisons on strings, and on windows, the paths coming out of the
mappings had backslashes in them.

This switches comparisons to FileSpecs, so the results come out right.

Modified:
lldb/trunk/unittests/Target/PathMappingListTest.cpp

Modified: lldb/trunk/unittests/Target/PathMappingListTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Target/PathMappingListTest.cpp?rev=333074&r1=333073&r2=333074&view=diff
==
--- lldb/trunk/unittests/Target/PathMappingListTest.cpp (original)
+++ lldb/trunk/unittests/Target/PathMappingListTest.cpp Wed May 23 03:32:05 2018
@@ -16,31 +16,30 @@
 using namespace lldb_private;
 
 namespace {
-  struct Matches {
-ConstString original;
-ConstString remapped;
-Matches(const char *o, const char *r) : original(o),
-remapped(r) {}
-  };
-  
-  void TestPathMappings(const PathMappingList &map,
-llvm::ArrayRef matches,
-llvm::ArrayRef fails) {
-ConstString actual_remapped;
-for (const auto &fail: fails) {
-  EXPECT_FALSE(map.RemapPath(fail, actual_remapped));
-}
-for (const auto &match: matches) {
-  FileSpec orig_spec(match.original.GetStringRef(), false);
-  std::string orig_normalized = orig_spec.GetPath();
-  EXPECT_TRUE(map.RemapPath(match.original, actual_remapped));
-  EXPECT_EQ(actual_remapped, match.remapped);
-  FileSpec remapped_spec(match.remapped.GetStringRef(), false);
-  FileSpec unmapped_spec;
-  EXPECT_TRUE(map.ReverseRemapPath(remapped_spec, unmapped_spec));
-  std::string unmapped_path = unmapped_spec.GetPath();
-  EXPECT_EQ(unmapped_path, orig_normalized);
-}
+struct Matches {
+  FileSpec original;
+  FileSpec remapped;
+  Matches(const char *o, const char *r)
+  : original(o, false), remapped(r, false) {}
+};
+} // namespace
+
+static void TestPathMappings(const PathMappingList &map,
+ llvm::ArrayRef matches,
+ llvm::ArrayRef fails) {
+  ConstString actual_remapped;
+  for (const auto &fail : fails) {
+EXPECT_FALSE(map.RemapPath(fail, actual_remapped));
+  }
+  for (const auto &match : matches) {
+std::string orig_normalized = match.original.GetPath();
+EXPECT_TRUE(
+map.RemapPath(ConstString(match.original.GetPath()), actual_remapped));
+EXPECT_EQ(FileSpec(actual_remapped.GetStringRef(), false), match.remapped);
+FileSpec unmapped_spec;
+EXPECT_TRUE(map.ReverseRemapPath(match.remapped, unmapped_spec));
+std::string unmapped_path = unmapped_spec.GetPath();
+EXPECT_EQ(unmapped_path, orig_normalized);
   }
 }
 


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

[Lldb-commits] [PATCH] D47250: Move ObjectFile initialization out of SystemInitializerCommon

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: zturner, clayborg.
Herald added a subscriber: mgorny.

For lldb-server, it is sufficient to parse only the native object file
format for its target OS (no other file can be loaded into a running
process). This moves the object file initialization code into specific
initializer classes: lldb-test and liblldb get all object files;
lldb-server gets only one of them. For this to work, I've needed to
create a special SystemInitializer for use in lldb-server, instead of it
calling directly into the common one.

This reduces the size of lldb-server by about 2%, which is not
earth-shattering, but it's an easy win, and it helps.


https://reviews.llvm.org/D47250

Files:
  source/API/SystemInitializerFull.cpp
  source/Initialization/CMakeLists.txt
  source/Initialization/SystemInitializerCommon.cpp
  tools/lldb-server/CMakeLists.txt
  tools/lldb-server/SystemInitializerLLGS.cpp
  tools/lldb-server/SystemInitializerLLGS.h
  tools/lldb-server/lldb-server.cpp
  tools/lldb-test/SystemInitializerTest.cpp

Index: tools/lldb-test/SystemInitializerTest.cpp
===
--- tools/lldb-test/SystemInitializerTest.cpp
+++ tools/lldb-test/SystemInitializerTest.cpp
@@ -60,6 +60,9 @@
 #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h"
 #include "Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h"
 #include "Plugins/MemoryHistory/asan/MemoryHistoryASan.h"
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
+#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
 #include "Plugins/OperatingSystem/Go/OperatingSystemGo.h"
 #include "Plugins/Platform/Android/PlatformAndroid.h"
 #include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
@@ -119,6 +122,11 @@
 
 void SystemInitializerTest::Initialize() {
   SystemInitializerCommon::Initialize();
+
+  ObjectFileELF::Initialize();
+  ObjectFileMachO::Initialize();
+  ObjectFilePECOFF::Initialize();
+
   ScriptInterpreterNone::Initialize();
 
   OperatingSystemGo::Initialize();
@@ -345,6 +353,10 @@
   PlatformDarwinKernel::Terminate();
 #endif
 
+  ObjectFileELF::Terminate();
+  ObjectFileMachO::Terminate();
+  ObjectFilePECOFF::Terminate();
+
   // Now shutdown the common parts, in reverse order.
   SystemInitializerCommon::Terminate();
 }
Index: tools/lldb-server/lldb-server.cpp
===
--- tools/lldb-server/lldb-server.cpp
+++ tools/lldb-server/lldb-server.cpp
@@ -7,7 +7,7 @@
 //
 //===--===//
 
-#include "lldb/Initialization/SystemInitializerCommon.h"
+#include "SystemInitializerLLGS.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
 #include "lldb/lldb-private.h"
 
@@ -35,8 +35,8 @@
 int main_platform(int argc, char *argv[]);
 
 static void initialize() {
-  g_debugger_lifetime->Initialize(
-  llvm::make_unique(), nullptr);
+  g_debugger_lifetime->Initialize(llvm::make_unique(),
+  nullptr);
 }
 
 static void terminate() { g_debugger_lifetime->Terminate(); }
Index: tools/lldb-server/SystemInitializerLLGS.h
===
--- /dev/null
+++ tools/lldb-server/SystemInitializerLLGS.h
@@ -0,0 +1,21 @@
+//===-- SystemInitializerLLGS.h -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLDB_SYSTEMINITIALIZERLLGS_H
+#define LLDB_SYSTEMINITIALIZERLLGS_H
+
+#include "lldb/Initialization/SystemInitializerCommon.h"
+
+class SystemInitializerLLGS : public lldb_private::SystemInitializerCommon {
+public:
+  void Initialize() override;
+  void Terminate() override;
+};
+
+#endif // LLDB_SYSTEMINITIALIZERLLGS_H
Index: tools/lldb-server/SystemInitializerLLGS.cpp
===
--- /dev/null
+++ tools/lldb-server/SystemInitializerLLGS.cpp
@@ -0,0 +1,33 @@
+//===-- SystemInitializerLLGS.cpp ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "SystemInitializerLLGS.h"
+
+#if defined(__APPLE__)
+#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
+using HostObjectFile = lldb_private::ObjectFileMachO;
+#elif defined(_WIN32)
+#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+using HostObjectFile = lldb_private::ObjectFilePECOFF;
+#else
+#include "Plugins/ObjectFile/ELF/Object

[Lldb-commits] [PATCH] D47253: DWARF: Move indexing code from DWARFUnit to ManualDWARFIndex

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, clayborg, aprantl.

I think this makes sense for several reasons:

- better separation of concerns: DWARFUnit's job should be to provide a nice 
interface to its users to access the unit contents. ManualDWARFIndex can then 
use this interface to build an index and provide it to its users.
- closer alignment with llvm parsers: there is no indexing equivalent in llvm, 
and there probably never will be, as the index is very centered around how lldb 
wants to access debug info. If we ever switch to llvm's parser, this will allow 
us swap out DWARFUnit implementations and keep indexing as-is.
- closer proximity of the indexing code to AppleDWARFIndex will make it easier 
to keep the two in sync (e.g. right now the two use very different algorithms 
to determine whether a DW_TAG_subroutine represents a "method"). This is my 
primary motivation for making this change now, but I am leaving this work to a 
separate patch.

The only interface change to DWARFUnit I needed to make was to add an
efficient way to iterate over the list of all DIEs. Adding this also
aligns us closer to the llvm parser.


https://reviews.llvm.org/D47253

Files:
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h

Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -53,6 +53,18 @@
 
 private:
   void Index();
+  void IndexUnit(DWARFUnit &unit, NameToDIE &func_basenames,
+ NameToDIE &func_fullnames, NameToDIE &func_methods,
+ NameToDIE &func_selectors, NameToDIE &objc_class_selectors,
+ NameToDIE &globals, NameToDIE &types, NameToDIE &namespaces);
+
+  static void
+  IndexUnitImpl(DWARFUnit &unit, const lldb::LanguageType cu_language,
+const DWARFFormValue::FixedFormSizes &fixed_form_sizes,
+const dw_offset_t cu_offset, NameToDIE &func_basenames,
+NameToDIE &func_fullnames, NameToDIE &func_methods,
+NameToDIE &func_selectors, NameToDIE &objc_class_selectors,
+NameToDIE &globals, NameToDIE &types, NameToDIE &namespaces);
 
   /// Non-null value means we haven't built the index yet.
   DWARFDebugInfo *m_debug_info;
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -8,8 +8,11 @@
 //===--===//
 
 #include "Plugins/SymbolFile/DWARF/ManualDWARFIndex.h"
+#include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
+#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Host/TaskPool.h"
 #include "lldb/Symbol/ObjectFile.h"
@@ -48,10 +51,10 @@
   auto parser_fn = [&](size_t cu_idx) {
 DWARFUnit *dwarf_cu = debug_info.GetCompileUnitAtIndex(cu_idx);
 if (dwarf_cu) {
-  dwarf_cu->Index(function_basenames[cu_idx], function_fullnames[cu_idx],
-  function_methods[cu_idx], function_selectors[cu_idx],
-  objc_class_selectors[cu_idx], globals[cu_idx],
-  types[cu_idx], namespaces[cu_idx]);
+  IndexUnit(*dwarf_cu, function_basenames[cu_idx],
+function_fullnames[cu_idx], function_methods[cu_idx],
+function_selectors[cu_idx], objc_class_selectors[cu_idx],
+globals[cu_idx], types[cu_idx], namespaces[cu_idx]);
 }
   };
 
@@ -108,6 +111,324 @@
   }
 }
 
+void ManualDWARFIndex::IndexUnit(DWARFUnit &unit, NameToDIE &func_basenames,
+ NameToDIE &func_fullnames,
+ NameToDIE &func_methods,
+ NameToDIE &func_selectors,
+ NameToDIE &objc_class_selectors,
+ NameToDIE &globals, NameToDIE &types,
+ NameToDIE &namespaces) {
+  Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_LOOKUPS);
+
+  if (log) {
+m_module.LogMessage(
+log, "ManualDWARFIndex::IndexUnit for compile unit at .debug_info[0x%8.8x]",
+unit.GetOffset());
+  }
+
+  const LanguageType cu_language = unit.GetLanguageType();
+  DWARFFormValue::FixedFormSizes fixed_form_sizes = unit.GetFixedFormSizes();
+
+  IndexUnitImpl(unit, cu_language, fixed_form_sizes, unit.GetOffset(),
+func_basename

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I've given this some more though as I see valid points and concerns on both 
sides of the argument. I'm leaning towards running test cases in parallel 
because it offloads more work to lit at negligible cost (running make and 
launching the inferior are the biggest time consumers).

Other advantages are that:

- It would also make it at lot easier to rerun a single test case because lit 
is in a better position to inform us how to reproduce a failure.
- It would give a more granular overview of what passed, failed or skipped. The 
cmake bot on GreenDragon runs the test suite with lit and that means that 
failures are only reported at file level, which imho is to coarse.


https://reviews.llvm.org/D46005



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

[Lldb-commits] [PATCH] D47062: Suggest lldb-dotest to reproduce a failure.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I'm going to hold off on this until we have decided what to do for 
https://reviews.llvm.org/D46005. If we run the test cases as separate lit 
invocations, then the test format is in a better position to report this 
information.


https://reviews.llvm.org/D47062



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

[Lldb-commits] [PATCH] D47250: Move ObjectFile initialization out of SystemInitializerCommon

[Greg Clayton via Phabricator via lldb-commits]

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

LGTM


https://reviews.llvm.org/D47250



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

[Lldb-commits] [PATCH] D47253: DWARF: Move indexing code from DWARFUnit to ManualDWARFIndex

[Greg Clayton via Phabricator via lldb-commits]

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

LGTM


https://reviews.llvm.org/D47253



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D47235#1109219, @labath wrote:

> In a way, this makes sense, but in a lot of other ways, it actually makes 
> things worse :)
>
> My long-term goal is to be able to build lldb-server without even having 
> clang checked out (because it doesn't really need anything clang-related), 
> and this would certainly make that impossible. Having Core depend on clang 
> does not worry me much because it depends on so many things already, so the 
> way I was thinking of resolving that is to move low-level things out of it 
> (my tentative list includes things like Event/Listener/Broadcaster, State, 
> Communication). That would leave Core with containing only the high-level 
> stuff for which a clang dependency is not that surprising (although I agree 
> that a ModuleList is not the best place for such a dependency).


Agree, but just because `Core` is already a problem doesn't mean we should just 
ignore it IMO.  At some point we're going to have to do something about it, 
even if it's not that surprising for `Core` to have a dependency on clang at 
some point in the future, it will *almost always* be surprising for two things 
to have a dependency on each other.  So from that angle, we need to be vigilant 
in outgoing dependencies from `Core`.

> I guess it would be nice to encapsulate this in some sort of a plugin (since 
> the setting is used from the clang expression parser plugin, I guess this 
> would be the natural home for it) , but I haven't looked in detail at could 
> that work. What I do know is that we already have the ability to inject 
> settings from within a plugin (see SymbolFileDWARF::DebuggerInitialize). 
> Maybe that would work here too?

I agree that a clang plugin seems like the "real" solution, I had come to the 
same conclusion yesterday.  But that is a significant amount of work obviously.

That's why I proposed in the other thread the idea of having a function called 
`Module::setDefaultClangModulesPath(const Twine&)` and just call that method in 
`SystemInitializerFull`.  That's very similar in spirit to how the plugins work 
anyway.  During initialization we call global functions on all of the plugins 
to have them initialize themselves, and they're free to muck with the world 
during this initialization.  So this solution is, IMO, an incremental step 
towards the "real" solution while still not making the problem worse.  If/when 
someone makes a real clang plugin, they just copy this code into that plugins 
initialize method.


https://reviews.llvm.org/D47235



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

Re: [Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Adrian Prantl via lldb-commits]

> On May 22, 2018, at 6:57 PM, Zachary Turner  wrote:
> 
> Yea I don’t think this addresses the problem. We should be able to link 
> against parts of lldb without a dependency on clang. Since this is about 
> configuring something related to clang, it seems like it should be isolated 
> to some part of lldb that interfaces with clang 

And this is the point where I need some help :-)
The intended layering of LLDB is not at all clear to me. Which LLDB libraries 
are allowed to link against clang? Do we have a diagram somewhere?

-- adrian

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

[Lldb-commits] [PATCH] D47265: WIP: lit: Run each test separately

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
Herald added subscribers: JDevlieghere, eraman.

This needs to be cleaned up before we can consider submitting it.
However, I am putting it out here, so we you can test it's impact on
your setup.

PS: This does not prevent the debug-info replication for "inline" tests.
It just implements it more nicely.


https://reviews.llvm.org/D47265

Files:
  lit/Suite/lldbtest.py
  packages/Python/lldbsuite/test/example/TestSequenceFunctions.py
  packages/Python/lldbsuite/test/lldbinline.py

Index: packages/Python/lldbsuite/test/lldbinline.py
===
--- packages/Python/lldbsuite/test/lldbinline.py
+++ packages/Python/lldbsuite/test/lldbinline.py
@@ -135,37 +135,11 @@
 makefile.flush()
 makefile.close()
 
-@add_test_categories(["dsym"])
-def __test_with_dsym(self):
+def _test(self):
 self.using_dsym = True
 self.BuildMakefile()
 self.build()
 self.do_test()
-__test_with_dsym.debug_info = "dsym"
-
-@add_test_categories(["dwarf"])
-def __test_with_dwarf(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwarf.debug_info = "dwarf"
-
-@add_test_categories(["dwo"])
-def __test_with_dwo(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwo.debug_info = "dwo"
-
-@add_test_categories(["gmodules"])
-def __test_with_gmodules(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_gmodules.debug_info = "gmodules"
 
 def execute_user_command(self, __command):
 exec(__command, globals(), locals())
@@ -237,18 +211,13 @@
 InlineTest.mydir = TestBase.compute_mydir(__file)
 
 test_name, _ = os.path.splitext(file_basename)
+
+test = ApplyDecoratorsToFunction(
+InlineTest._test, decorators)
 # Build the test case
-test = type(test_name, (InlineTest,), {'using_dsym': None})
+test = type(test_name, (InlineTest,), dict(using_dsym=None, test=test))
 test.name = test_name
 
-test.test_with_dsym = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dsym, decorators)
-test.test_with_dwarf = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwarf, decorators)
-test.test_with_dwo = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwo, decorators)
-test.test_with_gmodules = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_gmodules, decorators)
 
 # Add the test case to the globals, and hide InlineTest
 __globals.update({test_name: test})
Index: packages/Python/lldbsuite/test/example/TestSequenceFunctions.py
===
--- packages/Python/lldbsuite/test/example/TestSequenceFunctions.py
+++ /dev/null
@@ -1,34 +0,0 @@
-"""An example unittest copied from python tutorial."""
-
-import random
-import unittest
-import traceback
-
-
-class SequenceFunctionsTestCase(unittest.TestCase):
-
-def setUp(self):
-# traceback.print_stack()
-self.seq = list(range(10))
-
-def tearDown(self):
-# traceback.print_stack()
-pass
-
-def test_shuffle(self):
-# make sure the shuffled sequence does not lose any elements
-random.shuffle(self.seq)
-self.seq.sort()
-self.assertEqual(self.seq, list(range(10)))
-
-def test_choice(self):
-element = random.choice(self.seq)
-self.assertTrue(element in self.seq)
-
-def test_sample(self):
-self.assertRaises(ValueError, random.sample, self.seq, 20)
-for element in random.sample(self.seq, 5):
-self.assertTrue(element in self.seq)
-
-if __name__ == '__main__':
-unittest.main()
Index: lit/Suite/lldbtest.py
===
--- lit/Suite/lldbtest.py
+++ lit/Suite/lldbtest.py
@@ -13,38 +13,41 @@
 class LLDBTest(TestFormat):
 def __init__(self, dotest_cmd):
 self.dotest_cmd = dotest_cmd
+self.tests = None
+
+def getTests(self, bin_dir):
+if self.tests:
+return self.tests
+dumper = os.path.join(os.path.dirname(__file__), "lldbtestdumper.py")
+lldb_path = os.path.join(bin_dir, "lldb")
+output = subprocess.check_output([sys.executable, dumper, lldb_path])
+self.tests = [line.split() for line in output.splitlines()]
+return self.tests
+
 
 def getTestsInDirectory(self, testSuite, path_in_suite, litConfig,
 localConfig):
-source_path = testSuite.getSourcePath(path_in_suite)
-for filename in os.listdir(source_path):
-# Ignore dot files and excluded tests.
-if (filename.startswith('.') or filename in localConfig.excludes)

Re: [Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via lldb-commits]

There's not really a diagram, because we don't have an exact vision of what
the final layering is going to look like (some things will need to be split
up, entirely new targets will need to be introduced, etc).  Mostly it's
just built from experience based on what the primary logical function of a
target is, and then asking whether or not someone who wishes to use that
functionality should be required to link in all of that target's current
dependencies.  And hoping that a layering emerges from this process, at
which point we can then make a diagram as well as enforce it through CMake
/ LLVMBuild / etc.

Core is the fundamental target that contains the basic data structures
representing a generic debugger.  A generic debugger shouldn't need to
depend on clang.  Someone should be able to (in theory) link against Core
(and perhaps a few other targets) and build a Java debugger.  Or a debugger
for an embedded platform.  So a dependency on clang doesn't belong there.

LLDB is a specific debugger which is built on top of Core and other
libraries, and so that is why I think it makes sense to expose a static
function in Module which allows you to set the clang modules path, and then
we do that during LLDB initialization.  For the record, that still isn't
correct from a purity standpoint, because there is a method in Core which
claims to have something to do with clang modules.  But at least it's
relatively benign.

In any case, what do you think about that approach?

On Wed, May 23, 2018 at 8:42 AM Adrian Prantl  wrote:

>
>
> > On May 22, 2018, at 6:57 PM, Zachary Turner  wrote:
> >
> > Yea I don’t think this addresses the problem. We should be able to link
> against parts of lldb without a dependency on clang. Since this is about
> configuring something related to clang, it seems like it should be isolated
> to some part of lldb that interfaces with clang
>
> And this is the point where I need some help :-)
> The intended layering of LLDB is not at all clear to me. Which LLDB
> libraries are allowed to link against clang? Do we have a diagram somewhere?
>
> -- adrian
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

If the majority of active contributors think this is the right direction then 
don't let me hold it up.  I mostly just wanted to raise the concern, but if 
you've evaluated the pros and cons and made a decision, I'm fine with that :)


https://reviews.llvm.org/D46005



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

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thank you for the support Jonas. I've quickly prototyped a patch 
(https://reviews.llvm.org/D47265) of how would this integrate with lit. The 
nice part about it is that it is very small. If you consider the inline test 
refactor it even has negative LOC. It would definitely have negative LOC impact 
if this would allow us to remote test driver parts from dotest.

The not-so-nice part about it is that is has more impact on test running time 
than I expected. For me the time to run the tests goes from 1m38s to 2m30s. I 
am not extremely worried about that as that is still pretty fast for me (*), 
but I don't know how that scales, so it could be that this makes a significant 
difference for people with slower machines. I encourage you to give it a try 
(you need to apply it together with this patch), and let me know what you think.

(*) I should also point out that the competing solution of putting every test 
into it's own file would likely have similar impact, as this is mostly due to 
fixed startup cost of spinning up a python +lldb process for the test. I also 
hope that some of this slowdown can be reclaimed as we garbage collect parts of 
dotest that would become unneeded after this.


https://reviews.llvm.org/D46005



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

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

FWIW, I think the single biggest improvement one could make to the LLDB test 
suite runtime is to compile all inferiors up front as part of the CMake step.  
If you run the test suite twice every inferior is unnecessarily compiled twice.


https://reviews.llvm.org/D46005



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D47235#1109580, @zturner wrote:

> In https://reviews.llvm.org/D47235#1109219, @labath wrote:
>
> > I guess it would be nice to encapsulate this in some sort of a plugin 
> > (since the setting is used from the clang expression parser plugin, I guess 
> > this would be the natural home for it) , but I haven't looked in detail at 
> > could that work. What I do know is that we already have the ability to 
> > inject settings from within a plugin (see 
> > SymbolFileDWARF::DebuggerInitialize). Maybe that would work here too?
>
>
> I agree that a clang plugin seems like the "real" solution, I had come to the 
> same conclusion yesterday.  But that is a significant amount of work 
> obviously.


Is it that much work? We already have a clang (well, "clang expression parser" 
plugin). Getting **all** of the clang dependencies into that plugin is a 
completely different story, but I am hoping that simply getting this particular 
setting to live there would be just a matter of cargo-culting some code from 
SymbolFileDWARF.


https://reviews.llvm.org/D47235



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

Re: [Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Adrian Prantl via lldb-commits]

> On May 23, 2018, at 8:51 AM, Zachary Turner  wrote:
> 
> There's not really a diagram, because we don't have an exact vision of what 
> the final layering is going to look like (some things will need to be split 
> up, entirely new targets will need to be introduced, etc).  Mostly it's just 
> built from experience based on what the primary logical function of a target 
> is, and then asking whether or not someone who wishes to use that 
> functionality should be required to link in all of that target's current 
> dependencies.  And hoping that a layering emerges from this process, at which 
> point we can then make a diagram as well as enforce it through CMake / 
> LLVMBuild / etc.
> 
> Core is the fundamental target that contains the basic data structures 
> representing a generic debugger.  A generic debugger shouldn't need to depend 
> on clang.  Someone should be able to (in theory) link against Core (and 
> perhaps a few other targets) and build a Java debugger.  Or a debugger for an 
> embedded platform.  So a dependency on clang doesn't belong there.
> 
> LLDB is a specific debugger which is built on top of Core and other 
> libraries, and so that is why I think it makes sense to expose a static 
> function in Module which allows you to set the clang modules path, and then 
> we do that during LLDB initialization.

Could you point me at a specific source file? It's not obvious to me what "LLDB 
initialization" means to you.

-- adrian

>   For the record, that still isn't correct from a purity standpoint, because 
> there is a method in Core which claims to have something to do with clang 
> modules.  But at least it's relatively benign.
> 
> In any case, what do you think about that approach?  
> 
> On Wed, May 23, 2018 at 8:42 AM Adrian Prantl  > wrote:
> 
> 
> > On May 22, 2018, at 6:57 PM, Zachary Turner  > > wrote:
> > 
> > Yea I don’t think this addresses the problem. We should be able to link 
> > against parts of lldb without a dependency on clang. Since this is about 
> > configuring something related to clang, it seems like it should be isolated 
> > to some part of lldb that interfaces with clang 
> 
> And this is the point where I need some help :-)
> The intended layering of LLDB is not at all clear to me. Which LLDB libraries 
> are allowed to link against clang? Do we have a diagram somewhere?
> 
> -- adrian
> 

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

Re: [Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via lldb-commits]

SystemInitializerFull.cpp, in the
function SystemInitializerFull::Initialize().

On Wed, May 23, 2018 at 9:44 AM Adrian Prantl  wrote:

>
>
> On May 23, 2018, at 8:51 AM, Zachary Turner  wrote:
>
> There's not really a diagram, because we don't have an exact vision of
> what the final layering is going to look like (some things will need to be
> split up, entirely new targets will need to be introduced, etc).  Mostly
> it's just built from experience based on what the primary logical function
> of a target is, and then asking whether or not someone who wishes to use
> that functionality should be required to link in all of that target's
> current dependencies.  And hoping that a layering emerges from this
> process, at which point we can then make a diagram as well as enforce it
> through CMake / LLVMBuild / etc.
>
> Core is the fundamental target that contains the basic data structures
> representing a generic debugger.  A generic debugger shouldn't need to
> depend on clang.  Someone should be able to (in theory) link against Core
> (and perhaps a few other targets) and build a Java debugger.  Or a debugger
> for an embedded platform.  So a dependency on clang doesn't belong there.
>
> LLDB is a specific debugger which is built on top of Core and other
> libraries, and so that is why I think it makes sense to expose a static
> function in Module which allows you to set the clang modules path, and then
> we do that during LLDB initialization.
>
>
> Could you point me at a specific source file? It's not obvious to me what
> "LLDB initialization" means to you.
>
> -- adrian
>
>   For the record, that still isn't correct from a purity standpoint,
> because there is a method in Core which claims to have something to do with
> clang modules.  But at least it's relatively benign.
>
> In any case, what do you think about that approach?
>
> On Wed, May 23, 2018 at 8:42 AM Adrian Prantl  wrote:
>
>>
>>
>> > On May 22, 2018, at 6:57 PM, Zachary Turner  wrote:
>> >
>> > Yea I don’t think this addresses the problem. We should be able to link
>> against parts of lldb without a dependency on clang. Since this is about
>> configuring something related to clang, it seems like it should be isolated
>> to some part of lldb that interfaces with clang
>>
>> And this is the point where I need some help :-)
>> The intended layering of LLDB is not at all clear to me. Which LLDB
>> libraries are allowed to link against clang? Do we have a diagram somewhere?
>>
>> -- adrian
>>
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In https://reviews.llvm.org/D46005#1109693, @zturner wrote:

> FWIW, I think the single biggest improvement one could make to the LLDB test 
> suite runtime is to compile all inferiors up front as part of the CMake step. 
>  If you run the test suite twice every inferior is unnecessarily compiled 
> twice.


I'm a big proponent of moving as much logic as possible into the configuration 
stage, but a common use case (at least for us) is testing a single build with a 
different compiler/configuration.

> I think that would shave off up to 75% of the test suite runtime.


https://reviews.llvm.org/D46005



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

Re: [Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

[Jim Ingham via lldb-commits]

The expression command had two modes before introducing the REPL.  You can do:

(lldb) expr -- some C expression

or you can do:

(lldb) expr
Enter expressions, then terminate with an empty line to evaluate:
  1: first C expression
  2: Second C Expression
  3: 

The second only differs from the first in that it pushes an IOHandler to deal 
with gathering the entered text that the user typed up to the point where they 
type an empty expression.

The repl adds a third mode:

(lldb) expr -r --
  1> let a = 10
a: Int = 10
  2> print(a)
10
  3>  

Which works the same as "expr --toplevel" with the addition of a fancier 
IOHandler that looks for complete expressions and evaluates each completed 
expression as it sees it.  I'm glossing over a few details here (like the fact 
that ":" will switch you to the lldb command interpreter), but for the most 
part that's all it is.

Note, there's another "repl" that lldb provides:

 > lldb --repl

Which packages up "make a target out of a  little victim program we've stashed 
away in LLDB.framework, set a breakpoint in it, run to the breakpoint and then 
execute "expr -r --".  So there's also a little bit to support that.  But I 
don't think that materially changes how you should understand the REPL.  And 
then most people actually access this for swift by invoking "swift" with no 
arguments, but the swift driver with no arguments just turns around and exec's 
"lldb --repl" so that doesn't add much besides convenience.

Jim


 


> On May 23, 2018, at 1:46 AM, Pavel Labath via Phabricator 
>  wrote:
> 
> labath added a comment.
> 
> Both of the solutions sound plausible to me (extra struct vs. moving REPL to 
> `Command`). Maybe that's because I don't know enough about the REPL to have 
> formed an opinion on it.
> 
> 
> 
> 
> Comment at: lldb/include/lldb/Expression/REPL.h:31
> +bool TryAllThreads = true;
> +uint32_t Timeout = 0;
> +  };
> 
> If you stick with this solution, please make this `Timeout`
> 
> 
> https://reviews.llvm.org/D47232
> 
> 
> 

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

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

[Jim Ingham via Phabricator via lldb-commits]

jingham added a subscriber: zturner.
jingham added a comment.

The expression command had two modes before introducing the REPL.  You can do:

(lldb) expr -- some C expression

or you can do:

(lldb) expr
Enter expressions, then terminate with an empty line to evaluate:

  1: first C expression
  2: Second C Expression
  3: 

The second only differs from the first in that it pushes an IOHandler to deal 
with gathering the entered text that the user typed up to the point where they 
type an empty expression.

The repl adds a third mode:

(lldb) expr -r --

  1> let a = 10

a: Int = 10

  2> print(a)

10

  3>  

Which works the same as "expr --toplevel" with the addition of a fancier 
IOHandler that looks for complete expressions and evaluates each completed 
expression as it sees it.  I'm glossing over a few details here (like the fact 
that ":" will switch you to the lldb command interpreter), but for the most 
part that's all it is.

Note, there's another "repl" that lldb provides:

> lldb --repl

Which packages up "make a target out of a  little victim program we've stashed 
away in LLDB.framework, set a breakpoint in it, run to the breakpoint and then 
execute "expr -r --".  So there's also a little bit to support that.  But I 
don't think that materially changes how you should understand the REPL.  And 
then most people actually access this for swift by invoking "swift" with no 
arguments, but the swift driver with no arguments just turns around and exec's 
"lldb --repl" so that doesn't add much besides convenience.

Jim


https://reviews.llvm.org/D47232



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 148241.
aprantl added a comment.

Updated patch according to Zachary's suggestion.


https://reviews.llvm.org/D47235

Files:
  include/lldb/Core/ModuleList.h
  include/lldb/Host/HostInfoBase.h
  source/API/SystemInitializerFull.cpp
  source/Core/ModuleList.cpp
  source/Host/CMakeLists.txt
  tools/lldb-test/CMakeLists.txt
  tools/lldb-test/lldb-test.cpp

Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -34,6 +34,9 @@
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
+
+#include "clang/Driver/Driver.h"
+
 #include 
 
 using namespace lldb;
@@ -467,6 +470,11 @@
 
   cl::ParseCommandLineOptions(argc, argv, "LLDB Testing Utility\n");
 
+  // Retrieve the default modulecache path from clang.
+  llvm::SmallString<128> path;
+  clang::driver::Driver::getDefaultModuleCachePath(path);
+  ModuleListProperties::Initialize(path);
+
   SystemLifetimeManager DebuggerLifetime;
   DebuggerLifetime.Initialize(llvm::make_unique(),
   nullptr);
Index: tools/lldb-test/CMakeLists.txt
===
--- tools/lldb-test/CMakeLists.txt
+++ tools/lldb-test/CMakeLists.txt
@@ -19,6 +19,7 @@
 lldbUtility
 ${LLDB_ALL_PLUGINS}
 ${host_lib}
+clangDriver
 
   LINK_COMPONENTS
 Support
Index: source/Host/CMakeLists.txt
===
--- source/Host/CMakeLists.txt
+++ source/Host/CMakeLists.txt
@@ -185,7 +185,7 @@
 lldbUtility
 ${LLDB_PLUGINS}
 ${EXTRA_LIBS}
-  
+
   LINK_COMPONENTS
 Object
 Support
Index: source/Core/ModuleList.cpp
===
--- source/Core/ModuleList.cpp
+++ source/Core/ModuleList.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Symbols.h"
+#include "lldb/Host/HostInfoBase.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/OptionValueFileSpec.h"
 #include "lldb/Interpreter/Property.h"
@@ -34,7 +35,6 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h" // for fs
-#include "clang/Driver/Driver.h"
 
 #include  // for operator!=, time_point
 #include  // for shared_ptr
@@ -79,15 +79,20 @@
 
 enum { ePropertyEnableExternalLookup, ePropertyClangModulesCachePath };
 
+std::string g_default_clang_modules_cache_path;
 } // namespace
 
+void ModuleListProperties::Initialize(
+llvm::StringRef clang_modules_cache_path) {
+  g_default_clang_modules_cache_path = clang_modules_cache_path.str();
+}
+
 ModuleListProperties::ModuleListProperties() {
   m_collection_sp.reset(new OptionValueProperties(ConstString("symbols")));
   m_collection_sp->Initialize(g_properties);
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);
 }
 
 bool ModuleListProperties::GetEnableExternalLookup() const {
Index: source/API/SystemInitializerFull.cpp
===
--- source/API/SystemInitializerFull.cpp
+++ source/API/SystemInitializerFull.cpp
@@ -119,6 +119,7 @@
 #endif
 
 #include "llvm/Support/TargetSelect.h"
+#include "clang/Driver/Driver.h"
 
 #include 
 
@@ -385,6 +386,11 @@
   // AFTER PluginManager::Initialize is called.
 
   Debugger::SettingsInitialize();
+
+  // Retrieve the default modulecache path from clang.
+  llvm::SmallString<128> path;
+  clang::driver::Driver::getDefaultModuleCachePath(path);
+  ModuleListProperties::Initialize(path);
 }
 
 void SystemInitializerFull::InitializeSWIG() {
Index: include/lldb/Host/HostInfoBase.h
===
--- include/lldb/Host/HostInfoBase.h
+++ include/lldb/Host/HostInfoBase.h
@@ -89,7 +89,7 @@
   /// ArchSpec object.
   //---
   static ArchSpec GetAugmentedArchSpec(llvm::StringRef triple);
-
+  
 protected:
   static bool ComputeSharedLibraryDirectory(FileSpec &file_spec);
   static bool ComputeSupportExeDirectory(FileSpec &file_spec);
Index: include/lldb/Core/ModuleList.h
===
--- include/lldb/Core/ModuleList.h
+++ include/lldb/Core/ModuleList.h
@@ -79,6 +79,10 @@
 public:
   ModuleListProperties();
 
+  /// Set the default clang modules cache path.
+  /// This avoids Core depending on Clang.
+  static void Initialize(llvm::StringRef clang_modules_cache_path);
+
   FileSpec GetClangModulesCachePath() const;
   bool SetClangModulesCachePath(llvm::String

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D46005#1109761, @JDevlieghere wrote:

> In https://reviews.llvm.org/D46005#1109693, @zturner wrote:
>
> > FWIW, I think the single biggest improvement one could make to the LLDB 
> > test suite runtime is to compile all inferiors up front as part of the 
> > CMake step.  If you run the test suite twice every inferior is 
> > unnecessarily compiled twice.
>
>
> I'm a big proponent of moving as much logic as possible into the 
> configuration stage, but a common use case (at least for us) is testing a 
> single build with a different compiler/configuration.


Then the configure / CMake stage could build all inferiors with every possible 
configuration you want to test, each one writing to a different output 
directory.  Sorta like how in LLVM you can specify 
`LLVM_TARGETS_TO_BUILD=X86,ARM,...` you could specify something like 
`LLDB_TEST_INFERIOR_CONFIGS=gcc-x64;gcc-x86;clang-arm64`.

Obviously this is a big change and a lot of work, but I think it would make the 
test suite run in under 30 seconds


https://reviews.llvm.org/D46005



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 148248.
aprantl added a comment.

Forgot to update unit tests.


https://reviews.llvm.org/D47235

Files:
  include/lldb/API/SystemInitializerFull.h
  include/lldb/Core/ModuleList.h
  include/lldb/Host/HostInfoBase.h
  source/API/SystemInitializerFull.cpp
  source/Core/ModuleList.cpp
  tools/lldb-test/CMakeLists.txt
  tools/lldb-test/lldb-test.cpp
  unittests/Host/SymbolsTest.cpp
  unittests/Target/ModuleCacheTest.cpp

Index: unittests/Target/ModuleCacheTest.cpp
===
--- unittests/Target/ModuleCacheTest.cpp
+++ unittests/Target/ModuleCacheTest.cpp
@@ -5,7 +5,9 @@
 #include "llvm/Support/Path.h"
 
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "lldb/API/SystemInitializerFull.h"
 #include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/SymbolContext.h"
@@ -67,6 +69,7 @@
 void ModuleCacheTest::SetUpTestCase() {
   HostInfo::Initialize();
   ObjectFileELF::Initialize();
+  ModuleListProperties::Initialize("dummy");
 
   FileSpec tmpdir_spec;
   HostInfo::GetLLDBPath(lldb::ePathTypeLLDBTempSystemDir, s_cache_dir);
Index: unittests/Host/SymbolsTest.cpp
===
--- unittests/Host/SymbolsTest.cpp
+++ unittests/Host/SymbolsTest.cpp
@@ -9,20 +9,24 @@
 
 #include "gtest/gtest.h"
 
+#include "lldb/API/SystemInitializerFull.h"
 #include "lldb/Host/Symbols.h"
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 
 using namespace lldb_private;
 
 TEST(SymbolsTest,
  LocateExecutableSymbolFileForUnknownExecutableAndUnknownSymbolFile) {
+  ModuleListProperties::Initialize("dummy");
   ModuleSpec module_spec;
   FileSpec symbol_file_spec = Symbols::LocateExecutableSymbolFile(module_spec);
   EXPECT_TRUE(symbol_file_spec.GetFilename().IsEmpty());
 }
 
 TEST(SymbolsTest,
  LocateExecutableSymbolFileForUnknownExecutableAndMissingSymbolFile) {
+  ModuleListProperties::Initialize("dummy");
   ModuleSpec module_spec;
   // using a GUID here because the symbol file shouldn't actually exist on disk
   module_spec.GetSymbolFileSpec().SetFile(
Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -34,6 +34,9 @@
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
+
+#include "clang/Driver/Driver.h"
+
 #include 
 
 using namespace lldb;
@@ -467,6 +470,11 @@
 
   cl::ParseCommandLineOptions(argc, argv, "LLDB Testing Utility\n");
 
+  // Retrieve the default modulecache path from clang.
+  llvm::SmallString<128> path;
+  clang::driver::Driver::getDefaultModuleCachePath(path);
+  ModuleListProperties::Initialize(path);
+
   SystemLifetimeManager DebuggerLifetime;
   DebuggerLifetime.Initialize(llvm::make_unique(),
   nullptr);
Index: tools/lldb-test/CMakeLists.txt
===
--- tools/lldb-test/CMakeLists.txt
+++ tools/lldb-test/CMakeLists.txt
@@ -19,6 +19,7 @@
 lldbUtility
 ${LLDB_ALL_PLUGINS}
 ${host_lib}
+clangDriver
 
   LINK_COMPONENTS
 Support
Index: source/Core/ModuleList.cpp
===
--- source/Core/ModuleList.cpp
+++ source/Core/ModuleList.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Symbols.h"
+#include "lldb/Host/HostInfoBase.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/OptionValueFileSpec.h"
 #include "lldb/Interpreter/Property.h"
@@ -34,7 +35,6 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h" // for fs
-#include "clang/Driver/Driver.h"
 
 #include  // for operator!=, time_point
 #include  // for shared_ptr
@@ -79,15 +79,20 @@
 
 enum { ePropertyEnableExternalLookup, ePropertyClangModulesCachePath };
 
+std::string g_default_clang_modules_cache_path;
 } // namespace
 
+void ModuleListProperties::Initialize(
+llvm::StringRef clang_modules_cache_path) {
+  g_default_clang_modules_cache_path = clang_modules_cache_path.str();
+}
+
 ModuleListProperties::ModuleListProperties() {
   m_collection_sp.reset(new OptionValueProperties(ConstString("symbols")));
   m_collection_sp->Initialize(g_properties);
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);
 }
 
 bool ModuleListProperties::GetEnableExternalLookup() const {
Index: source/API/SystemInitializerFull.cpp
===

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

Can you revert the changes to `HostInfoBase.h`?  It looks like now it's only 
whitespace changes.




Comment at: source/Core/ModuleList.cpp:16
 #include "lldb/Host/Symbols.h"
+#include "lldb/Host/HostInfoBase.h"
 #include "lldb/Interpreter/OptionValueProperties.h"

I think we don't need this include anymore.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

I don't think this should be an assert.  After all, if the whole point is to 
make LLDB usable in situations where clang is not present, then someone using 
it in such an environment would probably never call the static function to 
begin with.  So I think we should just remove the assert and set it to whatever 
the value happens to be (including empty)


https://reviews.llvm.org/D47235



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

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In https://reviews.llvm.org/D46005#1109817, @zturner wrote:

> In https://reviews.llvm.org/D46005#1109761, @JDevlieghere wrote:
>
> > In https://reviews.llvm.org/D46005#1109693, @zturner wrote:
> >
> > > FWIW, I think the single biggest improvement one could make to the LLDB 
> > > test suite runtime is to compile all inferiors up front as part of the 
> > > CMake step.  If you run the test suite twice every inferior is 
> > > unnecessarily compiled twice.
> >
> >
> > I'm a big proponent of moving as much logic as possible into the 
> > configuration stage, but a common use case (at least for us) is testing a 
> > single build with a different compiler/configuration.
>
>
> Then the configure / CMake stage could build all inferiors with every 
> possible configuration you want to test, each one writing to a different 
> output directory.  Sorta like how in LLVM you can specify 
> `LLVM_TARGETS_TO_BUILD=X86,ARM,...` you could specify something like 
> `LLDB_TEST_INFERIOR_CONFIGS=gcc-x64;gcc-x86;clang-arm64`.
>
> Obviously this is a big change and a lot of work, but I think it would make 
> the test suite run in under 30 seconds


I don't think that moving this to the configure stage is the right choice. I'm 
also skeptical about your claim about the saved time (are you talking about 
Windows?).

In my opinion the configuration should configure what is being built, not what 
is being tested. I don't want to have to lock down at configure time which 
tests I will be running. As an analogy, this is similar to how LLVM has the 
in-tree regression tests but also the external test-suite that can be run with 
endless permutations of options, to test one specific build of LLVM, without 
having to reconfigure/rebuild.


https://reviews.llvm.org/D46005



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

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D46005#1109911, @aprantl wrote:

> In https://reviews.llvm.org/D46005#1109817, @zturner wrote:
>
> > In https://reviews.llvm.org/D46005#1109761, @JDevlieghere wrote:
> >
> > > In https://reviews.llvm.org/D46005#1109693, @zturner wrote:
> > >
> > > > FWIW, I think the single biggest improvement one could make to the LLDB 
> > > > test suite runtime is to compile all inferiors up front as part of the 
> > > > CMake step.  If you run the test suite twice every inferior is 
> > > > unnecessarily compiled twice.
> > >
> > >
> > > I'm a big proponent of moving as much logic as possible into the 
> > > configuration stage, but a common use case (at least for us) is testing a 
> > > single build with a different compiler/configuration.
> >
> >
> > Then the configure / CMake stage could build all inferiors with every 
> > possible configuration you want to test, each one writing to a different 
> > output directory.  Sorta like how in LLVM you can specify 
> > `LLVM_TARGETS_TO_BUILD=X86,ARM,...` you could specify something like 
> > `LLDB_TEST_INFERIOR_CONFIGS=gcc-x64;gcc-x86;clang-arm64`.
> >
> > Obviously this is a big change and a lot of work, but I think it would make 
> > the test suite run in under 30 seconds
>
>
> I don't think that moving this to the configure stage is the right choice. 
> I'm also skeptical about your claim about the saved time (are you talking 
> about Windows?).
>
> In my opinion the configuration should configure what is being built, not 
> what is being tested. I don't want to have to lock down at configure time 
> which tests I will be running. As an analogy, this is similar to how LLVM has 
> the in-tree regression tests but also the external test-suite that can be run 
> with endless permutations of options, to test one specific build of LLVM, 
> without having to reconfigure/rebuild.


This is a bit of a digression from this patch, but no I wasn't talking about 
specific to Windows.  The amount of time required to compile several thousand 
inferiors is quite significant, especially since it is completely redundant and 
shouldn't be changing between each run of the test suite.  So it's unnecessary.

Also just because it could be done during the CMake configure step doesn't mean 
it couldn't *also* be done via some external script.  Similar to how if you run 
`ninja check-lldb` it runs with the default set of options, but you can still 
run `dotest.py` manually to pass additional flags.  So building inferiors as 
part of the configure step does not necessitate locking down at configure time 
which tests you will be running.


https://reviews.llvm.org/D46005



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 148264.
aprantl added a comment.

Remove whitespace changes.


https://reviews.llvm.org/D47235

Files:
  include/lldb/API/SystemInitializerFull.h
  include/lldb/Core/ModuleList.h
  source/API/SystemInitializerFull.cpp
  source/Core/ModuleList.cpp
  tools/lldb-test/CMakeLists.txt
  tools/lldb-test/lldb-test.cpp
  unittests/Host/SymbolsTest.cpp
  unittests/Target/ModuleCacheTest.cpp

Index: unittests/Target/ModuleCacheTest.cpp
===
--- unittests/Target/ModuleCacheTest.cpp
+++ unittests/Target/ModuleCacheTest.cpp
@@ -5,7 +5,9 @@
 #include "llvm/Support/Path.h"
 
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "lldb/API/SystemInitializerFull.h"
 #include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/SymbolContext.h"
@@ -67,6 +69,7 @@
 void ModuleCacheTest::SetUpTestCase() {
   HostInfo::Initialize();
   ObjectFileELF::Initialize();
+  ModuleListProperties::Initialize("dummy");
 
   FileSpec tmpdir_spec;
   HostInfo::GetLLDBPath(lldb::ePathTypeLLDBTempSystemDir, s_cache_dir);
Index: unittests/Host/SymbolsTest.cpp
===
--- unittests/Host/SymbolsTest.cpp
+++ unittests/Host/SymbolsTest.cpp
@@ -9,20 +9,24 @@
 
 #include "gtest/gtest.h"
 
+#include "lldb/API/SystemInitializerFull.h"
 #include "lldb/Host/Symbols.h"
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 
 using namespace lldb_private;
 
 TEST(SymbolsTest,
  LocateExecutableSymbolFileForUnknownExecutableAndUnknownSymbolFile) {
+  ModuleListProperties::Initialize("dummy");
   ModuleSpec module_spec;
   FileSpec symbol_file_spec = Symbols::LocateExecutableSymbolFile(module_spec);
   EXPECT_TRUE(symbol_file_spec.GetFilename().IsEmpty());
 }
 
 TEST(SymbolsTest,
  LocateExecutableSymbolFileForUnknownExecutableAndMissingSymbolFile) {
+  ModuleListProperties::Initialize("dummy");
   ModuleSpec module_spec;
   // using a GUID here because the symbol file shouldn't actually exist on disk
   module_spec.GetSymbolFileSpec().SetFile(
Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -34,6 +34,9 @@
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
+
+#include "clang/Driver/Driver.h"
+
 #include 
 
 using namespace lldb;
@@ -467,6 +470,11 @@
 
   cl::ParseCommandLineOptions(argc, argv, "LLDB Testing Utility\n");
 
+  // Retrieve the default modulecache path from clang.
+  llvm::SmallString<128> path;
+  clang::driver::Driver::getDefaultModuleCachePath(path);
+  ModuleListProperties::Initialize(path);
+
   SystemLifetimeManager DebuggerLifetime;
   DebuggerLifetime.Initialize(llvm::make_unique(),
   nullptr);
Index: tools/lldb-test/CMakeLists.txt
===
--- tools/lldb-test/CMakeLists.txt
+++ tools/lldb-test/CMakeLists.txt
@@ -19,6 +19,7 @@
 lldbUtility
 ${LLDB_ALL_PLUGINS}
 ${host_lib}
+clangDriver
 
   LINK_COMPONENTS
 Support
Index: source/Core/ModuleList.cpp
===
--- source/Core/ModuleList.cpp
+++ source/Core/ModuleList.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Symbols.h"
+#include "lldb/Host/HostInfoBase.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/OptionValueFileSpec.h"
 #include "lldb/Interpreter/Property.h"
@@ -34,7 +35,6 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h" // for fs
-#include "clang/Driver/Driver.h"
 
 #include  // for operator!=, time_point
 #include  // for shared_ptr
@@ -79,15 +79,20 @@
 
 enum { ePropertyEnableExternalLookup, ePropertyClangModulesCachePath };
 
+std::string g_default_clang_modules_cache_path;
 } // namespace
 
+void ModuleListProperties::Initialize(
+llvm::StringRef clang_modules_cache_path) {
+  g_default_clang_modules_cache_path = clang_modules_cache_path.str();
+}
+
 ModuleListProperties::ModuleListProperties() {
   m_collection_sp.reset(new OptionValueProperties(ConstString("symbols")));
   m_collection_sp->Initialize(g_properties);
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);
 }
 
 bool ModuleListProperties::GetEnableExternalLookup() const {
Index: source/API/SystemInitializerFull.cpp
===
--- source/API/SystemInitializerFull.

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 148265.
aprantl added a comment.

Remove unused include.


https://reviews.llvm.org/D47235

Files:
  include/lldb/API/SystemInitializerFull.h
  include/lldb/Core/ModuleList.h
  source/API/SystemInitializerFull.cpp
  source/Core/ModuleList.cpp
  tools/lldb-test/CMakeLists.txt
  tools/lldb-test/lldb-test.cpp
  unittests/Host/SymbolsTest.cpp
  unittests/Target/ModuleCacheTest.cpp

Index: unittests/Target/ModuleCacheTest.cpp
===
--- unittests/Target/ModuleCacheTest.cpp
+++ unittests/Target/ModuleCacheTest.cpp
@@ -5,7 +5,9 @@
 #include "llvm/Support/Path.h"
 
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "lldb/API/SystemInitializerFull.h"
 #include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/SymbolContext.h"
@@ -67,6 +69,7 @@
 void ModuleCacheTest::SetUpTestCase() {
   HostInfo::Initialize();
   ObjectFileELF::Initialize();
+  ModuleListProperties::Initialize("dummy");
 
   FileSpec tmpdir_spec;
   HostInfo::GetLLDBPath(lldb::ePathTypeLLDBTempSystemDir, s_cache_dir);
Index: unittests/Host/SymbolsTest.cpp
===
--- unittests/Host/SymbolsTest.cpp
+++ unittests/Host/SymbolsTest.cpp
@@ -9,20 +9,24 @@
 
 #include "gtest/gtest.h"
 
+#include "lldb/API/SystemInitializerFull.h"
 #include "lldb/Host/Symbols.h"
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 
 using namespace lldb_private;
 
 TEST(SymbolsTest,
  LocateExecutableSymbolFileForUnknownExecutableAndUnknownSymbolFile) {
+  ModuleListProperties::Initialize("dummy");
   ModuleSpec module_spec;
   FileSpec symbol_file_spec = Symbols::LocateExecutableSymbolFile(module_spec);
   EXPECT_TRUE(symbol_file_spec.GetFilename().IsEmpty());
 }
 
 TEST(SymbolsTest,
  LocateExecutableSymbolFileForUnknownExecutableAndMissingSymbolFile) {
+  ModuleListProperties::Initialize("dummy");
   ModuleSpec module_spec;
   // using a GUID here because the symbol file shouldn't actually exist on disk
   module_spec.GetSymbolFileSpec().SetFile(
Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -34,6 +34,9 @@
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
+
+#include "clang/Driver/Driver.h"
+
 #include 
 
 using namespace lldb;
@@ -467,6 +470,11 @@
 
   cl::ParseCommandLineOptions(argc, argv, "LLDB Testing Utility\n");
 
+  // Retrieve the default modulecache path from clang.
+  llvm::SmallString<128> path;
+  clang::driver::Driver::getDefaultModuleCachePath(path);
+  ModuleListProperties::Initialize(path);
+
   SystemLifetimeManager DebuggerLifetime;
   DebuggerLifetime.Initialize(llvm::make_unique(),
   nullptr);
Index: tools/lldb-test/CMakeLists.txt
===
--- tools/lldb-test/CMakeLists.txt
+++ tools/lldb-test/CMakeLists.txt
@@ -19,6 +19,7 @@
 lldbUtility
 ${LLDB_ALL_PLUGINS}
 ${host_lib}
+clangDriver
 
   LINK_COMPONENTS
 Support
Index: source/Core/ModuleList.cpp
===
--- source/Core/ModuleList.cpp
+++ source/Core/ModuleList.cpp
@@ -34,7 +34,6 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h" // for fs
-#include "clang/Driver/Driver.h"
 
 #include  // for operator!=, time_point
 #include  // for shared_ptr
@@ -79,15 +78,20 @@
 
 enum { ePropertyEnableExternalLookup, ePropertyClangModulesCachePath };
 
+std::string g_default_clang_modules_cache_path;
 } // namespace
 
+void ModuleListProperties::Initialize(
+llvm::StringRef clang_modules_cache_path) {
+  g_default_clang_modules_cache_path = clang_modules_cache_path.str();
+}
+
 ModuleListProperties::ModuleListProperties() {
   m_collection_sp.reset(new OptionValueProperties(ConstString("symbols")));
   m_collection_sp->Initialize(g_properties);
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);
 }
 
 bool ModuleListProperties::GetEnableExternalLookup() const {
Index: source/API/SystemInitializerFull.cpp
===
--- source/API/SystemInitializerFull.cpp
+++ source/API/SystemInitializerFull.cpp
@@ -119,6 +119,7 @@
 #endif
 
 #include "llvm/Support/TargetSelect.h"
+#include "clang/Driver/Driver.h"
 
 #include 
 
@@ -385,6 +386,13 @@
   // AFTER PluginManager::Initialize is called.
 
   Debugger::SettingsInitialize();
+  InitializeClang();
+}
+
+void S

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

zturner wrote:
> I don't think this should be an assert.  After all, if the whole point is to 
> make LLDB usable in situations where clang is not present, then someone using 
> it in such an environment would probably never call the static function to 
> begin with.  So I think we should just remove the assert and set it to 
> whatever the value happens to be (including empty)
The assertion enforces that ModuleListProperties::Initialize() has been called. 
If we want to make it more convenient, we can add a default argument `= 
"dummy"` for clients that don't link against clang.


https://reviews.llvm.org/D47235



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

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In https://reviews.llvm.org/D46005#1109920, @zturner wrote:

> In https://reviews.llvm.org/D46005#1109911, @aprantl wrote:
>
> > In https://reviews.llvm.org/D46005#1109817, @zturner wrote:
> >
> > > In https://reviews.llvm.org/D46005#1109761, @JDevlieghere wrote:
> > >
> > > > In https://reviews.llvm.org/D46005#1109693, @zturner wrote:
> > > >
> > > > > FWIW, I think the single biggest improvement one could make to the 
> > > > > LLDB test suite runtime is to compile all inferiors up front as part 
> > > > > of the CMake step.  If you run the test suite twice every inferior is 
> > > > > unnecessarily compiled twice.
> > > >
> > > >
> > > > I'm a big proponent of moving as much logic as possible into the 
> > > > configuration stage, but a common use case (at least for us) is testing 
> > > > a single build with a different compiler/configuration.
> > >
> > >
> > > Then the configure / CMake stage could build all inferiors with every 
> > > possible configuration you want to test, each one writing to a different 
> > > output directory.  Sorta like how in LLVM you can specify 
> > > `LLVM_TARGETS_TO_BUILD=X86,ARM,...` you could specify something like 
> > > `LLDB_TEST_INFERIOR_CONFIGS=gcc-x64;gcc-x86;clang-arm64`.
> > >
> > > Obviously this is a big change and a lot of work, but I think it would 
> > > make the test suite run in under 30 seconds
> >
> >
> > I don't think that moving this to the configure stage is the right choice. 
> > I'm also skeptical about your claim about the saved time (are you talking 
> > about Windows?).
> >
> > In my opinion the configuration should configure what is being built, not 
> > what is being tested. I don't want to have to lock down at configure time 
> > which tests I will be running. As an analogy, this is similar to how LLVM 
> > has the in-tree regression tests but also the external test-suite that can 
> > be run with endless permutations of options, to test one specific build of 
> > LLVM, without having to reconfigure/rebuild.
>
>
> This is a bit of a digression from this patch,


Agreed :-)

but no I wasn't talking about specific to Windows.  The amount of time required 
to compile several thousand inferiors is quite significant, especially since it 
is completely redundant and shouldn't be changing between each run of the test 
suite.  So it's unnecessary.

> 

I suppose you are looking at this from the perspective of an LLDB developer, 
where rebuilding the tests after changing something in LLDB seems redundant. I 
was thinking about this from the perspective of our bots, and there LLVM 
changes much much faster than LLDB, so between two bot runs the only thing that 
changes is the compiler, which forces you to rebuild all the source-based tests 
anyway.

... but let's postpone this discussion for another day. As you said it's a 
diversion for this review.

> Also just because it could be done during the CMake configure step doesn't 
> mean it couldn't *also* be done via some external script.  Similar to how if 
> you run `ninja check-lldb` it runs with the default set of options, but you 
> can still run `dotest.py` manually to pass additional flags.  So building 
> inferiors as part of the configure step does not necessitate locking down at 
> configure time which tests you will be running.




https://reviews.llvm.org/D46005



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

aprantl wrote:
> zturner wrote:
> > I don't think this should be an assert.  After all, if the whole point is 
> > to make LLDB usable in situations where clang is not present, then someone 
> > using it in such an environment would probably never call the static 
> > function to begin with.  So I think we should just remove the assert and 
> > set it to whatever the value happens to be (including empty)
> The assertion enforces that ModuleListProperties::Initialize() has been 
> called. If we want to make it more convenient, we can add a default argument 
> `= "dummy"` for clients that don't link against clang.
I was actually thinking that instead of calling it `Initialize` (which sounds 
generic and like it's required) we would just call it 
`SetDefaultClangModulesCachePath` and have the user directly call that.  With a 
name like `Initialize`, it makes the user think that it's required, but in fact 
the only thing it's initializing is something that is optional, so it shouldn't 
be required.

It's possible I'm misunderstanding something though.


https://reviews.llvm.org/D47235



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

[Lldb-commits] [PATCH] D47275: 1/3: DWARFDIE split out to DWARFBasicDIE

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added reviewers: labath, clayborg.
Herald added subscribers: JDevlieghere, aprantl, mgorny.

As Pavel Labath said in https://reviews.llvm.org/D46810 this is new 
`DWARFBasicDIE` to be used for `DWARFUnit::GetUnitDIEOnly()`. This patch is 
only mechanical split without any use of it.


https://reviews.llvm.org/D47275

Files:
  source/Plugins/SymbolFile/DWARF/CMakeLists.txt
  source/Plugins/SymbolFile/DWARF/DWARFBasicDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFBasicDIE.h
  source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.h

Index: source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -10,125 +10,23 @@
 #ifndef SymbolFileDWARF_DWARFDIE_h_
 #define SymbolFileDWARF_DWARFDIE_h_
 
-#include "lldb/Core/dwarf.h"
-#include "lldb/lldb-types.h"
-
-struct DIERef;
-class DWARFASTParser;
-class DWARFAttributes;
-class DWARFUnit;
-class DWARFDebugInfoEntry;
-class DWARFDeclContext;
-class DWARFDIECollection;
-class SymbolFileDWARF;
-
-class DWARFDIE {
-public:
-  DWARFDIE() : m_cu(nullptr), m_die(nullptr) {}
-
-  DWARFDIE(DWARFUnit *cu, DWARFDebugInfoEntry *die)
-  : m_cu(cu), m_die(die) {}
-
-  DWARFDIE(const DWARFUnit *cu, DWARFDebugInfoEntry *die)
-  : m_cu(const_cast(cu)), m_die(die) {}
-
-  DWARFDIE(DWARFUnit *cu, const DWARFDebugInfoEntry *die)
-  : m_cu(cu), m_die(const_cast(die)) {}
-
-  DWARFDIE(const DWARFUnit *cu, const DWARFDebugInfoEntry *die)
-  : m_cu(const_cast(cu)),
-m_die(const_cast(die)) {}
-
-  //--
-  // Tests
-  //--
-  explicit operator bool() const { return IsValid(); }
-
-  bool IsValid() const { return m_cu && m_die; }
+#include "DWARFBasicDIE.h"
 
-  bool IsStructOrClass() const;
-
-  bool HasChildren() const;
-
-  bool Supports_DW_AT_APPLE_objc_complete_type() const;
+class DWARFDIE : public DWARFBasicDIE {
+public:
+  using DWARFBasicDIE::DWARFBasicDIE;
 
   //--
   // Accessors
   //--
-  SymbolFileDWARF *GetDWARF() const;
-
-  DWARFUnit *GetCU() const { return m_cu; }
-
-  DWARFDebugInfoEntry *GetDIE() const { return m_die; }
-
-  DIERef GetDIERef() const;
-
-  lldb_private::TypeSystem *GetTypeSystem() const;
-
-  DWARFASTParser *GetDWARFParser() const;
-
-  void Set(DWARFUnit *cu, DWARFDebugInfoEntry *die) {
-if (cu && die) {
-  m_cu = cu;
-  m_die = die;
-} else {
-  Clear();
-}
-  }
-
-  void Clear() {
-m_cu = nullptr;
-m_die = nullptr;
-  }
-
   lldb::ModuleSP GetContainingDWOModule() const;
 
   DWARFDIE
   GetContainingDWOModuleDIE() const;
 
-  //--
-  // Get the data that contains the attribute values for this DIE. Support
-  // for .debug_types means that any DIE can have its data either in the
-  // .debug_info or the .debug_types section; this method will return the
-  // correct section data.
-  //
-  // Clients must validate that this object is valid before calling this.
-  //--
-  const lldb_private::DWARFDataExtractor &GetData() const;
-
   //--
   // Accessing information about a DIE
   //--
-  dw_tag_t Tag() const;
-
-  const char *GetTagAsCString() const;
-
-  dw_offset_t GetOffset() const;
-
-  dw_offset_t GetCompileUnitRelativeOffset() const;
-
-  //--
-  // Get the LLDB user ID for this DIE. This is often just the DIE offset,
-  // but it might have a SymbolFileDWARF::GetID() in the high 32 bits if
-  // we are doing Darwin DWARF in .o file, or DWARF stand alone debug
-  // info.
-  //--
-  lldb::user_id_t GetID() const;
-
-  const char *GetName() const;
-
-  const char *GetMangledName() const;
-
-  const char *GetPubname() const;
-
-  const char *GetQualifiedName(std::string &storage) const;
-
-  lldb::LanguageType GetLanguage() const;
-
-  lldb::ModuleSP GetModule() const;
-
-  lldb_private::CompileUnit *GetLLDBCompileUnit() const;
-
   lldb_private::Type *ResolveType() const;
 
   //--
@@ -159,6 +57,7 @@
   //--
   DWARFDIE
   GetDIE(dw_offset_t die_offset) const;
+  using DWARFBasicDIE::GetDIE;
 
   DWARFDIE
   LookupDeepestBlock(lldb::addr_t file_addr) const;
@@ -182,5

[Lldb-commits] [PATCH] D47276: 2/3: Use DWARFBasicDIE as compile-time protection

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added reviewers: labath, clayborg.
Herald added subscribers: JDevlieghere, aprantl.

As suggested by Pavel Labath in https://reviews.llvm.org/D46810 
`DWARFUnit::GetUnitDIEOnly()` returning a pointer to `m_first_die` should not 
permit using methods like `GetFirstChild()`.


https://reviews.llvm.org/D47276

Files:
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -748,7 +748,7 @@
   } else {
 ModuleSP module_sp(m_obj_file->GetModule());
 if (module_sp) {
-  const DWARFDIE cu_die = dwarf_cu->GetUnitDIEOnly();
+  const DWARFDIE cu_die = dwarf_cu->DIE();
   if (cu_die) {
 FileSpec cu_file_spec{cu_die.GetName(), false};
 if (cu_file_spec) {
@@ -883,7 +883,7 @@
   assert(sc.comp_unit);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu) {
-const DWARFDIE cu_die = dwarf_cu->GetUnitDIEOnly();
+const DWARFBasicDIE cu_die = dwarf_cu->GetUnitDIEOnly();
 
 if (cu_die) {
   FileSpec cu_comp_dir = resolveCompDir(
@@ -922,7 +922,7 @@
   UpdateExternalModuleListIfNeeded();
 
   if (sc.comp_unit) {
-const DWARFDIE die = dwarf_cu->GetUnitDIEOnly();
+const DWARFDIE die = dwarf_cu->DIE();
 
 if (die) {
   for (DWARFDIE child_die = die.GetFirstChild(); child_die;
@@ -997,7 +997,7 @@
 
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu) {
-const DWARFDIE dwarf_cu_die = dwarf_cu->GetUnitDIEOnly();
+const DWARFBasicDIE dwarf_cu_die = dwarf_cu->GetUnitDIEOnly();
 if (dwarf_cu_die) {
   const dw_offset_t cu_line_offset =
   dwarf_cu_die.GetAttributeValueAsUnsigned(DW_AT_stmt_list,
@@ -1082,7 +1082,7 @@
   if (dwarf_cu == nullptr)
 return false;
 
-  const DWARFDIE dwarf_cu_die = dwarf_cu->GetUnitDIEOnly();
+  const DWARFBasicDIE dwarf_cu_die = dwarf_cu->GetUnitDIEOnly();
   if (!dwarf_cu_die)
 return false;
 
@@ -1578,7 +1578,7 @@
   for (uint32_t cu_idx = 0; cu_idx < num_compile_units; ++cu_idx) {
 DWARFUnit *dwarf_cu = debug_info->GetCompileUnitAtIndex(cu_idx);
 
-const DWARFDIE die = dwarf_cu->GetUnitDIEOnly();
+const DWARFBasicDIE die = dwarf_cu->GetUnitDIEOnly();
 if (die && die.HasChildren() == false) {
   const char *name = die.GetAttributeValueAsString(DW_AT_name, nullptr);
 
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -110,7 +110,7 @@
 
   void SetBaseAddress(dw_addr_t base_addr);
 
-  DWARFDIE GetUnitDIEOnly() { return DWARFDIE(this, GetUnitDIEPtrOnly()); }
+  DWARFBasicDIE GetUnitDIEOnly() { return DWARFDIE(this, GetUnitDIEPtrOnly()); }
 
   DWARFDIE DIE() { return DWARFDIE(this, DIEPtr()); }
 
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -209,7 +209,7 @@
   if (!dwo_cu)
 return; // Can't fetch the compile unit from the dwo file.
 
-  DWARFDIE dwo_cu_die = dwo_cu->GetUnitDIEOnly();
+  DWARFBasicDIE dwo_cu_die = dwo_cu->GetUnitDIEOnly();
   if (!dwo_cu_die.IsValid())
 return; // Can't fetch the compile unit DIE from the dwo file.
 
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -638,7 +638,7 @@
 
 void DWARFDebugInfoEntry::DumpLocation(SymbolFileDWARF *dwarf2Data,
DWARFUnit *cu, Stream &s) const {
-  const DWARFDIE cu_die = cu->GetUnitDIEOnly();
+  const DWARFBasicDIE cu_die = cu->GetUnitDIEOnly();
   const char *cu_name = NULL;
   if (cu_die)
 cu_name = cu_die.GetName();
@@ -912,7 +912,7 @@
   if (!dwo_cu)
 return 0;
 
-  DWARFDIE dwo_cu_die = dwo_cu->GetUnitDIEOnly();
+  DWARFBasicDIE dwo_cu_die = dwo_cu->GetUnitDIEOnly();
   if (!dwo_cu_die.IsValid())
 return 0;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

zturner wrote:
> aprantl wrote:
> > zturner wrote:
> > > I don't think this should be an assert.  After all, if the whole point is 
> > > to make LLDB usable in situations where clang is not present, then 
> > > someone using it in such an environment would probably never call the 
> > > static function to begin with.  So I think we should just remove the 
> > > assert and set it to whatever the value happens to be (including empty)
> > The assertion enforces that ModuleListProperties::Initialize() has been 
> > called. If we want to make it more convenient, we can add a default 
> > argument `= "dummy"` for clients that don't link against clang.
> I was actually thinking that instead of calling it `Initialize` (which sounds 
> generic and like it's required) we would just call it 
> `SetDefaultClangModulesCachePath` and have the user directly call that.  With 
> a name like `Initialize`, it makes the user think that it's required, but in 
> fact the only thing it's initializing is something that is optional, so it 
> shouldn't be required.
> 
> It's possible I'm misunderstanding something though.
My point was that this *is* required (for all clients of lldb that also link 
against clang). When LLDB initializes clang it must set a module cache path 
because clang doesn't implement a fallback.


https://reviews.llvm.org/D47235



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

aprantl wrote:
> zturner wrote:
> > aprantl wrote:
> > > zturner wrote:
> > > > I don't think this should be an assert.  After all, if the whole point 
> > > > is to make LLDB usable in situations where clang is not present, then 
> > > > someone using it in such an environment would probably never call the 
> > > > static function to begin with.  So I think we should just remove the 
> > > > assert and set it to whatever the value happens to be (including empty)
> > > The assertion enforces that ModuleListProperties::Initialize() has been 
> > > called. If we want to make it more convenient, we can add a default 
> > > argument `= "dummy"` for clients that don't link against clang.
> > I was actually thinking that instead of calling it `Initialize` (which 
> > sounds generic and like it's required) we would just call it 
> > `SetDefaultClangModulesCachePath` and have the user directly call that.  
> > With a name like `Initialize`, it makes the user think that it's required, 
> > but in fact the only thing it's initializing is something that is optional, 
> > so it shouldn't be required.
> > 
> > It's possible I'm misunderstanding something though.
> My point was that this *is* required (for all clients of lldb that also link 
> against clang). When LLDB initializes clang it must set a module cache path 
> because clang doesn't implement a fallback.
If there's a client of LLDB using the public API and/or clang then that client 
would also be using `SystemInitializerFull` (or at the very least, would be 
responsible for initializing the set of things they need, one of which would be 
this path).

My point is that `Core` should ultimately have no knowledge that something 
called clang even exists, and it definitely shouldn't be limiting the use of 
itself based on the needs of a specific client since it something that is 
useful to all clients.  If a particular client requires clang, that client 
should initialize clang.

With an assert, this is requiring a non clang-based client to run some 
initialization code that is only required for a clang-based client, which 
doesn't seem like a reasonable restriction (imagine if every downstream 
developer using every possible set of random 3rd party libraries started 
asserting in low-level debugger code that their optional component had been 
initialized).


https://reviews.llvm.org/D47235



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

zturner wrote:
> aprantl wrote:
> > zturner wrote:
> > > aprantl wrote:
> > > > zturner wrote:
> > > > > I don't think this should be an assert.  After all, if the whole 
> > > > > point is to make LLDB usable in situations where clang is not 
> > > > > present, then someone using it in such an environment would probably 
> > > > > never call the static function to begin with.  So I think we should 
> > > > > just remove the assert and set it to whatever the value happens to be 
> > > > > (including empty)
> > > > The assertion enforces that ModuleListProperties::Initialize() has been 
> > > > called. If we want to make it more convenient, we can add a default 
> > > > argument `= "dummy"` for clients that don't link against clang.
> > > I was actually thinking that instead of calling it `Initialize` (which 
> > > sounds generic and like it's required) we would just call it 
> > > `SetDefaultClangModulesCachePath` and have the user directly call that.  
> > > With a name like `Initialize`, it makes the user think that it's 
> > > required, but in fact the only thing it's initializing is something that 
> > > is optional, so it shouldn't be required.
> > > 
> > > It's possible I'm misunderstanding something though.
> > My point was that this *is* required (for all clients of lldb that also 
> > link against clang). When LLDB initializes clang it must set a module cache 
> > path because clang doesn't implement a fallback.
> If there's a client of LLDB using the public API and/or clang then that 
> client would also be using `SystemInitializerFull` (or at the very least, 
> would be responsible for initializing the set of things they need, one of 
> which would be this path).
> 
> My point is that `Core` should ultimately have no knowledge that something 
> called clang even exists, and it definitely shouldn't be limiting the use of 
> itself based on the needs of a specific client since it something that is 
> useful to all clients.  If a particular client requires clang, that client 
> should initialize clang.
> 
> With an assert, this is requiring a non clang-based client to run some 
> initialization code that is only required for a clang-based client, which 
> doesn't seem like a reasonable restriction (imagine if every downstream 
> developer using every possible set of random 3rd party libraries started 
> asserting in low-level debugger code that their optional component had been 
> initialized).
In short, `Core` is too low level to be making any assumptions whatsoever about 
the needs of a particular client.  It may be required for all clients of lldb 
that use clang, but `Core` is not the right place to be making decisions based 
on whether a client of lldb uses clang (or any other optional external library 
/ component).


https://reviews.llvm.org/D47235



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

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: compnerd, sas, labath, beanz, zturner.
Herald added a subscriber: mgorny.

Generating LLDB.framework when building with CMake+Ninja will copy the
lldb-private headers because public_headers contains them, even though we try
to make sure they don't get copied by removing root_private_headers from
root_public_headers.


https://reviews.llvm.org/D47278

Files:
  source/API/CMakeLists.txt


Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -162,8 +162,7 @@
 endif()
 
 if(LLDB_BUILD_FRAMEWORK)
-  file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h
-  ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
+  file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
   file(GLOB root_public_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
   file(GLOB root_private_headers 
${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h)
   list(REMOVE_ITEM root_public_headers ${root_private_headers})


Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -162,8 +162,7 @@
 endif()
 
 if(LLDB_BUILD_FRAMEWORK)
-  file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h
-  ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
+  file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
   file(GLOB root_public_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
   file(GLOB root_private_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h)
   list(REMOVE_ITEM root_public_headers ${root_private_headers})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

zturner wrote:
> zturner wrote:
> > aprantl wrote:
> > > zturner wrote:
> > > > aprantl wrote:
> > > > > zturner wrote:
> > > > > > I don't think this should be an assert.  After all, if the whole 
> > > > > > point is to make LLDB usable in situations where clang is not 
> > > > > > present, then someone using it in such an environment would 
> > > > > > probably never call the static function to begin with.  So I think 
> > > > > > we should just remove the assert and set it to whatever the value 
> > > > > > happens to be (including empty)
> > > > > The assertion enforces that ModuleListProperties::Initialize() has 
> > > > > been called. If we want to make it more convenient, we can add a 
> > > > > default argument `= "dummy"` for clients that don't link against 
> > > > > clang.
> > > > I was actually thinking that instead of calling it `Initialize` (which 
> > > > sounds generic and like it's required) we would just call it 
> > > > `SetDefaultClangModulesCachePath` and have the user directly call that. 
> > > >  With a name like `Initialize`, it makes the user think that it's 
> > > > required, but in fact the only thing it's initializing is something 
> > > > that is optional, so it shouldn't be required.
> > > > 
> > > > It's possible I'm misunderstanding something though.
> > > My point was that this *is* required (for all clients of lldb that also 
> > > link against clang). When LLDB initializes clang it must set a module 
> > > cache path because clang doesn't implement a fallback.
> > If there's a client of LLDB using the public API and/or clang then that 
> > client would also be using `SystemInitializerFull` (or at the very least, 
> > would be responsible for initializing the set of things they need, one of 
> > which would be this path).
> > 
> > My point is that `Core` should ultimately have no knowledge that something 
> > called clang even exists, and it definitely shouldn't be limiting the use 
> > of itself based on the needs of a specific client since it something that 
> > is useful to all clients.  If a particular client requires clang, that 
> > client should initialize clang.
> > 
> > With an assert, this is requiring a non clang-based client to run some 
> > initialization code that is only required for a clang-based client, which 
> > doesn't seem like a reasonable restriction (imagine if every downstream 
> > developer using every possible set of random 3rd party libraries started 
> > asserting in low-level debugger code that their optional component had been 
> > initialized).
> In short, `Core` is too low level to be making any assumptions whatsoever 
> about the needs of a particular client.  It may be required for all clients 
> of lldb that use clang, but `Core` is not the right place to be making 
> decisions based on whether a client of lldb uses clang (or any other optional 
> external library / component).
To put this in perspective, imagine if LLVM's optimization pass library had 
something like `assert(driverIsClang());`


https://reviews.llvm.org/D47235



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

zturner wrote:
> zturner wrote:
> > zturner wrote:
> > > aprantl wrote:
> > > > zturner wrote:
> > > > > aprantl wrote:
> > > > > > zturner wrote:
> > > > > > > I don't think this should be an assert.  After all, if the whole 
> > > > > > > point is to make LLDB usable in situations where clang is not 
> > > > > > > present, then someone using it in such an environment would 
> > > > > > > probably never call the static function to begin with.  So I 
> > > > > > > think we should just remove the assert and set it to whatever the 
> > > > > > > value happens to be (including empty)
> > > > > > The assertion enforces that ModuleListProperties::Initialize() has 
> > > > > > been called. If we want to make it more convenient, we can add a 
> > > > > > default argument `= "dummy"` for clients that don't link against 
> > > > > > clang.
> > > > > I was actually thinking that instead of calling it `Initialize` 
> > > > > (which sounds generic and like it's required) we would just call it 
> > > > > `SetDefaultClangModulesCachePath` and have the user directly call 
> > > > > that.  With a name like `Initialize`, it makes the user think that 
> > > > > it's required, but in fact the only thing it's initializing is 
> > > > > something that is optional, so it shouldn't be required.
> > > > > 
> > > > > It's possible I'm misunderstanding something though.
> > > > My point was that this *is* required (for all clients of lldb that also 
> > > > link against clang). When LLDB initializes clang it must set a module 
> > > > cache path because clang doesn't implement a fallback.
> > > If there's a client of LLDB using the public API and/or clang then that 
> > > client would also be using `SystemInitializerFull` (or at the very least, 
> > > would be responsible for initializing the set of things they need, one of 
> > > which would be this path).
> > > 
> > > My point is that `Core` should ultimately have no knowledge that 
> > > something called clang even exists, and it definitely shouldn't be 
> > > limiting the use of itself based on the needs of a specific client since 
> > > it something that is useful to all clients.  If a particular client 
> > > requires clang, that client should initialize clang.
> > > 
> > > With an assert, this is requiring a non clang-based client to run some 
> > > initialization code that is only required for a clang-based client, which 
> > > doesn't seem like a reasonable restriction (imagine if every downstream 
> > > developer using every possible set of random 3rd party libraries started 
> > > asserting in low-level debugger code that their optional component had 
> > > been initialized).
> > In short, `Core` is too low level to be making any assumptions whatsoever 
> > about the needs of a particular client.  It may be required for all clients 
> > of lldb that use clang, but `Core` is not the right place to be making 
> > decisions based on whether a client of lldb uses clang (or any other 
> > optional external library / component).
> To put this in perspective, imagine if LLVM's optimization pass library had 
> something like `assert(driverIsClang());`
The assertion is not supposed to check that Clang has been initialized. It is 
supposed to check that ModuleListProperties::Initialize() has been called. The 
fact that in order to call this function a client may want to get a string from 
the Clang Driver is an (ugly) implementation detail. And clients that don't use 
clang (such as the confusingly named unit tests) can pass in any nonempty 
string (which as I offered earlier could be made into a default argument).


https://reviews.llvm.org/D47235



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

[Lldb-commits] [PATCH] D47275: 1/3: DWARFDIE split out to DWARFBasicDIE

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Marked things that don't belong in DWARFBasicDIE.

Also DWARFBasicDIE doesn't really explain what it actually is. Maybe we should 
rename this DWARFUnitDIE? DWARFTopDIE? DWARFRootDIE?




Comment at: source/Plugins/SymbolFile/DWARF/DWARFBasicDIE.h:49
+
+  bool IsStructOrClass() const;
+

This will never be true for DWARFBasicDIE. Remove?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFBasicDIE.h:115
+
+  const char *GetMangledName() const;
+

This will never be useful for DWARFBasicDIE since it will represent the first 
DIE. Remove?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFBasicDIE.h:117
+
+  const char *GetPubname() const;
+

This will never be useful for DWARFBasicDIE since it will represent the first 
DIE. Remove?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFBasicDIE.h:119
+
+  const char *GetQualifiedName(std::string &storage) const;
+

This will never be useful for DWARFBasicDIE since it will represent the first 
DIE. Remove?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFBasicDIE.h:151-155
+  bool GetDIENamesAndRanges(const char *&name, const char *&mangled,
+DWARFRangeList &ranges, int &decl_file,
+int &decl_line, int &decl_column, int &call_file,
+int &call_line, int &call_column,
+lldb_private::DWARFExpression *frame_base) const;

This will never be useful for DWARFBasicDIE since it will represent the first 
DIE. Remove?


https://reviews.llvm.org/D47275



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

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

It would be good to verify all headers that are in the LLDB.framework produced 
by Xcode builds are also in the LLDB.framework from cmake/ninja


https://reviews.llvm.org/D47278



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

[Lldb-commits] [PATCH] D47276: 2/3: Use DWARFBasicDIE as compile-time protection

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Waiting for answers on renaming from patch 1/3 before commenting.


https://reviews.llvm.org/D47276



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

aprantl wrote:
> zturner wrote:
> > zturner wrote:
> > > zturner wrote:
> > > > aprantl wrote:
> > > > > zturner wrote:
> > > > > > aprantl wrote:
> > > > > > > zturner wrote:
> > > > > > > > I don't think this should be an assert.  After all, if the 
> > > > > > > > whole point is to make LLDB usable in situations where clang is 
> > > > > > > > not present, then someone using it in such an environment would 
> > > > > > > > probably never call the static function to begin with.  So I 
> > > > > > > > think we should just remove the assert and set it to whatever 
> > > > > > > > the value happens to be (including empty)
> > > > > > > The assertion enforces that ModuleListProperties::Initialize() 
> > > > > > > has been called. If we want to make it more convenient, we can 
> > > > > > > add a default argument `= "dummy"` for clients that don't link 
> > > > > > > against clang.
> > > > > > I was actually thinking that instead of calling it `Initialize` 
> > > > > > (which sounds generic and like it's required) we would just call it 
> > > > > > `SetDefaultClangModulesCachePath` and have the user directly call 
> > > > > > that.  With a name like `Initialize`, it makes the user think that 
> > > > > > it's required, but in fact the only thing it's initializing is 
> > > > > > something that is optional, so it shouldn't be required.
> > > > > > 
> > > > > > It's possible I'm misunderstanding something though.
> > > > > My point was that this *is* required (for all clients of lldb that 
> > > > > also link against clang). When LLDB initializes clang it must set a 
> > > > > module cache path because clang doesn't implement a fallback.
> > > > If there's a client of LLDB using the public API and/or clang then that 
> > > > client would also be using `SystemInitializerFull` (or at the very 
> > > > least, would be responsible for initializing the set of things they 
> > > > need, one of which would be this path).
> > > > 
> > > > My point is that `Core` should ultimately have no knowledge that 
> > > > something called clang even exists, and it definitely shouldn't be 
> > > > limiting the use of itself based on the needs of a specific client 
> > > > since it something that is useful to all clients.  If a particular 
> > > > client requires clang, that client should initialize clang.
> > > > 
> > > > With an assert, this is requiring a non clang-based client to run some 
> > > > initialization code that is only required for a clang-based client, 
> > > > which doesn't seem like a reasonable restriction (imagine if every 
> > > > downstream developer using every possible set of random 3rd party 
> > > > libraries started asserting in low-level debugger code that their 
> > > > optional component had been initialized).
> > > In short, `Core` is too low level to be making any assumptions whatsoever 
> > > about the needs of a particular client.  It may be required for all 
> > > clients of lldb that use clang, but `Core` is not the right place to be 
> > > making decisions based on whether a client of lldb uses clang (or any 
> > > other optional external library / component).
> > To put this in perspective, imagine if LLVM's optimization pass library had 
> > something like `assert(driverIsClang());`
> The assertion is not supposed to check that Clang has been initialized. It is 
> supposed to check that ModuleListProperties::Initialize() has been called. 
> The fact that in order to call this function a client may want to get a 
> string from the Clang Driver is an (ugly) implementation detail. And clients 
> that don't use clang (such as the confusingly named unit tests) can pass in 
> any nonempty string (which as I offered earlier could be made into a default 
> argument).
But why must it even be a nonempty string?  And for that matter, if they're not 
going to use clang anyway, why even require the function to be called in the 
first place?  If it were an initialization function that did multiple things, 
it might be a stronger argument.  But as it stands, its only purpose is, in 
fact, to set a value for this path, which people who aren't using clang 
shouldn't be required to do.

This is making a decision in a low level library for the purpose of 1 specific 
client, which doesn't seem right.  I'm not entirely opposed to an assert, but 
it should only happen in clients that are using clang, otherwise this is 
effectively 'assert that the user has executed a no-op', which doesn't make 
sense.


https://reviews.llvm.org/D47235



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

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

zturner wrote:
> aprantl wrote:
> > zturner wrote:
> > > zturner wrote:
> > > > zturner wrote:
> > > > > aprantl wrote:
> > > > > > zturner wrote:
> > > > > > > aprantl wrote:
> > > > > > > > zturner wrote:
> > > > > > > > > I don't think this should be an assert.  After all, if the 
> > > > > > > > > whole point is to make LLDB usable in situations where clang 
> > > > > > > > > is not present, then someone using it in such an environment 
> > > > > > > > > would probably never call the static function to begin with.  
> > > > > > > > > So I think we should just remove the assert and set it to 
> > > > > > > > > whatever the value happens to be (including empty)
> > > > > > > > The assertion enforces that ModuleListProperties::Initialize() 
> > > > > > > > has been called. If we want to make it more convenient, we can 
> > > > > > > > add a default argument `= "dummy"` for clients that don't link 
> > > > > > > > against clang.
> > > > > > > I was actually thinking that instead of calling it `Initialize` 
> > > > > > > (which sounds generic and like it's required) we would just call 
> > > > > > > it `SetDefaultClangModulesCachePath` and have the user directly 
> > > > > > > call that.  With a name like `Initialize`, it makes the user 
> > > > > > > think that it's required, but in fact the only thing it's 
> > > > > > > initializing is something that is optional, so it shouldn't be 
> > > > > > > required.
> > > > > > > 
> > > > > > > It's possible I'm misunderstanding something though.
> > > > > > My point was that this *is* required (for all clients of lldb that 
> > > > > > also link against clang). When LLDB initializes clang it must set a 
> > > > > > module cache path because clang doesn't implement a fallback.
> > > > > If there's a client of LLDB using the public API and/or clang then 
> > > > > that client would also be using `SystemInitializerFull` (or at the 
> > > > > very least, would be responsible for initializing the set of things 
> > > > > they need, one of which would be this path).
> > > > > 
> > > > > My point is that `Core` should ultimately have no knowledge that 
> > > > > something called clang even exists, and it definitely shouldn't be 
> > > > > limiting the use of itself based on the needs of a specific client 
> > > > > since it something that is useful to all clients.  If a particular 
> > > > > client requires clang, that client should initialize clang.
> > > > > 
> > > > > With an assert, this is requiring a non clang-based client to run 
> > > > > some initialization code that is only required for a clang-based 
> > > > > client, which doesn't seem like a reasonable restriction (imagine if 
> > > > > every downstream developer using every possible set of random 3rd 
> > > > > party libraries started asserting in low-level debugger code that 
> > > > > their optional component had been initialized).
> > > > In short, `Core` is too low level to be making any assumptions 
> > > > whatsoever about the needs of a particular client.  It may be required 
> > > > for all clients of lldb that use clang, but `Core` is not the right 
> > > > place to be making decisions based on whether a client of lldb uses 
> > > > clang (or any other optional external library / component).
> > > To put this in perspective, imagine if LLVM's optimization pass library 
> > > had something like `assert(driverIsClang());`
> > The assertion is not supposed to check that Clang has been initialized. It 
> > is supposed to check that ModuleListProperties::Initialize() has been 
> > called. The fact that in order to call this function a client may want to 
> > get a string from the Clang Driver is an (ugly) implementation detail. And 
> > clients that don't use clang (such as the confusingly named unit tests) can 
> > pass in any nonempty string (which as I offered earlier could be made into 
> > a default argument).
> But why must it even be a nonempty string?  And for that matter, if they're 
> not going to use clang anyway, why even require the function to be called in 
> the first place?  If it were an initialization function that did multiple 
> things, it might be a stronger argument.  But as it stands, its only purpose 
> is, in fact, to set a value for this path, which people who aren't using 
> clang shouldn't be required to do.
> 
> This is making a decision in a low level library for the purpose of 1 
> specific client, which doesn't seem right.  I'm not entirely opposed to an 
> assert, but it should only happen in clients that are using clang, otherwise 
> this is effectively 'assert that the user has executed a no-op', which 
> doesn't mak

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Core/ModuleList.cpp:94
 
-  llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  assert(!g_default_clang_modules_cache_path.empty());
+  SetClangModulesCachePath(g_default_clang_modules_cache_path);

zturner wrote:
> zturner wrote:
> > aprantl wrote:
> > > zturner wrote:
> > > > zturner wrote:
> > > > > zturner wrote:
> > > > > > aprantl wrote:
> > > > > > > zturner wrote:
> > > > > > > > aprantl wrote:
> > > > > > > > > zturner wrote:
> > > > > > > > > > I don't think this should be an assert.  After all, if the 
> > > > > > > > > > whole point is to make LLDB usable in situations where 
> > > > > > > > > > clang is not present, then someone using it in such an 
> > > > > > > > > > environment would probably never call the static function 
> > > > > > > > > > to begin with.  So I think we should just remove the assert 
> > > > > > > > > > and set it to whatever the value happens to be (including 
> > > > > > > > > > empty)
> > > > > > > > > The assertion enforces that 
> > > > > > > > > ModuleListProperties::Initialize() has been called. If we 
> > > > > > > > > want to make it more convenient, we can add a default 
> > > > > > > > > argument `= "dummy"` for clients that don't link against 
> > > > > > > > > clang.
> > > > > > > > I was actually thinking that instead of calling it `Initialize` 
> > > > > > > > (which sounds generic and like it's required) we would just 
> > > > > > > > call it `SetDefaultClangModulesCachePath` and have the user 
> > > > > > > > directly call that.  With a name like `Initialize`, it makes 
> > > > > > > > the user think that it's required, but in fact the only thing 
> > > > > > > > it's initializing is something that is optional, so it 
> > > > > > > > shouldn't be required.
> > > > > > > > 
> > > > > > > > It's possible I'm misunderstanding something though.
> > > > > > > My point was that this *is* required (for all clients of lldb 
> > > > > > > that also link against clang). When LLDB initializes clang it 
> > > > > > > must set a module cache path because clang doesn't implement a 
> > > > > > > fallback.
> > > > > > If there's a client of LLDB using the public API and/or clang then 
> > > > > > that client would also be using `SystemInitializerFull` (or at the 
> > > > > > very least, would be responsible for initializing the set of things 
> > > > > > they need, one of which would be this path).
> > > > > > 
> > > > > > My point is that `Core` should ultimately have no knowledge that 
> > > > > > something called clang even exists, and it definitely shouldn't be 
> > > > > > limiting the use of itself based on the needs of a specific client 
> > > > > > since it something that is useful to all clients.  If a particular 
> > > > > > client requires clang, that client should initialize clang.
> > > > > > 
> > > > > > With an assert, this is requiring a non clang-based client to run 
> > > > > > some initialization code that is only required for a clang-based 
> > > > > > client, which doesn't seem like a reasonable restriction (imagine 
> > > > > > if every downstream developer using every possible set of random 
> > > > > > 3rd party libraries started asserting in low-level debugger code 
> > > > > > that their optional component had been initialized).
> > > > > In short, `Core` is too low level to be making any assumptions 
> > > > > whatsoever about the needs of a particular client.  It may be 
> > > > > required for all clients of lldb that use clang, but `Core` is not 
> > > > > the right place to be making decisions based on whether a client of 
> > > > > lldb uses clang (or any other optional external library / component).
> > > > To put this in perspective, imagine if LLVM's optimization pass library 
> > > > had something like `assert(driverIsClang());`
> > > The assertion is not supposed to check that Clang has been initialized. 
> > > It is supposed to check that ModuleListProperties::Initialize() has been 
> > > called. The fact that in order to call this function a client may want to 
> > > get a string from the Clang Driver is an (ugly) implementation detail. 
> > > And clients that don't use clang (such as the confusingly named unit 
> > > tests) can pass in any nonempty string (which as I offered earlier could 
> > > be made into a default argument).
> > But why must it even be a nonempty string?  And for that matter, if they're 
> > not going to use clang anyway, why even require the function to be called 
> > in the first place?  If it were an initialization function that did 
> > multiple things, it might be a stronger argument.  But as it stands, its 
> > only purpose is, in fact, to set a value for this path, which people who 
> > aren't using clang shouldn't be required to do.
> > 
> > This is making a decision in a low level library for the purpose of 1 
> > specific client, which doesn't 

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

I also think that'd be a great idea. The only way I can think to do that would 
be to build LLDB.framework using both build systems and compare the two. Is 
there a faster way?


https://reviews.llvm.org/D47278



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

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D47278#1110092, @xiaobai wrote:

> I also think that'd be a great idea. The only way I can think to do that 
> would be to build LLDB.framework using both build systems and compare the 
> two. Is there a faster way?


That is the only guaranteed way as new headers could come along. LLDB.framework 
doesn't have headers in installed Xcode.app bundles


https://reviews.llvm.org/D47278



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

Re: [Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via lldb-commits]

Actually, now I’m starting to wonder why can’t
ClangModulesDeclVendor.cpp just call this function in clangDriver to get
the default if the accessor returns an empty string? That sidesteps all of
this initialization funny business and lets the client just handle it.

Would that work?
On Wed, May 23, 2018 at 1:25 PM Zachary Turner via Phabricator <
revi...@reviews.llvm.org> wrote:

> zturner added inline comments.
>
>
> 
> Comment at: source/Core/ModuleList.cpp:94
>
> -  llvm::SmallString<128> path;
> -  clang::driver::Driver::getDefaultModuleCachePath(path);
> -  SetClangModulesCachePath(path);
> +  assert(!g_default_clang_modules_cache_path.empty());
> +  SetClangModulesCachePath(g_default_clang_modules_cache_path);
> 
> zturner wrote:
> > zturner wrote:
> > > aprantl wrote:
> > > > zturner wrote:
> > > > > zturner wrote:
> > > > > > zturner wrote:
> > > > > > > aprantl wrote:
> > > > > > > > zturner wrote:
> > > > > > > > > aprantl wrote:
> > > > > > > > > > zturner wrote:
> > > > > > > > > > > I don't think this should be an assert.  After all, if
> the whole point is to make LLDB usable in situations where clang is not
> present, then someone using it in such an environment would probably never
> call the static function to begin with.  So I think we should just remove
> the assert and set it to whatever the value happens to be (including empty)
> > > > > > > > > > The assertion enforces that
> ModuleListProperties::Initialize() has been called. If we want to make it
> more convenient, we can add a default argument `= "dummy"` for clients that
> don't link against clang.
> > > > > > > > > I was actually thinking that instead of calling it
> `Initialize` (which sounds generic and like it's required) we would just
> call it `SetDefaultClangModulesCachePath` and have the user directly call
> that.  With a name like `Initialize`, it makes the user think that it's
> required, but in fact the only thing it's initializing is something that is
> optional, so it shouldn't be required.
> > > > > > > > >
> > > > > > > > > It's possible I'm misunderstanding something though.
> > > > > > > > My point was that this *is* required (for all clients of
> lldb that also link against clang). When LLDB initializes clang it must set
> a module cache path because clang doesn't implement a fallback.
> > > > > > > If there's a client of LLDB using the public API and/or clang
> then that client would also be using `SystemInitializerFull` (or at the
> very least, would be responsible for initializing the set of things they
> need, one of which would be this path).
> > > > > > >
> > > > > > > My point is that `Core` should ultimately have no knowledge
> that something called clang even exists, and it definitely shouldn't be
> limiting the use of itself based on the needs of a specific client since it
> something that is useful to all clients.  If a particular client requires
> clang, that client should initialize clang.
> > > > > > >
> > > > > > > With an assert, this is requiring a non clang-based client to
> run some initialization code that is only required for a clang-based
> client, which doesn't seem like a reasonable restriction (imagine if every
> downstream developer using every possible set of random 3rd party libraries
> started asserting in low-level debugger code that their optional component
> had been initialized).
> > > > > > In short, `Core` is too low level to be making any assumptions
> whatsoever about the needs of a particular client.  It may be required for
> all clients of lldb that use clang, but `Core` is not the right place to be
> making decisions based on whether a client of lldb uses clang (or any other
> optional external library / component).
> > > > > To put this in perspective, imagine if LLVM's optimization pass
> library had something like `assert(driverIsClang());`
> > > > The assertion is not supposed to check that Clang has been
> initialized. It is supposed to check that
> ModuleListProperties::Initialize() has been called. The fact that in order
> to call this function a client may want to get a string from the Clang
> Driver is an (ugly) implementation detail. And clients that don't use clang
> (such as the confusingly named unit tests) can pass in any nonempty string
> (which as I offered earlier could be made into a default argument).
> > > But why must it even be a nonempty string?  And for that matter, if
> they're not going to use clang anyway, why even require the function to be
> called in the first place?  If it were an initialization function that did
> multiple things, it might be a stronger argument.  But as it stands, its
> only purpose is, in fact, to set a value for this path, which people who
> aren't using clang shouldn't be required to do.
> > >
> > > This is making a decision in a low level library for the purpose of 1
> specific client, which doesn't seem right.  I'm not entirely opposed to an
> assert, but it should only happen in clie

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

Actually, now I’m starting to wonder why can’t
ClangModulesDeclVendor.cpp just call this function in clangDriver to get
the default if the accessor returns an empty string? That sidesteps all of
this initialization funny business and lets the client just handle it.

Would that work?


https://reviews.llvm.org/D47235



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

Re: [Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

[Adrian Prantl via lldb-commits]

> On May 23, 2018, at 1:33 PM, Zachary Turner  wrote:
> 
> Actually, now I’m starting to wonder why can’t ClangModulesDeclVendor.cpp 
> just call this function in clangDriver to get the default if the accessor 
> returns an empty string? That sidesteps all of this initialization funny 
> business and lets the client just handle it.

Since it is surfaced as a property, users expect that "settings list" returns a 
proper value. ClangModulesDeclVendor is only initialized once a Target exists. 
Xcode may also use this property to set the module cache path and initializing 
the property later than at LLDB initialization time could create a race 
condition.
ClangModulesDeclVendor also isn't the only user of the property. The Swift 
Plugin also needs to know it and I'd like to avoid duplicating the code.


-- adrian


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

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In https://reviews.llvm.org/D47278#1110104, @clayborg wrote:

> That is the only guaranteed way as new headers could come along. 
> LLDB.framework doesn't have headers in installed Xcode.app bundles


In that case, adding this test could double build times since we would 
effectively be building lldb twice. I'm not sure if this is already a pain 
point, but if it is, then I don't think we should add this as a test right now.


https://reviews.llvm.org/D47278



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

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

sorry, not as a test, but just as a way to figure out if we are getting all the 
needed header files when we modify this framework header file copying code.


https://reviews.llvm.org/D47278



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

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In https://reviews.llvm.org/D47278#1110155, @clayborg wrote:

> sorry, not as a test, but just as a way to figure out if we are getting all 
> the needed header files when we modify this framework header file copying 
> code.


Ah, yeah. I'm in the process of trying to build LLDB.framework with cmake+ninja 
and get the same thing as when you build with xcodebuild.
As far as headers go, we roughly have parity after this change. CMake copies 
SystemInitializerFull.h into LLDB.Framework/Headers while Xcode doesn't, but 
I'm not sure that actually matters that much.


https://reviews.llvm.org/D47278



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

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D47278#1110164, @xiaobai wrote:

> In https://reviews.llvm.org/D47278#1110155, @clayborg wrote:
>
> > sorry, not as a test, but just as a way to figure out if we are getting all 
> > the needed header files when we modify this framework header file copying 
> > code.
>
>
> Ah, yeah. I'm in the process of trying to build LLDB.framework with 
> cmake+ninja and get the same thing as when you build with xcodebuild.
>  As far as headers go, we roughly have parity after this change. CMake copies 
> SystemInitializerFull.h into LLDB.Framework/Headers while Xcode doesn't, but 
> I'm not sure that actually matters that much.


It does as we can't expose any lldb_private functions and this exposes 
lldb_private::SystemInitializerFull which requires 
lldb_private::SystemInitializerCommon. The only references to stuff inside 
lldb_private in headers that make it into the LLDB.framework can be forward 
references like:

  namespace lldb_private {
 class Foo;
  }


https://reviews.llvm.org/D47278



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

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In https://reviews.llvm.org/D47278#1110171, @clayborg wrote:

> In https://reviews.llvm.org/D47278#1110164, @xiaobai wrote:
>
> > In https://reviews.llvm.org/D47278#1110155, @clayborg wrote:
> >
> > > sorry, not as a test, but just as a way to figure out if we are getting 
> > > all the needed header files when we modify this framework header file 
> > > copying code.
> >
> >
> > Ah, yeah. I'm in the process of trying to build LLDB.framework with 
> > cmake+ninja and get the same thing as when you build with xcodebuild.
> >  As far as headers go, we roughly have parity after this change. CMake 
> > copies SystemInitializerFull.h into LLDB.Framework/Headers while Xcode 
> > doesn't, but I'm not sure that actually matters that much.
>
>
> It does as we can't expose any lldb_private functions and this exposes 
> lldb_private::SystemInitializerFull which requires 
> lldb_private::SystemInitializerCommon. The only references to stuff inside 
> lldb_private in headers that make it into the LLDB.framework can be forward 
> references like:
>
>   namespace lldb_private {
>  class Foo;
>   }
>


I see, thanks for explaining why it matters. I'll update this revision.


https://reviews.llvm.org/D47278



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

[Lldb-commits] [PATCH] D47285: Add PPC64le support information

[Alexandre Yukio Yamashita via Phabricator via lldb-commits]

alexandreyy created this revision.
alexandreyy added a reviewer: clayborg.

Add PPC64le support information on LLDB site


https://reviews.llvm.org/D47285

Files:
  www/features.html
  www/index.html
  www/status.html


Index: www/status.html
===
--- www/status.html
+++ www/status.html
@@ -60,7 +60,7 @@
 
 Feature
 FreeBSD(x86_64)
-Linux(x86_64)
+Linux(x86_64 and PPC64le)
 Mac OS X (i386/x86_64 and ARM/Thumb)
 Windows (i386)
 
Index: www/index.html
===
--- www/index.html
+++ www/index.html
@@ -92,7 +92,7 @@
Mac OS X desktop user space 
debugging for i386 and x86-64
iOS simulator debugging on 
i386
iOS device debugging on 
ARM
-   Linux local user-space 
debugging for i386 and x86-64
+   Linux local user-space 
debugging for i386, x86-64 and PPC64le
FreeBSD local user-space 
debugging for i386 and x86-64
Windows local user-space 
debugging for i386 (*)

Index: www/features.html
===
--- www/features.html
+++ www/features.html
@@ -38,7 +38,7 @@
   for an executable object.
   Disassembly plug-ins for each 
architecture. Support currently includes
   an LLVM disassembler for http://blog.llvm.org/2010/01/x86-disassembler.html";>i386, x86-64
-  , & ARM/Thumb.
+  , ARM/Thumb, and PPC64le
Debugger plug-ins implement the 
host and target specific functions
required to debug.

@@ -57,4 +57,4 @@

 
 
-
\ No newline at end of file
+


Index: www/status.html
===
--- www/status.html
+++ www/status.html
@@ -60,7 +60,7 @@
 
 Feature
 FreeBSD(x86_64)
-Linux(x86_64)
+Linux(x86_64 and PPC64le)
 Mac OS X (i386/x86_64 and ARM/Thumb)
 Windows (i386)
 
Index: www/index.html
===
--- www/index.html
+++ www/index.html
@@ -92,7 +92,7 @@
 	   	Mac OS X desktop user space debugging for i386 and x86-64
 	   	iOS simulator debugging on i386
 	   	iOS device debugging on ARM
-	   	Linux local user-space debugging for i386 and x86-64
+	   	Linux local user-space debugging for i386, x86-64 and PPC64le
 	   	FreeBSD local user-space debugging for i386 and x86-64
 	   	Windows local user-space debugging for i386 (*)
 
Index: www/features.html
===
--- www/features.html
+++ www/features.html
@@ -38,7 +38,7 @@
 	   for an executable object.
 	   Disassembly plug-ins for each architecture. Support currently includes
 	   an LLVM disassembler for http://blog.llvm.org/2010/01/x86-disassembler.html";>i386, x86-64
-	   , & ARM/Thumb.
+	   , ARM/Thumb, and PPC64le
 	Debugger plug-ins implement the host and target specific functions
 	required to debug.
 	
@@ -57,4 +57,4 @@
 	
 
 
-
\ No newline at end of file
+
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47285: Add PPC64le support information

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Is PPC64le only supported? no PPC64?


https://reviews.llvm.org/D47285



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

[Lldb-commits] [lldb] r333134 - Remove spurious dependency on Process/elf-core from Process/Utility.

[James Y Knight via lldb-commits]

Author: jyknight
Date: Wed May 23 15:04:20 2018
New Revision: 333134

URL: http://llvm.org/viewvc/llvm-project?rev=333134&view=rev
Log:
Remove spurious dependency on Process/elf-core from Process/Utility.

These checks do absolutely nothing other than cause a library layering
violation in the code.

Modified:
lldb/trunk/source/Plugins/Process/Utility/CMakeLists.txt
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_powerpc.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_ppc64le.cpp
lldb/trunk/unittests/Symbol/CMakeLists.txt

Modified: lldb/trunk/source/Plugins/Process/Utility/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/CMakeLists.txt?rev=333134&r1=333133&r2=333134&view=diff
==
--- lldb/trunk/source/Plugins/Process/Utility/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/Process/Utility/CMakeLists.txt Wed May 23 
15:04:20 2018
@@ -58,7 +58,6 @@ add_lldb_library(lldbPluginProcessUtilit
 lldbSymbol
 lldbTarget
 lldbUtility
-lldbPluginProcessElfCore
   LINK_COMPONENTS
 Support
   )

Modified: lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm.cpp?rev=333134&r1=333133&r2=333134&view=diff
==
--- lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm.cpp Wed 
May 23 15:04:20 2018
@@ -13,6 +13,7 @@
 
 #include "lldb/Core/RegisterValue.h"
 #include "lldb/Core/Scalar.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/DataBufferHeap.h"
@@ -20,7 +21,6 @@
 #include "lldb/Utility/Endian.h"
 #include "llvm/Support/Compiler.h"
 
-#include "Plugins/Process/elf-core/ProcessElfCore.h"
 #include "RegisterContextPOSIX_arm.h"
 
 using namespace lldb;
@@ -107,11 +107,6 @@ RegisterContextPOSIX_arm::RegisterContex
   }
 
   ::memset(&m_fpr, 0, sizeof m_fpr);
-
-  // elf-core yet to support ReadFPR()
-  lldb::ProcessSP base = CalculateProcess();
-  if (base.get()->GetPluginName() == ProcessElfCore::GetPluginNameStatic())
-return;
 }
 
 RegisterContextPOSIX_arm::~RegisterContextPOSIX_arm() {}

Modified: 
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp?rev=333134&r1=333133&r2=333134&view=diff
==
--- lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp 
Wed May 23 15:04:20 2018
@@ -13,6 +13,7 @@
 
 #include "lldb/Core/RegisterValue.h"
 #include "lldb/Core/Scalar.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/DataBufferHeap.h"
@@ -20,7 +21,6 @@
 #include "lldb/Utility/Endian.h"
 #include "llvm/Support/Compiler.h"
 
-#include "Plugins/Process/elf-core/ProcessElfCore.h"
 #include "RegisterContextPOSIX_arm64.h"
 
 using namespace lldb;
@@ -126,11 +126,6 @@ RegisterContextPOSIX_arm64::RegisterCont
   }
 
   ::memset(&m_fpr, 0, sizeof m_fpr);
-
-  // elf-core yet to support ReadFPR()
-  lldb::ProcessSP base = CalculateProcess();
-  if (base.get()->GetPluginName() == ProcessElfCore::GetPluginNameStatic())
-return;
 }
 
 RegisterContextPOSIX_arm64::~RegisterContextPOSIX_arm64() {}

Modified: 
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp?rev=333134&r1=333133&r2=333134&view=diff
==
--- lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp 
Wed May 23 15:04:20 2018
@@ -13,6 +13,7 @@
 
 #include "lldb/Core/RegisterValue.h"
 #include "lldb/Core/Scalar.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/DataBufferHeap.h"
@@ -20,11 +21,10 @@
 #include "lldb/Utility/Endian.h"
 #include "llvm/Support/Compiler.h"
 
-#include "Plugins/Process/elf-core/ProcessElfCore.h"
 #include "RegisterContextPOSIX_mips64.h"
 #include "RegisterContextFreeBSD_mips64.

[Lldb-commits] [PATCH] D47285: Add PPC64le support information

[Alexandre Yukio Yamashita via Phabricator via lldb-commits]

alexandreyy added a comment.

In https://reviews.llvm.org/D47285#1110232, @clayborg wrote:

> Is PPC64le only supported? no PPC64?


Only PPC64le is supported.


https://reviews.llvm.org/D47285



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

[Lldb-commits] [PATCH] D47285: Add PPC64le support information

[Alexandre Yukio Yamashita via Phabricator via lldb-commits]

alexandreyy added a comment.

In https://reviews.llvm.org/D47285#1110260, @alexandreyy wrote:

> In https://reviews.llvm.org/D47285#1110232, @clayborg wrote:
>
> > Is PPC64le only supported? no PPC64?
>
>
> Only PPC64le is supported.


Thanks @clayborg 
@labath Could you commit this patch?


https://reviews.llvm.org/D47285



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

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Alex Langford via Phabricator via lldb-commits]

xiaobai updated this revision to Diff 148305.
xiaobai added a comment.

Remove SystemInitializerFull.h from framework headers


https://reviews.llvm.org/D47278

Files:
  source/API/CMakeLists.txt


Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -162,8 +162,9 @@
 endif()
 
 if(LLDB_BUILD_FRAMEWORK)
-  file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h
-  ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
+  file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
+  list(REMOVE_ITEM public_headers
+${LLDB_SOURCE_DIR}/include/lldb/API/SystemInitializerFull.h)
   file(GLOB root_public_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
   file(GLOB root_private_headers 
${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h)
   list(REMOVE_ITEM root_public_headers ${root_private_headers})


Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -162,8 +162,9 @@
 endif()
 
 if(LLDB_BUILD_FRAMEWORK)
-  file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h
-  ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
+  file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
+  list(REMOVE_ITEM public_headers
+${LLDB_SOURCE_DIR}/include/lldb/API/SystemInitializerFull.h)
   file(GLOB root_public_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
   file(GLOB root_private_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h)
   list(REMOVE_ITEM root_public_headers ${root_private_headers})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

The issue is actually that SystemInitializerFull.h and 
SystemInitializerFull.cpp are in the wrong directories. They belong in the 
"lldb/Initialization" and "Source//Initialization". We should fix this and then 
some/all of your changes won't be needed?


https://reviews.llvm.org/D47278



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

[Lldb-commits] [lldb] r333140 - Add a --synchronous option to lldb-mi to facilitate reliable testing.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Wed May 23 16:33:50 2018
New Revision: 333140

URL: http://llvm.org/viewvc/llvm-project?rev=333140&view=rev
Log:
Add a --synchronous option to lldb-mi to facilitate reliable testing.

Patch by Alexander Polyakov!

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

Modified:
lldb/trunk/tools/lldb-mi/MICmnResources.cpp
lldb/trunk/tools/lldb-mi/MICmnResources.h
lldb/trunk/tools/lldb-mi/MIDriver.cpp
lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp

Modified: lldb/trunk/tools/lldb-mi/MICmnResources.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnResources.cpp?rev=333140&r1=333139&r2=333140&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmnResources.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnResources.cpp Wed May 23 16:33:50 2018
@@ -110,6 +110,8 @@ const CMICmnResources::SRsrcTextData
 {IDE_MI_APP_ARG_EXECUTABLE, "executable (NOT IMPLEMENTED)\n\tThe file "
 "path to the executable i.e. '\"C:\\My "
 "Dev\\foo.exe\"'."},
+{IDE_MI_APP_ARG_SYNCHRONOUS, "--synchronous\n\tBlock until each 
command "
+ "has finished executing.\n\tUsed for 
testing only."},
 {IDS_STDIN_ERR_INVALID_PROMPT,
  "Stdin. Invalid prompt description '%s'"},
 {IDS_STDIN_ERR_THREAD_CREATION_FAILED,

Modified: lldb/trunk/tools/lldb-mi/MICmnResources.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnResources.h?rev=333140&r1=333139&r2=333140&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmnResources.h (original)
+++ lldb/trunk/tools/lldb-mi/MICmnResources.h Wed May 23 16:33:50 2018
@@ -77,6 +77,7 @@ enum {
   IDE_MI_APP_ARG_APP_LOG_DIR,
   IDE_MI_APP_ARG_EXAMPLE,
   IDE_MI_APP_ARG_EXECUTABLE,
+  IDE_MI_APP_ARG_SYNCHRONOUS,
 
   IDS_STDIN_ERR_INVALID_PROMPT,
   IDS_STDIN_ERR_THREAD_CREATION_FAILED,

Modified: lldb/trunk/tools/lldb-mi/MIDriver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIDriver.cpp?rev=333140&r1=333139&r2=333140&view=diff
==
--- lldb/trunk/tools/lldb-mi/MIDriver.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIDriver.cpp Wed May 23 16:33:50 2018
@@ -382,6 +382,7 @@ lldb::SBError CMIDriver::DoParseArgs(con
 //  that are only handled by *this driver:
 //  --executable 
 //  --source  or -s 
+//  --synchronous
 //  The application's options --interpreter and --executable in code 
act
 //  very similar.
 //  The --executable is necessary to differentiate whether the MI 
Driver
@@ -397,6 +398,7 @@ lldb::SBError CMIDriver::DoParseArgs(con
 //  argument for a debug session. Using --interpreter on the command
 //  line does not
 //  issue additional commands to initialise a debug session.
+//  Option --synchronous disables an asynchronous mode in the lldb-mi 
driver.
 // Type:Overridden.
 // Args:argc- (R)   An integer that contains the count of arguments
 // that follow in
@@ -469,6 +471,8 @@ lldb::SBError CMIDriver::ParseArgs(const
 // command line
   { // See fn description.
 bHaveExecutableLongOption = true;
+  } else if (strArg.compare("--synchronous") == 0) {
+CMICmnLLDBDebugSessionInfo::Instance().GetDebugger().SetAsync(false);
   }
 }
   }

Modified: lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp?rev=333140&r1=333139&r2=333140&view=diff
==
--- lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp Wed May 23 16:33:50 2018
@@ -640,6 +640,7 @@ CMIUtilString CMIDriverMgr::GetHelpOnCmd
   MIRSRC(IDE_MI_APP_ARG_VERSION), MIRSRC(IDE_MI_APP_ARG_VERSION_LONG),
   MIRSRC(IDE_MI_APP_ARG_INTERPRETER), MIRSRC(IDE_MI_APP_ARG_SOURCE),
   MIRSRC(IDE_MI_APP_ARG_EXECUTEABLE),
+  MIRSRC(IDE_MI_APP_ARG_SYNCHRONOUS),
   CMIUtilString::Format(
   MIRSRC(IDE_MI_APP_ARG_APP_LOG),
   CMICmnLogMediumFile::Instance().GetFileName().c_str()),


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

[Lldb-commits] [PATCH] D47110: [LLDB, lldb-mi] Add option --synchronous.

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333140: Add a --synchronous option to lldb-mi to facilitate 
reliable testing. (authored by adrian, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47110?vs=147806&id=148318#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47110

Files:
  lldb/trunk/tools/lldb-mi/MICmnResources.cpp
  lldb/trunk/tools/lldb-mi/MICmnResources.h
  lldb/trunk/tools/lldb-mi/MIDriver.cpp
  lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp


Index: lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
===
--- lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
+++ lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
@@ -640,6 +640,7 @@
   MIRSRC(IDE_MI_APP_ARG_VERSION), MIRSRC(IDE_MI_APP_ARG_VERSION_LONG),
   MIRSRC(IDE_MI_APP_ARG_INTERPRETER), MIRSRC(IDE_MI_APP_ARG_SOURCE),
   MIRSRC(IDE_MI_APP_ARG_EXECUTEABLE),
+  MIRSRC(IDE_MI_APP_ARG_SYNCHRONOUS),
   CMIUtilString::Format(
   MIRSRC(IDE_MI_APP_ARG_APP_LOG),
   CMICmnLogMediumFile::Instance().GetFileName().c_str()),
Index: lldb/trunk/tools/lldb-mi/MIDriver.cpp
===
--- lldb/trunk/tools/lldb-mi/MIDriver.cpp
+++ lldb/trunk/tools/lldb-mi/MIDriver.cpp
@@ -382,6 +382,7 @@
 //  that are only handled by *this driver:
 //  --executable 
 //  --source  or -s 
+//  --synchronous
 //  The application's options --interpreter and --executable in code 
act
 //  very similar.
 //  The --executable is necessary to differentiate whether the MI 
Driver
@@ -397,6 +398,7 @@
 //  argument for a debug session. Using --interpreter on the command
 //  line does not
 //  issue additional commands to initialise a debug session.
+//  Option --synchronous disables an asynchronous mode in the lldb-mi 
driver.
 // Type:Overridden.
 // Args:argc- (R)   An integer that contains the count of arguments
 // that follow in
@@ -469,6 +471,8 @@
 // command line
   { // See fn description.
 bHaveExecutableLongOption = true;
+  } else if (strArg.compare("--synchronous") == 0) {
+CMICmnLLDBDebugSessionInfo::Instance().GetDebugger().SetAsync(false);
   }
 }
   }
Index: lldb/trunk/tools/lldb-mi/MICmnResources.cpp
===
--- lldb/trunk/tools/lldb-mi/MICmnResources.cpp
+++ lldb/trunk/tools/lldb-mi/MICmnResources.cpp
@@ -110,6 +110,8 @@
 {IDE_MI_APP_ARG_EXECUTABLE, "executable (NOT IMPLEMENTED)\n\tThe file "
 "path to the executable i.e. '\"C:\\My "
 "Dev\\foo.exe\"'."},
+{IDE_MI_APP_ARG_SYNCHRONOUS, "--synchronous\n\tBlock until each 
command "
+ "has finished executing.\n\tUsed for 
testing only."},
 {IDS_STDIN_ERR_INVALID_PROMPT,
  "Stdin. Invalid prompt description '%s'"},
 {IDS_STDIN_ERR_THREAD_CREATION_FAILED,
Index: lldb/trunk/tools/lldb-mi/MICmnResources.h
===
--- lldb/trunk/tools/lldb-mi/MICmnResources.h
+++ lldb/trunk/tools/lldb-mi/MICmnResources.h
@@ -77,6 +77,7 @@
   IDE_MI_APP_ARG_APP_LOG_DIR,
   IDE_MI_APP_ARG_EXAMPLE,
   IDE_MI_APP_ARG_EXECUTABLE,
+  IDE_MI_APP_ARG_SYNCHRONOUS,
 
   IDS_STDIN_ERR_INVALID_PROMPT,
   IDS_STDIN_ERR_THREAD_CREATION_FAILED,


Index: lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
===
--- lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
+++ lldb/trunk/tools/lldb-mi/MIDriverMgr.cpp
@@ -640,6 +640,7 @@
   MIRSRC(IDE_MI_APP_ARG_VERSION), MIRSRC(IDE_MI_APP_ARG_VERSION_LONG),
   MIRSRC(IDE_MI_APP_ARG_INTERPRETER), MIRSRC(IDE_MI_APP_ARG_SOURCE),
   MIRSRC(IDE_MI_APP_ARG_EXECUTEABLE),
+  MIRSRC(IDE_MI_APP_ARG_SYNCHRONOUS),
   CMIUtilString::Format(
   MIRSRC(IDE_MI_APP_ARG_APP_LOG),
   CMICmnLogMediumFile::Instance().GetFileName().c_str()),
Index: lldb/trunk/tools/lldb-mi/MIDriver.cpp
===
--- lldb/trunk/tools/lldb-mi/MIDriver.cpp
+++ lldb/trunk/tools/lldb-mi/MIDriver.cpp
@@ -382,6 +382,7 @@
 //  that are only handled by *this driver:
 //  --executable 
 //  --source  or -s 
+//  --synchronous
 //  The application's options --interpreter and --executable in code act
 //  very similar.
 //  The --executable is necessary to differentiate whether the MI Driver
@@ -397,6 +398,7 @@
 //  argument for a debug session. Using --interpreter on the command
 //  line does not
 //  issue additional commands to initialise a d

[Lldb-commits] [PATCH] D47302: [lldb, lldb-mi] Add method AddCurrentTargetSharedObjectPath to the SBDebugger.

[Alexander Polyakov via Phabricator via lldb-commits]

polyakov.alex created this revision.
polyakov.alex added a reviewer: aprantl.
Herald added a subscriber: llvm-commits.

The new method adds to the current target a path to search for a shared objects.


Repository:
  rL LLVM

https://reviews.llvm.org/D47302

Files:
  include/lldb/API/SBDebugger.h
  source/API/SBDebugger.cpp


Index: source/API/SBDebugger.cpp
===
--- source/API/SBDebugger.cpp
+++ source/API/SBDebugger.cpp
@@ -1123,6 +1123,38 @@
   return false;
 }
 
+bool SBDebugger::AddCurrentTargetSharedObjectPath(const char *from,
+  const char *to,
+  lldb::SBError &sb_error) {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
+  auto target_sp = GetSelectedTarget().GetSP();
+
+  if (target_sp->IsValid()) {
+if (from[0] && to[0]) {
+  if (log)
+log->Printf("SBDebugger::AddCurrentPlatformSharedObjectPath: '%s' -> 
'%s'",
+from, to);
+  target_sp->GetImageSearchPathList().Append(
+  ConstString(from), ConstString(to), true);
+  return true;
+} else {
+  if (from[0])
+sb_error.SetErrorString(" can't be empty> "
+
"(SBDebugger::AddCurrentPlatformSharedObjectPath) "
+"returned false");
+  else
+sb_error.SetErrorString(" can't be empty> "
+
"(SBDebugger::AddCurrentPlatformSharedObjectPath) "
+"returned false");
+  return false;
+}
+  }
+  sb_error.SetErrorString("invalid target "
+  "(SBDebugger::AddCurrentPlatformSharedObjectPath) "
+  "returned false");
+  return false;
+}
+
 bool SBDebugger::GetCloseInputOnEOF() const {
   return (m_opaque_sp ? m_opaque_sp->GetCloseInputOnEOF() : false);
 }
Index: include/lldb/API/SBDebugger.h
===
--- include/lldb/API/SBDebugger.h
+++ include/lldb/API/SBDebugger.h
@@ -161,6 +161,10 @@
 
   bool SetCurrentPlatformSDKRoot(const char *sysroot);
 
+  bool AddCurrentTargetSharedObjectPath(const char *from,
+const char *to,
+lldb::SBError &error);
+
   // FIXME: Once we get the set show stuff in place, the driver won't need
   // an interface to the Set/Get UseExternalEditor.
   bool SetUseExternalEditor(bool input);


Index: source/API/SBDebugger.cpp
===
--- source/API/SBDebugger.cpp
+++ source/API/SBDebugger.cpp
@@ -1123,6 +1123,38 @@
   return false;
 }
 
+bool SBDebugger::AddCurrentTargetSharedObjectPath(const char *from,
+  const char *to,
+  lldb::SBError &sb_error) {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
+  auto target_sp = GetSelectedTarget().GetSP();
+
+  if (target_sp->IsValid()) {
+if (from[0] && to[0]) {
+  if (log)
+log->Printf("SBDebugger::AddCurrentPlatformSharedObjectPath: '%s' -> '%s'",
+from, to);
+  target_sp->GetImageSearchPathList().Append(
+  ConstString(from), ConstString(to), true);
+  return true;
+} else {
+  if (from[0])
+sb_error.SetErrorString(" can't be empty> "
+"(SBDebugger::AddCurrentPlatformSharedObjectPath) "
+"returned false");
+  else
+sb_error.SetErrorString(" can't be empty> "
+"(SBDebugger::AddCurrentPlatformSharedObjectPath) "
+"returned false");
+  return false;
+}
+  }
+  sb_error.SetErrorString("invalid target "
+  "(SBDebugger::AddCurrentPlatformSharedObjectPath) "
+  "returned false");
+  return false;
+}
+
 bool SBDebugger::GetCloseInputOnEOF() const {
   return (m_opaque_sp ? m_opaque_sp->GetCloseInputOnEOF() : false);
 }
Index: include/lldb/API/SBDebugger.h
===
--- include/lldb/API/SBDebugger.h
+++ include/lldb/API/SBDebugger.h
@@ -161,6 +161,10 @@
 
   bool SetCurrentPlatformSDKRoot(const char *sysroot);
 
+  bool AddCurrentTargetSharedObjectPath(const char *from,
+const char *to,
+lldb::SBError &error);
+
   // FIXME: Once we get the set show stuff in place, the driver won't need
   // an interface to the Set/Get UseExternalEditor.
   bool SetUseExternalEditor(bool input);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47302: [lldb, lldb-mi] Add method AddCurrentTargetSharedObjectPath to the SBDebugger.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

The missing context here is that the lldb-mi -target-select command currently 
calls `HandleCommand("target modules search-paths add", ...)`.
Is adding a new SBAPI the right approach to implementing this functionality 
without going through HandleCommand? Or is HandleCommand appropriate in this 
case?


Repository:
  rL LLVM

https://reviews.llvm.org/D47302



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

[Lldb-commits] [lldb] r333143 - Break dependency from Core to ObjectFileJIT.

[Zachary Turner via lldb-commits]

Author: zturner
Date: Wed May 23 16:56:09 2018
New Revision: 333143

URL: http://llvm.org/viewvc/llvm-project?rev=333143&view=rev
Log:
Break dependency from Core to ObjectFileJIT.

The only reason this was here was so that Module could have a
function called CreateJITModule which created things in a special
order.  Instead of making this specific to creating JIT modules,
I converted this into a template function that can create a module
for any type of object file plugin and just forwards arguments
through.  Since the template is not instantiated in Core, the linker
(and header file) dependency moves to the point where it is
instantiated, which only happens in Expression.  Conceptually, this
location also makes more sense for a dependency on ObjectFileJIT.
After all, we JIT expressions so it's no surprise that Expression
needs to make use of ObjectFileJIT.

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

Modified:
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/source/Core/CMakeLists.txt
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Expression/CMakeLists.txt
lldb/trunk/source/Expression/IRExecutionUnit.cpp

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=333143&r1=333142&r2=333143&view=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Wed May 23 16:56:09 2018
@@ -155,8 +155,23 @@ public:
 
   Module(const ModuleSpec &module_spec);
 
-  static lldb::ModuleSP
-  CreateJITModule(const lldb::ObjectFileJITDelegateSP &delegate_sp);
+  template 
+  static lldb::ModuleSP CreateModuleFromObjectFile(Args &&... args) {
+// Must create a module and place it into a shared pointer before we can
+// create an object file since it has a std::weak_ptr back to the module,
+// so we need to control the creation carefully in this static function
+lldb::ModuleSP module_sp(new Module());
+module_sp->m_objfile_sp =
+std::make_shared(module_sp, 
std::forward(args)...);
+
+// Once we get the object file, update our module with the object file's
+// architecture since it might differ in vendor/os if some parts were
+// unknown.
+if (!module_sp->m_objfile_sp->GetArchitecture(module_sp->m_arch))
+  return nullptr;
+
+return module_sp;
+  }
 
   //--
   /// Destructor.

Modified: lldb/trunk/source/Core/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/CMakeLists.txt?rev=333143&r1=333142&r2=333143&view=diff
==
--- lldb/trunk/source/Core/CMakeLists.txt (original)
+++ lldb/trunk/source/Core/CMakeLists.txt Wed May 23 16:56:09 2018
@@ -69,7 +69,6 @@ add_lldb_library(lldbCore
 lldbPluginProcessUtility
 lldbPluginCPlusPlusLanguage
 lldbPluginObjCLanguage
-lldbPluginObjectFileJIT
 ${LLDB_CURSES_LIBS}
 
   LINK_COMPONENTS

Modified: lldb/trunk/source/Core/Module.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Module.cpp?rev=333143&r1=333142&r2=333143&view=diff
==
--- lldb/trunk/source/Core/Module.cpp (original)
+++ lldb/trunk/source/Core/Module.cpp Wed May 23 16:56:09 2018
@@ -53,7 +53,6 @@
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
-#include "Plugins/ObjectFile/JIT/ObjectFileJIT.h"
 
 #include "llvm/ADT/STLExtras.h"// for make_unique
 #include "llvm/Support/Compiler.h" // for LLVM_PRETT...
@@ -1652,26 +1651,6 @@ uint32_t Module::GetVersion(uint32_t *ve
   return 0;
 }
 
-ModuleSP
-Module::CreateJITModule(const lldb::ObjectFileJITDelegateSP &delegate_sp) {
-  if (delegate_sp) {
-// Must create a module and place it into a shared pointer before we can
-// create an object file since it has a std::weak_ptr back to the module,
-// so we need to control the creation carefully in this static function
-ModuleSP module_sp(new Module());
-module_sp->m_objfile_sp =
-std::make_shared(module_sp, delegate_sp);
-if (module_sp->m_objfile_sp) {
-  // Once we get the object file, update our module with the object file's
-  // architecture since it might differ in vendor/os if some parts were
-  // unknown.
-  module_sp->m_objfile_sp->GetArchitecture(module_sp->m_arch);
-}
-return module_sp;
-  }
-  return ModuleSP();
-}
-
 bool Module::GetIsDynamicLinkEditor() {
   ObjectFile *obj_file = GetObjectFile();
 

Modified: lldb/trunk/source/Expression/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/CMakeLists.txt?rev=333143&r1=333142&r2=333143&view=diff
===

[Lldb-commits] [PATCH] D47228: Break dependency from Core to ObjectFileJIT

[Zachary Turner via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333143: Break dependency from Core to ObjectFileJIT. 
(authored by zturner, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47228?vs=148100&id=148322#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47228

Files:
  lldb/trunk/include/lldb/Core/Module.h
  lldb/trunk/source/Core/CMakeLists.txt
  lldb/trunk/source/Core/Module.cpp
  lldb/trunk/source/Expression/CMakeLists.txt
  lldb/trunk/source/Expression/IRExecutionUnit.cpp

Index: lldb/trunk/source/Core/CMakeLists.txt
===
--- lldb/trunk/source/Core/CMakeLists.txt
+++ lldb/trunk/source/Core/CMakeLists.txt
@@ -69,7 +69,6 @@
 lldbPluginProcessUtility
 lldbPluginCPlusPlusLanguage
 lldbPluginObjCLanguage
-lldbPluginObjectFileJIT
 ${LLDB_CURSES_LIBS}
 
   LINK_COMPONENTS
Index: lldb/trunk/source/Core/Module.cpp
===
--- lldb/trunk/source/Core/Module.cpp
+++ lldb/trunk/source/Core/Module.cpp
@@ -53,7 +53,6 @@
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
-#include "Plugins/ObjectFile/JIT/ObjectFileJIT.h"
 
 #include "llvm/ADT/STLExtras.h"// for make_unique
 #include "llvm/Support/Compiler.h" // for LLVM_PRETT...
@@ -1652,26 +1651,6 @@
   return 0;
 }
 
-ModuleSP
-Module::CreateJITModule(const lldb::ObjectFileJITDelegateSP &delegate_sp) {
-  if (delegate_sp) {
-// Must create a module and place it into a shared pointer before we can
-// create an object file since it has a std::weak_ptr back to the module,
-// so we need to control the creation carefully in this static function
-ModuleSP module_sp(new Module());
-module_sp->m_objfile_sp =
-std::make_shared(module_sp, delegate_sp);
-if (module_sp->m_objfile_sp) {
-  // Once we get the object file, update our module with the object file's
-  // architecture since it might differ in vendor/os if some parts were
-  // unknown.
-  module_sp->m_objfile_sp->GetArchitecture(module_sp->m_arch);
-}
-return module_sp;
-  }
-  return ModuleSP();
-}
-
 bool Module::GetIsDynamicLinkEditor() {
   ObjectFile *obj_file = GetObjectFile();
 
Index: lldb/trunk/source/Expression/IRExecutionUnit.cpp
===
--- lldb/trunk/source/Expression/IRExecutionUnit.cpp
+++ lldb/trunk/source/Expression/IRExecutionUnit.cpp
@@ -33,6 +33,7 @@
 #include "lldb/Utility/Log.h"
 
 #include "lldb/../../source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "lldb/../../source/Plugins/ObjectFile/JIT/ObjectFileJIT.h"
 
 using namespace lldb_private;
 
@@ -1225,15 +1226,18 @@
 lldb::ModuleSP IRExecutionUnit::GetJITModule() {
   ExecutionContext exe_ctx(GetBestExecutionContextScope());
   Target *target = exe_ctx.GetTargetPtr();
-  if (target) {
-lldb::ModuleSP jit_module_sp = lldb_private::Module::CreateJITModule(
-std::static_pointer_cast(
-shared_from_this()));
-if (jit_module_sp) {
-  bool changed = false;
-  jit_module_sp->SetLoadAddress(*target, 0, true, changed);
-}
-return jit_module_sp;
-  }
-  return lldb::ModuleSP();
+  if (!target)
+return nullptr;
+
+  auto Delegate = std::static_pointer_cast(
+  shared_from_this());
+
+  lldb::ModuleSP jit_module_sp =
+  lldb_private::Module::CreateModuleFromObjectFile(Delegate);
+  if (!jit_module_sp)
+return nullptr;
+
+  bool changed = false;
+  jit_module_sp->SetLoadAddress(*target, 0, true, changed);
+  return jit_module_sp;
 }
Index: lldb/trunk/source/Expression/CMakeLists.txt
===
--- lldb/trunk/source/Expression/CMakeLists.txt
+++ lldb/trunk/source/Expression/CMakeLists.txt
@@ -30,6 +30,7 @@
 lldbTarget
 lldbUtility
 lldbPluginExpressionParserClang
+lldbPluginObjectFileJIT
 
   LINK_COMPONENTS
 Core
Index: lldb/trunk/include/lldb/Core/Module.h
===
--- lldb/trunk/include/lldb/Core/Module.h
+++ lldb/trunk/include/lldb/Core/Module.h
@@ -155,8 +155,23 @@
 
   Module(const ModuleSpec &module_spec);
 
-  static lldb::ModuleSP
-  CreateJITModule(const lldb::ObjectFileJITDelegateSP &delegate_sp);
+  template 
+  static lldb::ModuleSP CreateModuleFromObjectFile(Args &&... args) {
+// Must create a module and place it into a shared pointer before we can
+// create an object file since it has a std::weak_ptr back to the module,
+// so we need to control the creation carefully in this static function
+lldb::ModuleSP module_sp(new Module());
+module_sp->m_objfile_sp =
+std::make_shared(module_sp, std::forward(args)...);
+
+// Once we get the object file, update our module with the object file

[Lldb-commits] [PATCH] D47302: [lldb, lldb-mi] Add method AddCurrentTargetSharedObjectPath to the SBDebugger.

[Alexander Polyakov via Phabricator via lldb-commits]

polyakov.alex added a comment.

In https://reviews.llvm.org/D47302#1110441, @aprantl wrote:

> The missing context here is that the lldb-mi -target-select command currently 
> calls `HandleCommand("target modules search-paths add", ...)`.
>  Is adding a new SBAPI the right approach to implementing this functionality 
> without going through HandleCommand? Or is HandleCommand appropriate in this 
> case?


This approach is not mandatory, we could do almost the same(except errors 
processing) right in TargetSelect::Execute method. As you can see, the main 
thing here is:

  target_sp->GetImageSearchPathList().Append(
ConstString(from), ConstString(to), true);


Repository:
  rL LLVM

https://reviews.llvm.org/D47302



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

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D47232#1108865, @jingham wrote:

> Perhaps a better way to handle this is to think of REPL.cpp/h as adjuncts of 
> CommandObjectExpression.cpp/h - they mostly define the fancy IOHandler that 
> gets pushed when you run the command in this mode.  I think it would be fine 
> to move them to Command, at which point using all the OptionGroups would be 
> expected.


`Core` doesn't currently have a dependency on `Commands` (few things do 
actually), and I would prefer not to add one.  But several things include 
`REPL.h` (notably `Debugger.cpp` and `Target.cpp`.  So if I move this file to 
`Commands` it would introduce 2 extra edges in the dependency graph both of 
which would introduce a new layering violation.


https://reviews.llvm.org/D47232



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lit/lit.cfg:61
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+

It looks like "lldb-mi" may not be a valid substitution. On Darwin the command

`# RUN: %lldb_mi `

is expanded to

bin/lldb -S .../llvm/tools/lldb/lit/lit-lldb-init_mi < ...

So it behaves like `%lldb`+`_mi.`



Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [lldb] r333145 - Add missing include.

[Zachary Turner via lldb-commits]

Author: zturner
Date: Wed May 23 17:11:24 2018
New Revision: 333145

URL: http://llvm.org/viewvc/llvm-project?rev=333145&view=rev
Log:
Add missing include.

Modified:
lldb/trunk/include/lldb/Core/Module.h

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=333145&r1=333144&r2=333145&view=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Wed May 23 17:11:24 2018
@@ -12,6 +12,7 @@
 
 #include "lldb/Core/Address.h"// for Address
 #include "lldb/Core/ModuleSpec.h" // for ModuleSpec
+#include "lldb/Symbol/ObjectFile.h" // for ObjectFile
 #include "lldb/Symbol/SymbolContextScope.h"
 #include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Target/PathMappingList.h"


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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lit/lit.cfg:61
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+

aprantl wrote:
> It looks like "lldb-mi" may not be a valid substitution. On Darwin the command
> 
> `# RUN: %lldb_mi `
> 
> is expanded to
> 
> bin/lldb -S .../llvm/tools/lldb/lit/lit-lldb-init_mi < ...
> 
> So it behaves like `%lldb`+`_mi.`
> 
Can you see if you can figure out what's going on? Perhaps we need to rename it 
to lldbmi without the underscore?


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

FWIW `REPL` only uses 4 of the 10+ fields of 
`CommandObjectExpression::Options`.  If you don't like the separate struct 
idea, I can just pass them directly as as independent arguments to 
`REPL::SetCommandOptions`.


https://reviews.llvm.org/D47232



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Alexander Polyakov via Phabricator via lldb-commits]

polyakov.alex added inline comments.



Comment at: lit/lit.cfg:61
 
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+

aprantl wrote:
> aprantl wrote:
> > It looks like "lldb-mi" may not be a valid substitution. On Darwin the 
> > command
> > 
> > `# RUN: %lldb_mi `
> > 
> > is expanded to
> > 
> > bin/lldb -S .../llvm/tools/lldb/lit/lit-lldb-init_mi < ...
> > 
> > So it behaves like `%lldb`+`_mi.`
> > 
> Can you see if you can figure out what's going on? Perhaps we need to rename 
> it to lldbmi without the underscore?
On linux it's ok. I'll remove underscore and be appreciated if you check it on 
Darwin.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



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

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

[Alexander Polyakov via Phabricator via lldb-commits]

polyakov.alex updated this revision to Diff 148323.
polyakov.alex added a comment.

Removed underscore in the lldb-mi variable from lit.cfg.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  lit/lit.cfg
  lit/tools/lldb-mi/breakpoint/break-insert.test
  lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
  lit/tools/lldb-mi/breakpoint/lit.local.cfg
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp

Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
Index: lit/tools/lldb-mi/breakpoint/lit.local.cfg
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.test']
Index: lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/inputs/break-insert.c
@@ -0,0 +1,7 @@
+int breakpoint() { // Breakpoint will be set here.
+  return 0;
+}
+
+int main() {
+  return breakpoint();
+}
Index: lit/tools/lldb-mi/breakpoint/break-insert.test
===
--- /dev/null
+++ lit/tools/lldb-mi/breakpoint/break-insert.test
@@ -0,0 +1,15 @@
+# RUN: %cc %p/inputs/break-insert.c -g
+# RUN: %lldbmi < %s | FileCheck %s
+
+# Test that a breakpoint can be inserted before creating a target.
+
+-break-insert breakpoint
+# CHECK: ^done,bkpt={number="1"
+
+-file-exec-and-symbols a.out
+# CHECK: ^done
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
Index: lit/lit.cfg
===
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -58,6 +58,8 @@
 lldb = "%s -S %s/lit-lldb-init" % (lit.util.which('lldb', lldb_tools_dir),
config.test_source_root)
 
+lldbmi = lit.util.which('lldb-mi', lldb_tools_dir)
+
 if not os.path.exists(config.cc):
 config.cc = lit.util.which(config.cc, config.environment['PATH'])
 
@@ -79,6 +81,7 @@
 config.substitutions.append(('%cc', config.cc))
 config.substitutions.append(('%cxx', config.cxx))
 
+config.substitutions.append(('%lldbmi', lldbmi + " --synchronous"))
 config.substitutions.append(('%lldb', lldb))
 
 if debugserver is not None:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47302: [lldb, lldb-mi] Add method AddCurrentTargetSharedObjectPath to the SBDebugger.

[Jim Ingham via Phabricator via lldb-commits]

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

Adding an SB API to set the target's search paths seems fine to me.  I think in 
theory the totality of LLDB functionality should be available through the SB 
API's, (a) because it is always awkward to fall back on HandleCommand and (b) 
because at some point we really should make the command-line be a fairly simple 
client of the SB API's as that would greatly reduce our testing surface.  We're 
only going to get there one bit at a time.

But this seems the wrong way to do it.  You are setting a property on an 
SBTarget so this should be a method on the SBTarget.  The selected target is 
really not reliable.  For instance, having this only available for the selected 
target would make this impossible to use in a breakpoint command, since you 
could have two running targets, target A could be selected but target B could 
hit a breakpoint.  We won't select B till we've decided that B is actually 
going to stop - which you don't know when you are executing breakpoint 
commands.  So API like this should really not rely on the selected target.  
BTW, I see this is right below SBDebugger::SetCurrentPlatformSDKRoot, but I 
don't approve of that one either...

Also, if you're going to the trouble of adding a "AddSharedObjectPath, you 
should also add ListSharedObjectPaths, and that will make it easier to write a 
test for this command.


Repository:
  rL LLVM

https://reviews.llvm.org/D47302



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

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I worry when concerns of layering which seem a little artificial to me would 
make us add a shadow class for something that is already well expressed as it 
is.  If you pass them as arguments, and then I add another useful one and want 
to use it in both the regular and the REPL versions of the expression parser, 
it would be ugly to have to add another argument to this function when I should 
have been able to share the structure.

Seems like the thing you are actually objecting to is the use of 
CommandObjectExpression::CommandOptions in REPL.h.  REPL.h also use 
OptionGroupValueObjectDisplay or OptionGroupFormat.  They live in Interpreter, 
which this and other files in Expression have to use for other reasons as well 
as to get these option groups.  Their usage doesn't change your graph.  So 
another way to address your concerns would be to make an 
OptionGroupExpressionOptions in source/Interpreter and have 
CommandObjectExpression and REPL.h use that.  There's no reason other than 
convenience that the option group for expression specific options live in the 
CommandObjectExpression class.

That would be fine, and then you could leave REPL.cpp where it is.


https://reviews.llvm.org/D47232



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

Re: [Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

[Zachary Turner via lldb-commits]

On Wed, May 23, 2018 at 7:04 PM Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:

> jingham added a comment.
>
> I worry when concerns of layering which seem a little artificial to me
> would make us add a shadow class for something that is already well
> expressed as it is.  If you pass them as arguments, and then I add another
> useful one and want to use it in both the regular and the REPL versions of
> the expression parser, it would be ugly to have to add another argument to
> this function when I should have been able to share the structure.


I don’t think the concerns are artificial.  Layering violations prevent
lldb from being used as a library and pulling in only the parts you need.
In addition, they actually prevent a lot of our internal tooling from
working well so it’s a high priority for us to fix all of them.  Finally,
resolving them and achieving proper layering improves build parallelism
which benefits all developers, not to mention the build infrastructure and
bots


>
> Seems like the thing you are actually objecting to is the use of
> CommandObjectExpression::CommandOptions in REPL.h.  REPL.h also use
> OptionGroupValueObjectDisplay or OptionGroupFormat.  They live in
> Interpreter, which this and other files in Expression have to use for other
> reasons as well as to get these option groups.  Their usage doesn't change
> your graph.  So another way to address your concerns would be to make an
> OptionGroupExpressionOptions in source/Interpreter and have
> CommandObjectExpression and REPL.h use that.  There's no reason other than
> convenience that the option group for expression specific options live in
> the CommandObjectExpression class.
>
> That would be fine, and then you could leave REPL.cpp where it is.


Isn’t that quite a bit more complicated than just creating a new structure
as I’ve done?  I think the original patch is pretty simple and
straightforward.  I’d prefer a simple solution that has low impact and gets
the job done over one that tacks on a bunch of additional machinery that
YAGN.

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

[Lldb-commits] [lldb] r333151 - Remove unused include, and corresponding library dependency.

[James Y Knight via lldb-commits]

Author: jyknight
Date: Wed May 23 20:42:38 2018
New Revision: 333151

URL: http://llvm.org/viewvc/llvm-project?rev=333151&view=rev
Log:
Remove unused include, and corresponding library dependency.

Modified:
lldb/trunk/source/Symbol/CMakeLists.txt
lldb/trunk/source/Symbol/ObjectFile.cpp

Modified: lldb/trunk/source/Symbol/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/CMakeLists.txt?rev=333151&r1=333150&r2=333151&view=diff
==
--- lldb/trunk/source/Symbol/CMakeLists.txt (original)
+++ lldb/trunk/source/Symbol/CMakeLists.txt Wed May 23 20:42:38 2018
@@ -50,7 +50,6 @@ add_lldb_library(lldbSymbol
 lldbPluginExpressionParserGo
 lldbPluginSymbolFileDWARF
 lldbPluginSymbolFilePDB
-lldbPluginObjectContainerBSDArchive
 lldbPluginCPlusPlusLanguage
 lldbPluginObjCLanguage
 

Modified: lldb/trunk/source/Symbol/ObjectFile.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ObjectFile.cpp?rev=333151&r1=333150&r2=333151&view=diff
==
--- lldb/trunk/source/Symbol/ObjectFile.cpp (original)
+++ lldb/trunk/source/Symbol/ObjectFile.cpp Wed May 23 20:42:38 2018
@@ -8,7 +8,6 @@
 
//===--===//
 
 #include "lldb/Symbol/ObjectFile.h"
-#include "Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"


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