[Lldb-commits] [PATCH] D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules()

[Pavel Labath via Phabricator via lldb-commits]

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

Thanks for adding the test. looks good to me.




Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h:289-291
+  mutable std::unique_ptr m_filespec_ap;
+  typedef llvm::object::OwningBinary OWNBINType;
+  mutable std::unique_ptr m_owningbin_up;

Consider using `Optional` instead of `unique_ptr` to reduce pointer chasing.

Also, do these have to be `mutable`? I was expecting that you needed to 
lazy-initialize them from some `const` member, but I couldn't find any evidence 
of that...


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

https://reviews.llvm.org/D53094



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

[Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't see any tests :(.

Also, the three bullet points in the description sound like rather independent 
pieces of functionality. Would it be possible to split them up into separate 
patches? That would make things easier to review, particularly for those who 
don't look at this code very often :).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55142



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

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

[Pavel Labath via Phabricator via lldb-commits]

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

In D55038#1314968 , @JDevlieghere 
wrote:

> In D55038#1314336 , @labath wrote:
>
> > I think the canonical way to do that would be to define a new feature in 
> > lit, which gets set when the target supports remote debugging and then use 
> > that feature in the REQUIRES directive.
>
>
> I've changed the check to non-windows (as it wouldn't compile there anyway). 
> Is there anything not covered by this that would warrant this new feature?


Well.. FreeBSD also uses a local debugging plugin, though I don't believe 
anyone runs tests there on a regular basis. I suppose that could be handled by 
adding `system-freebsd` into the `UNSUPPORTED` clause...

Looks good to me, modulo one comment.




Comment at: source/Initialization/SystemLifetimeManager.cpp:27
 
-void SystemLifetimeManager::Initialize(
+llvm::Error SystemLifetimeManager::Initialize(
 std::unique_ptr initializer,

The calls to this function in lldb-server and lldb-test need to be updated to 
handle the newly returned Error object.


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

https://reviews.llvm.org/D55038



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

[Lldb-commits] [PATCH] D55128: [CMake] Store path to vendor-specific headers in clang-headers target property

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348116: [CMake] Store path to vendor-specific headers in 
clang-headers target property (authored by stefan.graenitz, committed by ).

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55128

Files:
  cfe/trunk/lib/Headers/CMakeLists.txt


Index: cfe/trunk/lib/Headers/CMakeLists.txt
===
--- cfe/trunk/lib/Headers/CMakeLists.txt
+++ cfe/trunk/lib/Headers/CMakeLists.txt
@@ -144,7 +144,7 @@
   list(APPEND out_files ${dst})
 endforeach( f )
 
-add_custom_command(OUTPUT ${output_dir}/arm_neon.h 
+add_custom_command(OUTPUT ${output_dir}/arm_neon.h
   DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h
   COMMAND ${CMAKE_COMMAND} -E copy_if_different 
${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h ${output_dir}/arm_neon.h
   COMMENT "Copying clang's arm_neon.h...")
@@ -156,7 +156,9 @@
 list(APPEND out_files ${output_dir}/arm_fp16.h)
 
 add_custom_target(clang-headers ALL DEPENDS ${out_files})
-set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+set_target_properties(clang-headers PROPERTIES
+  FOLDER "Misc"
+  RUNTIME_OUTPUT_DIRECTORY "${output_dir}")
 
 install(
   FILES ${files} ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h


Index: cfe/trunk/lib/Headers/CMakeLists.txt
===
--- cfe/trunk/lib/Headers/CMakeLists.txt
+++ cfe/trunk/lib/Headers/CMakeLists.txt
@@ -144,7 +144,7 @@
   list(APPEND out_files ${dst})
 endforeach( f )
 
-add_custom_command(OUTPUT ${output_dir}/arm_neon.h 
+add_custom_command(OUTPUT ${output_dir}/arm_neon.h
   DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h
   COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h ${output_dir}/arm_neon.h
   COMMENT "Copying clang's arm_neon.h...")
@@ -156,7 +156,9 @@
 list(APPEND out_files ${output_dir}/arm_fp16.h)
 
 add_custom_target(clang-headers ALL DEPENDS ${out_files})
-set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+set_target_properties(clang-headers PROPERTIES
+  FOLDER "Misc"
+  RUNTIME_OUTPUT_DIRECTORY "${output_dir}")
 
 install(
   FILES ${files} ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55114: [CMake] Add LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR for custom dSYM target directory on Darwin

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348118: [CMake] Add LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR 
for custom dSYM target… (authored by stefan.graenitz, committed by ).

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55114

Files:
  llvm/trunk/cmake/modules/AddLLVM.cmake


Index: llvm/trunk/cmake/modules/AddLLVM.cmake
===
--- llvm/trunk/cmake/modules/AddLLVM.cmake
+++ llvm/trunk/cmake/modules/AddLLVM.cmake
@@ -1597,6 +1597,13 @@
 endif()
   endif()
 
+  if(LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR)
+if(APPLE)
+  set(output_name "$.dSYM")
+  set(output_path 
"-o=${LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR}/${output_name}")
+endif()
+  endif()
+
   if(APPLE)
 if(CMAKE_CXX_FLAGS MATCHES "-flto"
   OR CMAKE_CXX_FLAGS_${uppercase_CMAKE_BUILD_TYPE} MATCHES "-flto")
@@ -1609,7 +1616,7 @@
   set(CMAKE_DSYMUTIL xcrun dsymutil)
 endif()
 add_custom_command(TARGET ${name} POST_BUILD
-  COMMAND ${CMAKE_DSYMUTIL} $
+  COMMAND ${CMAKE_DSYMUTIL} ${output_path} $
   ${strip_command}
   )
   else()


Index: llvm/trunk/cmake/modules/AddLLVM.cmake
===
--- llvm/trunk/cmake/modules/AddLLVM.cmake
+++ llvm/trunk/cmake/modules/AddLLVM.cmake
@@ -1597,6 +1597,13 @@
 endif()
   endif()
 
+  if(LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR)
+if(APPLE)
+  set(output_name "$.dSYM")
+  set(output_path "-o=${LLVM_EXTERNALIZE_DEBUGINFO_OUTPUT_DIR}/${output_name}")
+endif()
+  endif()
+
   if(APPLE)
 if(CMAKE_CXX_FLAGS MATCHES "-flto"
   OR CMAKE_CXX_FLAGS_${uppercase_CMAKE_BUILD_TYPE} MATCHES "-flto")
@@ -1609,7 +1616,7 @@
   set(CMAKE_DSYMUTIL xcrun dsymutil)
 endif()
 add_custom_command(TARGET ${name} POST_BUILD
-  COMMAND ${CMAKE_DSYMUTIL} $
+  COMMAND ${CMAKE_DSYMUTIL} ${output_path} $
   ${strip_command}
   )
   else()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55214: Introduce ObjectFileBreakpad

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, zturner, lemo, amccarth.
Herald added subscribers: fedor.sergeev, mgorny.

This patch adds the scaffolding necessary for lldb to recognise symbol
files generated by breakpad. These (textual) files contain just enough
information to be able to produce a backtrace from a crash
dump. This information includes:

- UUID, architecture and name of the module
- line tables
- list of symbols
- unwind information

A minimal breakpad file could look like this:
MODULE Linux x86_64 24B5D199F0F766FF5DC30 a.out
INFO CODE_ID B52499D1F0F766FF5DC3
FILE 0 /tmp/a.c
FUNC 1010 10 0 _start
1010 4 4 0
1014 5 5 0
1019 5 6 0
101e 2 7 0
PUBLIC 1010 0 _start
STACK CFI INIT 1010 10 .cfa: $rsp 8 + .ra: .cfa -8 + ^
STACK CFI 1011 $rbp: .cfa -16 + ^ .cfa: $rsp 16 +
STACK CFI 1014 .cfa: $rbp 16 +

Even though this data would normally be considered "symbol" information,
in the current lldb infrastructure it is assumed every SymbolFile object
is backed by an ObjectFile instance. So, in order to better interoperate
with the rest of the code (particularly symbol vendors).

In this patch I just parse the breakpad header, which is enough to
populate the UUID and architecture fields of the ObjectFile interface.
The rough plan for followup patches is to expose the individual parts of
the breakpad file as ObjectFile "sections", which can then be used by
other parts of the codebase (SymbolFileBreakpad ?) to vend the necessary
information.


https://reviews.llvm.org/D55214

Files:
  include/lldb/Symbol/ObjectFile.h
  lit/Modules/Breakpad/Inputs/identification-linux.syms
  lit/Modules/Breakpad/Inputs/identification-macosx.syms
  lit/Modules/Breakpad/Inputs/identification-windows.syms
  lit/Modules/Breakpad/breakpad-identification.test
  lit/Modules/Breakpad/lit.local.cfg
  source/Plugins/ObjectFile/Breakpad/CMakeLists.txt
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  source/Plugins/ObjectFile/CMakeLists.txt
  source/Symbol/ObjectFile.cpp
  tools/lldb-test/SystemInitializerTest.cpp
  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
@@ -734,9 +734,16 @@
   continue;
 }
 
+auto *ObjectPtr = ModulePtr->GetObjectFile();
+
+Printer.formatLine("Plugin name: {0}", ObjectPtr->GetPluginName());
 Printer.formatLine("Architecture: {0}",
ModulePtr->GetArchitecture().GetTriple().getTriple());
 Printer.formatLine("UUID: {0}", ModulePtr->GetUUID().GetAsString());
+Printer.formatLine("Executable: {0}", ObjectPtr->IsExecutable());
+Printer.formatLine("Stripped: {0}", ObjectPtr->IsStripped());
+Printer.formatLine("Type: {0}", ObjectPtr->GetType());
+Printer.formatLine("Strata: {0}", ObjectPtr->GetStrata());
 
 size_t Count = Sections->GetNumSections(0);
 Printer.formatLine("Showing {0} sections", Count);
Index: tools/lldb-test/SystemInitializerTest.cpp
===
--- tools/lldb-test/SystemInitializerTest.cpp
+++ tools/lldb-test/SystemInitializerTest.cpp
@@ -52,6 +52,7 @@
 #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h"
 #include "Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h"
 #include "Plugins/MemoryHistory/asan/MemoryHistoryASan.h"
+#include "Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h"
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
@@ -114,6 +115,7 @@
 void SystemInitializerTest::Initialize() {
   SystemInitializerCommon::Initialize();
 
+  breakpad::ObjectFileBreakpad::Initialize();
   ObjectFileELF::Initialize();
   ObjectFileMachO::Initialize();
   ObjectFilePECOFF::Initialize();
@@ -327,6 +329,7 @@
   PlatformDarwinKernel::Terminate();
 #endif
 
+  breakpad::ObjectFileBreakpad::Terminate();
   ObjectFileELF::Terminate();
   ObjectFileMachO::Terminate();
   ObjectFilePECOFF::Terminate();
Index: source/Symbol/ObjectFile.cpp
===
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -688,3 +688,63 @@
  uint64_t Offset) {
   return FileSystem::Instance().CreateDataBuffer(file.GetPath(), Size, Offset);
 }
+
+void llvm::format_provider::format(
+const ObjectFile::Type &type, raw_ostream &OS, StringRef Style) {
+  switch (type) {
+  case ObjectFile::eTypeInvalid:
+OS << "invalid";
+break;
+  case ObjectFile::eTypeCoreFile:
+OS << "core file";
+break;
+  case ObjectFile::eTypeExecutable:
+OS << "executable";
+break;
+  case ObjectFile::eTypeDebugInfo:
+OS << "debug info";
+break;
+  case ObjectFile::eTyp

[Lldb-commits] [lldb] r348136 - [PDB] Support PDB-backed expressions evaluation (+ fix stuck test)

[Aleksandr Urakov via lldb-commits]

Author: aleksandr.urakov
Date: Mon Dec  3 05:31:13 2018
New Revision: 348136

URL: http://llvm.org/viewvc/llvm-project?rev=348136&view=rev
Log:
[PDB] Support PDB-backed expressions evaluation (+ fix stuck test)

Summary:
This patch contains several small fixes, which makes it possible to evaluate
expressions on Windows using information from PDB. The changes are:
- several sanitize checks;
- make IRExecutionUnit::MemoryManager::getSymbolAddress to not return a magic
  value on a failure, because callers wait 0 in this case;
- entry point required to be a file address, not RVA, in the ObjectFilePECOFF;
- do not crash on a debuggee second chance exception - it may be an expression
  evaluation crash. Also fix detection of "crushed" threads in tests;
- create parameter declarations for functions in AST to make it possible to call
  debugee functions from expressions;
- relax name searching rules for variables, functions, namespaces and types. Now
  it works just like in the DWARF plugin;
- fix endless recursion in SymbolFilePDB::ParseCompileUnitFunctionForPDBFunc.

Reviewers: zturner, asmith, stella.stamenova

Reviewed By: stella.stamenova, asmith

Tags: #lldb

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

Added:
lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest.cpp
lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest0.script
lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest1.script
lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest2.script
lldb/trunk/lit/SymbolFile/PDB/expressions.test
Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py
lldb/trunk/source/Expression/IRExecutionUnit.cpp
lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.h
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Added: lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest.cpp?rev=348136&view=auto
==
--- lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest.cpp (added)
+++ lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest.cpp Mon Dec  3 
05:31:13 2018
@@ -0,0 +1,20 @@
+namespace N0 {
+namespace N1 {
+
+char *buf0 = nullptr;
+char buf1[] = {0, 1, 2, 3, 4, 5, 6, 7};
+
+char sum(char *buf, int size) {
+  char result = 0;
+  for (int i = 0; i < size; i++)
+result += buf[i];
+  return result;
+}
+
+} // namespace N1
+} // namespace N0
+
+int main() {
+  char result = N0::N1::sum(N0::N1::buf1, sizeof(N0::N1::buf1));
+  return 0;
+}

Added: lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest0.script
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest0.script?rev=348136&view=auto
==
--- lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest0.script (added)
+++ lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest0.script Mon Dec  3 
05:31:13 2018
@@ -0,0 +1,7 @@
+breakpoint set --file ExpressionsTest.cpp --line 19
+run
+print result
+print N0::N1::sum(N0::N1::buf1, sizeof(N0::N1::buf1))
+print N1::sum(N1::buf1, sizeof(N1::buf1))
+print sum(buf1, sizeof(buf1))
+print sum(buf1, 10)

Added: lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest1.script
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest1.script?rev=348136&view=auto
==
--- lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest1.script (added)
+++ lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest1.script Mon Dec  3 
05:31:13 2018
@@ -0,0 +1 @@
+print sum(buf0, 1)

Added: lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest2.script
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest2.script?rev=348136&view=auto
==
--- lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest2.script (added)
+++ lldb/trunk/lit/SymbolFile/PDB/Inputs/ExpressionsTest2.script Mon Dec  3 
05:31:13 2018
@@ -0,0 +1,2 @@
+print sum(buf0, result - 28)
+print sum(buf1 + 3, 3)

Added: lldb/trunk/lit/SymbolFile/PDB/expressions.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/expressions.test?rev=348136&view=auto
==
--- lldb/trunk/lit/SymbolFile/PDB/expressions.test (added)
+++ lldb/trunk/lit/SymbolFile/PDB/expressions.test Mon Dec  3 05:31:13 2018
@@ -0,0 +1,36 @@
+REQUIRES: system-windows, msvc
+RUN: %msvc_cl /Zi /GS- /c %S/Inputs/ExpressionsTest.cpp /Fo%t.obj
+RUN: %msvc_link /debug:full /nodefaultlib /en

[Lldb-commits] [PATCH] D55214: Introduce ObjectFileBreakpad

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp:66-84
+  if (os == llvm::Triple::Win32) {
+// In binary form, the module id should have 20 bytes: 16 bytes for UUID,
+// and 4 bytes for the "age". However, in the textual format, the 4 bytes 
of
+// age are printed via %x, which can lead to shorter strings. So, we pad 
the
+// string with zeroes after the 16 bytes, to obtain a string of appropriate
+// size.
+if (token.size() < 33 || token.size() > 40)

@lemo: Does this part make sense? It seems that on linux the breakpad files 
have the `INFO CODE_ID` section, which contains the UUID without the funny 
trailing zero. So I could try fetching the UUID from there instead, but only on 
linux, as that section is not present mac (and on windows it contains something 
completely different). Right now I compute the UUID on linux by chopping off 
the trailing zero (as I have to do that anyway for mac), but I could do 
something different is there's any advantage to that.


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

https://reviews.llvm.org/D55214



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

[Lldb-commits] [PATCH] D54843: [Expr] Check the language before ignoring Objective C keywords

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov marked an inline comment as done.
aleksandr.urakov added a comment.

Ping! Can you look at this, please?


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

https://reviews.llvm.org/D54843



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

Re: [Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file

[Pavel Labath via lldb-commits]

On 29/11/2018 23:34, Greg Clayton wrote:




On Nov 29, 2018, at 10:55 AM, Pavel Labath via Phabricator 
 wrote:

labath added a comment.

I've recently started looking at adding a new symbol file format (breakpad 
symbols). While researching the best way to achieve that, I started comparing 
the operation of PDB and DWARF symbol files. I noticed a very important 
difference there, and I think that is the cause of our problems here. In the 
DWARF implementation, a symbol file is an overlay on top of an object file - it 
takes the data contained by the object file and presents it in a more 
structured way.

However, that is not the case with PDB (both implementations). These take the 
debug information from a completely different file, which is not backed by an 
ObjectFile instance, and then present that. Since the SymbolFile interface 
requires them to be backed by an object file, they both pretend they are backed 
by the original EXE file, but in reality the data comes from elsewhere.

If we had an ObjectFilePDB (which not also not ideal, though in a way it is a 
better fit to the current lldb organization), then this could expose the PDB 
symtab via the existing ObjectFile interface and we could reuse the existing 
mechanism for merging symtabs from two object files.

I am asking this because now I am facing a choice in how to implement breakpad 
symbols. I could go the PDB way, and read the symbols without an intervening 
object file, or I could create an ObjectFileBreakpad and then (possibly) a 
SymbolFileBreakpad sitting on top of that.

The drawbacks of the PDB approach I see are:

- I lose the ability to do matching of the (real) object file via symbol 
vendors. The PDB symbol files now basically implement their own little symbol 
vendors inside them, which is mostly fine if you just need to find the PDB next 
to the exe file. However, things could get a bit messy if you wanted to 
implement some more complex searching on multiple paths, or downloading them 
from the internet.
- I'll hit issues when attempting to unwind (which is the real meat of the 
breakpad symbols), because unwind info is currently provided via the ObjectFile 
interface (ObjectFile::GetUnwindTable).

The drawbacks of the ObjectFile approach are:

- more code  - it needs a new ObjectFile and a new SymbolFile class (possibly 
also a SymbolVendor)
- it will probably look a bit weird because Breakpad files (and PDBs) aren't 
really object files

I'd like to hear your thoughts on this, if you have any.


Since the Breakpad files contain symbol and debug info, I would make a 
ObjectFileBreakpad and SymbolFileBreakpad route. We might want to just load a 
break pad file and be able to symbolicate with it. Each symbol vendor will need 
to be able to use a breakpad file (since Breakpad produces files for all 
architectures), so we can build some functionality into the SymbolVendor base 
class for file formats that any file format (ELF, Mach-o, COFF) might want to 
use.





Just to close this, I've decided to try going the ObjectFile route (in 
D55214). It shouldn't take me long to reach the point where I need to 
vend symtab and unwind information to the rest of lldb, at which point 
we can decide whether that was the right move.


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

[Lldb-commits] [PATCH] D54843: [Expr] Check the language before ignoring Objective C keywords

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks fine to me. Jim Ingham should ok this as well.


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

https://reviews.llvm.org/D54843



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

[Lldb-commits] [PATCH] D55214: Introduce ObjectFileBreakpad

[Greg Clayton via Phabricator via lldb-commits]

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

This looks like a good start.


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

https://reviews.llvm.org/D55214



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

[Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

[Leonid Mashinskiy via Phabricator via lldb-commits]

leonid.mashinskiy updated this revision to Diff 176411.
leonid.mashinskiy added a comment.

Added unit tests for translator and fix issues mentioned by review


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55122

Files:
  lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp
  lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script
  lit/SymbolFile/PDB/variables-locations.test
  source/Expression/DWARFExpression.cpp
  source/Plugins/SymbolFile/PDB/CMakeLists.txt
  source/Plugins/SymbolFile/PDB/CodeViewRegisterMapping.cpp
  source/Plugins/SymbolFile/PDB/CodeViewRegisterMapping.h
  source/Plugins/SymbolFile/PDB/PDBFPOProgramToDWARFExpression.cpp
  source/Plugins/SymbolFile/PDB/PDBFPOProgramToDWARFExpression.h
  source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  unittests/SymbolFile/PDB/CMakeLists.txt
  unittests/SymbolFile/PDB/PDBFPOProgramToDWARFExpressionTests.cpp

Index: unittests/SymbolFile/PDB/PDBFPOProgramToDWARFExpressionTests.cpp
===
--- /dev/null
+++ unittests/SymbolFile/PDB/PDBFPOProgramToDWARFExpressionTests.cpp
@@ -0,0 +1,154 @@
+//===-- PDBFPOProgramToDWARFExpressionTests.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/SymbolFile/PDB/PDBFPOProgramToDWARFExpression.h"
+
+#include "lldb/Core/StreamBuffer.h"
+#include "lldb/Expression/DWARFExpression.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/StreamString.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+/// Valid programs tests
+
+static void
+CheckValidProgramTranslation(llvm::StringRef fpo_program,
+ llvm::StringRef target_register_name,
+ llvm::StringRef expected_dwarf_expression) {
+  // initial setup
+  ArchSpec arch_spec("i686-pc-windows");
+  llvm::Triple::ArchType arch_type = arch_spec.GetMachine();
+  ByteOrder byte_order = arch_spec.GetByteOrder();
+  uint32_t address_size = arch_spec.GetAddressByteSize();
+  uint32_t byte_size = arch_spec.GetDataByteSize();
+
+  // program translation
+  StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order);
+  ASSERT_TRUE(TranslateFPOProgramToDWARFExpression(
+  fpo_program, target_register_name, arch_type, stream));
+
+  // print dwarf expression to comparable textual representation
+  DataBufferSP buffer =
+  std::make_shared(stream.GetData(), stream.GetSize());
+  DataExtractor extractor(buffer, byte_order, address_size, byte_size);
+
+  StreamString result_dwarf_expression;
+  ASSERT_TRUE(DWARFExpression::PrintDWARFExpression(
+  result_dwarf_expression, extractor, address_size, 4, false));
+
+  // actual check
+  ASSERT_EQ(expected_dwarf_expression, result_dwarf_expression.GetString());
+}
+
+TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentConst) {
+  CheckValidProgramTranslation("$T0 0 = ", "$T0", "DW_OP_constu 0x0");
+}
+
+TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentRegisterRef) {
+  CheckValidProgramTranslation("$T0 $ebp = ", "$T0", "DW_OP_breg6 +0");
+}
+
+TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentExpressionPlus) {
+  CheckValidProgramTranslation("$T0 $ebp 4 + = ", "$T0",
+   "DW_OP_breg6 +0, DW_OP_constu 0x4, DW_OP_plus ");
+}
+
+TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentExpressionDeref) {
+  CheckValidProgramTranslation("$T0 $ebp ^ = ", "$T0",
+   "DW_OP_breg6 +0, DW_OP_deref ");
+}
+
+TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentExpressionMinus) {
+  CheckValidProgramTranslation(
+  "$T0 $ebp 4 - = ", "$T0",
+  "DW_OP_breg6 +0, DW_OP_constu 0x4, DW_OP_minus ");
+}
+
+TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentExpressionAlign) {
+  CheckValidProgramTranslation("$T0 $ebp 128 @ = ", "$T0",
+   "DW_OP_breg6 +0, DW_OP_constu 0x80, DW_OP_lit1 "
+   ", DW_OP_minus , DW_OP_not , DW_OP_and ");
+}
+
+TEST(PDBFPOProgramToDWARFExpressionTests, MultipleIndependentAssignments) {
+  CheckValidProgramTranslation("$T1 1 = $T0 0 =", "$T0", "DW_OP_constu 0x0");
+}
+
+TEST(PDBFPOProgramToDWARFExpressionTests, MultipleDependentAssignments) {
+  CheckValidProgramTranslation(
+  "$T1 $ebp 4 + = $T0 $T1 8 - 128 @ = ", "$T0",
+  "DW_OP_breg6 +0, DW_OP_constu 0x4, DW_OP_plus , DW_OP_constu 0x8, "
+  "DW_OP_minus , DW_OP_constu 0x80, DW_OP_lit1 , DW_OP_min

[Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

[Leonid Mashinskiy via Phabricator via lldb-commits]

leonid.mashinskiy added a comment.

Seems like we can't move to NativePDB plugin right now, because there are still 
not implemented some methods like `ParseVariablesForContext` which this code 
integration based on.
But when it will - we can easily do the transition.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55122



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

Re: [Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

[Zachary Turner via lldb-commits]

In that case, can we implement that method first? I was planning to do it
this week, but if someone else wants to do it immediately that’s fine too
On Mon, Dec 3, 2018 at 8:11 AM Leonid Mashinskiy via Phabricator <
revi...@reviews.llvm.org> wrote:

> leonid.mashinskiy added a comment.
>
> Seems like we can't move to NativePDB plugin right now, because there are
> still not implemented some methods like `ParseVariablesForContext` which
> this code integration based on.
> But when it will - we can easily do the transition.
>
>
> Repository:
>   rLLDB LLDB
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55122/new/
>
> https://reviews.llvm.org/D55122
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

I can do it, but unfortunately not this week... I want to join the native 
plugin development some later, at the end of this month, after some current 
work.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55122



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

Re: [Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

[Zachary Turner via lldb-commits]

Is the only reason we need these methods so that we can use lldb-test? If
so, perhaps the tests can be written in terms of lldb itself, similar to
the existing NativePDB tests
On Mon, Dec 3, 2018 at 8:39 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:

> aleksandr.urakov added a comment.
>
> I can do it, but unfortunately not this week... I want to join the native
> plugin development some later, at the end of this month, after some current
> work.
>
>
> Repository:
>   rLLDB LLDB
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55122/new/
>
> https://reviews.llvm.org/D55122
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

[Aleksandr Urakov via lldb-commits]

If I understand correctly, the native plugin processes for now globals and
constants, but it can't create locals, right? But this FPO conversion is
applicable to locals only, so we need to implement locals creation in the
new plugin first, before moving the conversion there. That's why we need
these functions.

Am Mo., 3. Dez. 2018, 19:41 hat Zachary Turner 
geschrieben:

> Is the only reason we need these methods so that we can use lldb-test? If
> so, perhaps the tests can be written in terms of lldb itself, similar to
> the existing NativePDB tests
> On Mon, Dec 3, 2018 at 8:39 AM Aleksandr Urakov via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> aleksandr.urakov added a comment.
>>
>> I can do it, but unfortunately not this week... I want to join the native
>> plugin development some later, at the end of this month, after some current
>> work.
>>
>>
>> Repository:
>>   rLLDB LLDB
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D55122/new/
>>
>> https://reviews.llvm.org/D55122
>>
>>
>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

[Zachary Turner via lldb-commits]

Ok that makes sense. I’ll work on this soon, I’m trying to get all existing
dia PDB tests passing with the native plugin so i can delete old plugin, so
I’ll keep this in mind and probably do this next.

I’m trying to fix some bugs in clang so that we emit compatible debug info
to msvc, which currently blocks conversion of a few tests. So after that
I’ll work on this
On Mon, Dec 3, 2018 at 9:02 AM Aleksandr Urakov <
aleksandr.ura...@jetbrains.com> wrote:

> If I understand correctly, the native plugin processes for now globals and
> constants, but it can't create locals, right? But this FPO conversion is
> applicable to locals only, so we need to implement locals creation in the
> new plugin first, before moving the conversion there. That's why we need
> these functions.
>
> Am Mo., 3. Dez. 2018, 19:41 hat Zachary Turner 
> geschrieben:
>
>> Is the only reason we need these methods so that we can use lldb-test? If
>> so, perhaps the tests can be written in terms of lldb itself, similar to
>> the existing NativePDB tests
>> On Mon, Dec 3, 2018 at 8:39 AM Aleksandr Urakov via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>>> aleksandr.urakov added a comment.
>>>
>>> I can do it, but unfortunately not this week... I want to join the
>>> native plugin development some later, at the end of this month, after some
>>> current work.
>>>
>>>
>>> Repository:
>>>   rLLDB LLDB
>>>
>>> CHANGES SINCE LAST ACTION
>>>   https://reviews.llvm.org/D55122/new/
>>>
>>> https://reviews.llvm.org/D55122
>>>
>>>
>>>
>>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

[Aleksandr Urakov via lldb-commits]

Ok, thank you! I'll contact you when I will be ready to work on the new
plugin.

Am Mo., 3. Dez. 2018, 20:07 hat Zachary Turner 
geschrieben:

> Ok that makes sense. I’ll work on this soon, I’m trying to get all
> existing dia PDB tests passing with the native plugin so i can delete old
> plugin, so I’ll keep this in mind and probably do this next.
>
> I’m trying to fix some bugs in clang so that we emit compatible debug info
> to msvc, which currently blocks conversion of a few tests. So after that
> I’ll work on this
> On Mon, Dec 3, 2018 at 9:02 AM Aleksandr Urakov <
> aleksandr.ura...@jetbrains.com> wrote:
>
>> If I understand correctly, the native plugin processes for now globals
>> and constants, but it can't create locals, right? But this FPO conversion
>> is applicable to locals only, so we need to implement locals creation in
>> the new plugin first, before moving the conversion there. That's why we
>> need these functions.
>>
>> Am Mo., 3. Dez. 2018, 19:41 hat Zachary Turner 
>> geschrieben:
>>
>>> Is the only reason we need these methods so that we can use lldb-test?
>>> If so, perhaps the tests can be written in terms of lldb itself, similar to
>>> the existing NativePDB tests
>>> On Mon, Dec 3, 2018 at 8:39 AM Aleksandr Urakov via Phabricator <
>>> revi...@reviews.llvm.org> wrote:
>>>
 aleksandr.urakov added a comment.

 I can do it, but unfortunately not this week... I want to join the
 native plugin development some later, at the end of this month, after some
 current work.


 Repository:
   rLLDB LLDB

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

 https://reviews.llvm.org/D55122




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

[Lldb-commits] [lldb] r348152 - [Reproducers] Change how reproducers are initialized.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Dec  3 09:28:29 2018
New Revision: 348152

URL: http://llvm.org/viewvc/llvm-project?rev=348152&view=rev
Log:
[Reproducers] Change how reproducers are initialized.

This patch changes the way the reproducer is initialized. Rather than
making changes at run time we now do everything at initialization time.
To make this happen we had to introduce initializer options and their SB
variant. This allows us to tell the initializer that we're running in
reproducer capture/replay mode.

Because of this change we also had to alter our testing strategy. We
cannot reinitialize LLDB when using the dotest infrastructure. Instead
we use lit and invoke two instances of the driver.

Another consequence is that we can no longer enable capture or replay
through commands. This was bound to go away form the beginning, but I
had something in mind where you could enable/disable specific providers.
However this seems like it adds very little value right now so the
corresponding commands were removed.

Finally this change also means you now have to control this through the
driver, for which I replaced --reproducer with --capture and --replay to
differentiate between the two modes.

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

Added:
lldb/trunk/include/lldb/API/SBInitializerOptions.h
lldb/trunk/lit/Reproducer/
lldb/trunk/lit/Reproducer/Inputs/
lldb/trunk/lit/Reproducer/Inputs/GDBRemoteCapture.in
lldb/trunk/lit/Reproducer/Inputs/GDBRemoteReplay.in
lldb/trunk/lit/Reproducer/Inputs/simple.c
lldb/trunk/lit/Reproducer/TestDriverOptions.test
lldb/trunk/lit/Reproducer/TestGDBRemoteRepro.test
lldb/trunk/scripts/interface/SBInitializerOptions.i
lldb/trunk/source/API/SBInitializerOptions.cpp
Removed:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/reproducer/gdb-remote/Makefile

lldb/trunk/packages/Python/lldbsuite/test/functionalities/reproducer/gdb-remote/TestGdbRemoteReproducer.py
Modified:
lldb/trunk/include/lldb/API/SBDebugger.h
lldb/trunk/include/lldb/API/SBDefines.h
lldb/trunk/include/lldb/API/SBFileSpec.h
lldb/trunk/include/lldb/Core/Debugger.h
lldb/trunk/include/lldb/Host/HostInfoBase.h
lldb/trunk/include/lldb/Initialization/SystemInitializer.h
lldb/trunk/include/lldb/Initialization/SystemInitializerCommon.h
lldb/trunk/include/lldb/Initialization/SystemLifetimeManager.h
lldb/trunk/include/lldb/Utility/Reproducer.h
lldb/trunk/scripts/interface/SBDebugger.i
lldb/trunk/scripts/lldb.swig
lldb/trunk/source/API/CMakeLists.txt
lldb/trunk/source/API/SBDebugger.cpp
lldb/trunk/source/API/SystemInitializerFull.cpp
lldb/trunk/source/API/SystemInitializerFull.h
lldb/trunk/source/Commands/CommandObjectReproducer.cpp
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Host/common/HostInfoBase.cpp
lldb/trunk/source/Initialization/SystemInitializerCommon.cpp
lldb/trunk/source/Initialization/SystemLifetimeManager.cpp
lldb/trunk/source/Utility/Reproducer.cpp
lldb/trunk/tools/driver/Driver.cpp
lldb/trunk/tools/driver/Options.td
lldb/trunk/tools/lldb-server/SystemInitializerLLGS.cpp
lldb/trunk/tools/lldb-server/SystemInitializerLLGS.h
lldb/trunk/tools/lldb-server/lldb-server.cpp
lldb/trunk/tools/lldb-test/SystemInitializerTest.cpp
lldb/trunk/tools/lldb-test/SystemInitializerTest.h
lldb/trunk/tools/lldb-test/lldb-test.cpp
lldb/trunk/unittests/Utility/ReproducerTest.cpp

Modified: lldb/trunk/include/lldb/API/SBDebugger.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBDebugger.h?rev=348152&r1=348151&r2=348152&view=diff
==
--- lldb/trunk/include/lldb/API/SBDebugger.h (original)
+++ lldb/trunk/include/lldb/API/SBDebugger.h Mon Dec  3 09:28:29 2018
@@ -13,6 +13,7 @@
 #include 
 
 #include "lldb/API/SBDefines.h"
+#include "lldb/API/SBInitializerOptions.h"
 #include "lldb/API/SBPlatform.h"
 
 namespace lldb {
@@ -45,6 +46,7 @@ public:
   lldb::SBDebugger &operator=(const lldb::SBDebugger &rhs);
 
   static void Initialize();
+  static lldb::SBError Initialize(SBInitializerOptions &options);
 
   static void Terminate();
 
@@ -228,8 +230,6 @@ public:
 
   const char *GetReproducerPath() const;
 
-  lldb::SBError ReplayReproducer(const char *path);
-
   lldb::ScriptLanguage GetScriptLanguage() const;
 
   void SetScriptLanguage(lldb::ScriptLanguage script_lang);

Modified: lldb/trunk/include/lldb/API/SBDefines.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBDefines.h?rev=348152&r1=348151&r2=348152&view=diff
==
--- lldb/trunk/include/lldb/API/SBDefines.h (original)
+++ lldb/trunk/include/lldb/API/SBDefines.h Mon Dec  3 09:28:29 2018
@@ -51,6 +51,7 @@ class LLDB_API SBFileSpecList;
 class LLDB_API SBFrame;
 class LLDB_API SBFunction;
 class LLDB_AP

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348152: [Reproducers] Change how reproducers are 
initialized. (authored by JDevlieghere, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55038?vs=176161&id=176418#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55038

Files:
  lldb/trunk/include/lldb/API/SBDebugger.h
  lldb/trunk/include/lldb/API/SBDefines.h
  lldb/trunk/include/lldb/API/SBFileSpec.h
  lldb/trunk/include/lldb/API/SBInitializerOptions.h
  lldb/trunk/include/lldb/Core/Debugger.h
  lldb/trunk/include/lldb/Host/HostInfoBase.h
  lldb/trunk/include/lldb/Initialization/SystemInitializer.h
  lldb/trunk/include/lldb/Initialization/SystemInitializerCommon.h
  lldb/trunk/include/lldb/Initialization/SystemLifetimeManager.h
  lldb/trunk/include/lldb/Utility/Reproducer.h
  lldb/trunk/lit/Reproducer/Inputs/GDBRemoteCapture.in
  lldb/trunk/lit/Reproducer/Inputs/GDBRemoteReplay.in
  lldb/trunk/lit/Reproducer/Inputs/simple.c
  lldb/trunk/lit/Reproducer/TestDriverOptions.test
  lldb/trunk/lit/Reproducer/TestGDBRemoteRepro.test
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/reproducer/gdb-remote/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/reproducer/gdb-remote/TestGdbRemoteReproducer.py
  lldb/trunk/scripts/interface/SBDebugger.i
  lldb/trunk/scripts/interface/SBInitializerOptions.i
  lldb/trunk/scripts/lldb.swig
  lldb/trunk/source/API/CMakeLists.txt
  lldb/trunk/source/API/SBDebugger.cpp
  lldb/trunk/source/API/SBInitializerOptions.cpp
  lldb/trunk/source/API/SystemInitializerFull.cpp
  lldb/trunk/source/API/SystemInitializerFull.h
  lldb/trunk/source/Commands/CommandObjectReproducer.cpp
  lldb/trunk/source/Core/Debugger.cpp
  lldb/trunk/source/Host/common/HostInfoBase.cpp
  lldb/trunk/source/Initialization/SystemInitializerCommon.cpp
  lldb/trunk/source/Initialization/SystemLifetimeManager.cpp
  lldb/trunk/source/Utility/Reproducer.cpp
  lldb/trunk/tools/driver/Driver.cpp
  lldb/trunk/tools/driver/Options.td
  lldb/trunk/tools/lldb-server/SystemInitializerLLGS.cpp
  lldb/trunk/tools/lldb-server/SystemInitializerLLGS.h
  lldb/trunk/tools/lldb-server/lldb-server.cpp
  lldb/trunk/tools/lldb-test/SystemInitializerTest.cpp
  lldb/trunk/tools/lldb-test/SystemInitializerTest.h
  lldb/trunk/tools/lldb-test/lldb-test.cpp
  lldb/trunk/unittests/Utility/ReproducerTest.cpp

Index: lldb/trunk/unittests/Utility/ReproducerTest.cpp
===
--- lldb/trunk/unittests/Utility/ReproducerTest.cpp
+++ lldb/trunk/unittests/Utility/ReproducerTest.cpp
@@ -32,10 +32,18 @@
   static char ID;
 };
 
+class DummyReproducer : public Reproducer {
+public:
+  DummyReproducer() : Reproducer(){};
+
+  using Reproducer::SetCapture;
+  using Reproducer::SetReplay;
+};
+
 char DummyProvider::ID = 0;
 
 TEST(ReproducerTest, SetCapture) {
-  Reproducer reproducer;
+  DummyReproducer reproducer;
 
   // Initially both generator and loader are unset.
   EXPECT_EQ(nullptr, reproducer.GetGenerator());
@@ -59,7 +67,7 @@
 }
 
 TEST(ReproducerTest, SetReplay) {
-  Reproducer reproducer;
+  DummyReproducer reproducer;
 
   // Initially both generator and loader are unset.
   EXPECT_EQ(nullptr, reproducer.GetGenerator());
@@ -80,7 +88,7 @@
 }
 
 TEST(GeneratorTest, Create) {
-  Reproducer reproducer;
+  DummyReproducer reproducer;
 
   EXPECT_THAT_ERROR(reproducer.SetCapture(FileSpec("/bogus/path")),
 Succeeded());
@@ -95,7 +103,7 @@
 }
 
 TEST(GeneratorTest, Get) {
-  Reproducer reproducer;
+  DummyReproducer reproducer;
 
   EXPECT_THAT_ERROR(reproducer.SetCapture(FileSpec("/bogus/path")),
 Succeeded());
@@ -109,7 +117,7 @@
 }
 
 TEST(GeneratorTest, GetOrCreate) {
-  Reproducer reproducer;
+  DummyReproducer reproducer;
 
   EXPECT_THAT_ERROR(reproducer.SetCapture(FileSpec("/bogus/path")),
 Succeeded());
Index: lldb/trunk/tools/lldb-server/lldb-server.cpp
===
--- lldb/trunk/tools/lldb-server/lldb-server.cpp
+++ lldb/trunk/tools/lldb-server/lldb-server.cpp
@@ -38,8 +38,9 @@
 int main_platform(int argc, char *argv[]);
 
 static void initialize() {
-  g_debugger_lifetime->Initialize(llvm::make_unique(),
-  nullptr);
+  if (auto e = g_debugger_lifetime->Initialize(
+  llvm::make_unique(), {}, nullptr))
+llvm::consumeError(std::move(e));
 }
 
 static void terminate() { g_debugger_lifetime->Terminate(); }
Index: lldb/trunk/tools/lldb-server/SystemInitializerLLGS.h
===
--- lldb/trunk/tools/lldb-server/SystemInitializerLLGS.h
+++ lldb/trunk/tools/lldb-server/SystemInitializerLLGS.h
@@ -10,11 +10,13 @@
 #ifndef LLDB_SYSTEMINITIALIZERLLGS_H
 #define LLDB_SYSTEMIN

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

One of these tests fail on Windows:


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55038



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

[Lldb-commits] [PATCH] D55230: [lit] Multiple build outputs and default target bitness

[Zachary Turner via Phabricator via lldb-commits]

zturner created this revision.
zturner added reviewers: labath, stella.stamenova, aleksandr.urakov.
Herald added a subscriber: mgorny.

This patch introduces two improvements to the `build.py` script.

First, it supports multiple build inputs and outputs.  This is helpful if you 
want to run with `mode=compile` but you want to compile many source files at 
once.  Now, you can specify an output directory and it will write all of them 
to that location.

Second, it changes the default architecture of test executables to be whatever 
the system is.

Both of these will help with porting DIA PDB tests over to the new build 
script, but are also generally useful features that most tests will probably 
want anyway.


https://reviews.llvm.org/D55230

Files:
  lldb/lit/BuildScript/modes.test
  lldb/lit/BuildScript/script-args.test
  lldb/lit/BuildScript/toolchain-clang-cl.test
  lldb/lit/BuildScript/toolchain-msvc.test
  lldb/lit/CMakeLists.txt
  lldb/lit/helper/build.py
  lldb/lit/helper/toolchain.py
  lldb/lit/lit.site.cfg.py.in

Index: lldb/lit/lit.site.cfg.py.in
===
--- lldb/lit/lit.site.cfg.py.in
+++ lldb/lit/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
+config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: lldb/lit/helper/toolchain.py
===
--- lldb/lit/helper/toolchain.py
+++ lldb/lit/helper/toolchain.py
@@ -20,15 +20,18 @@
command=FindTool('lldb-mi'),
extra_args=['--synchronous'],
unresolved='ignore')
+
+
 build_script = os.path.dirname(__file__)
 build_script = os.path.join(build_script, 'build.py')
 build_script_args = [build_script, 
 '--compiler=any', # Default to best compiler
-'--arch=64'] # Default to 64-bit, user can override
+'--arch=' + str(config.lldb_bitness)]
 if config.lldb_lit_tools_dir:
 build_script_args.append('--tools-dir={0}'.format(config.lldb_lit_tools_dir))
 if config.lldb_tools_dir:
 build_script_args.append('--tools-dir={0}'.format(config.lldb_tools_dir))
+
 primary_tools = [
 ToolSubst('%lldb',
   command=FindTool('lldb'),
Index: lldb/lit/helper/build.py
===
--- lldb/lit/helper/build.py
+++ lldb/lit/helper/build.py
@@ -23,7 +23,9 @@
 metavar='arch',
 dest='arch',
 required=True,
-help='Specify the architecture to target.  Valid values=[32,64]')
+default='host',
+choices=['32', '64', 'host'],
+help='Specify the architecture to target.')
 
 parser.add_argument('--compiler',
 metavar='compiler',
@@ -48,9 +50,16 @@
 parser.add_argument('--output', '-o',
 dest='output',
 metavar='file',
-required=True,
+required=False,
+default='',
 help='Path to output file')
 
+parser.add_argument('--outdir', '-d',
+dest='outdir',
+metavar='directory',
+required=False,
+help='Directory for output files')
+
 parser.add_argument('--nodefaultlib',
 dest='nodefaultlib',
 action='store_true',
@@ -81,9 +90,16 @@
 default=False,
 help='Print verbose output')
 
-parser.add_argument('input',
+parser.add_argument('-n', '--dry-run',
+dest='dry',
+action='store_true',
+default=False,
+help='Print the commands that would run, but dont actually run them')
+
+parser.add_argument('inputs',
 metavar='file',
-help='Source file to compile / object file to link')
+nargs='+',
+help='Source file(s) to compile / object file(s) to link')
 
 
 args = parser.parse_args(args=sys.argv[1:])
@@ -128,14 +144,18 @@
 except AttributeError:
 raise TypeError('not sure how to convert %s to %s' % (type(b), str))
 
+def format_text(lines, indent_0, indent_n):
+result = ' ' * indent_0 + lines[0]
+for next in lines[1:]:
+result = result + '\n{0}{1}'.format(' ' * indent_n, next)
+return result
+
 def print_environment(env):
 for e in env:
 value = env[e]
-split = value.split(os.pathsep)
-print('{0} = {1}'.format(e, split[0]

[Lldb-commits] [lldb] r348186 - Skip TestDriverOptions on Windows

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Dec  3 12:36:21 2018
New Revision: 348186

URL: http://llvm.org/viewvc/llvm-project?rev=348186&view=rev
Log:
Skip TestDriverOptions on Windows

It's not clear to me why this is failing on Windows. Maybe it has
something to do with the path?

Modified:
lldb/trunk/lit/Reproducer/TestDriverOptions.test

Modified: lldb/trunk/lit/Reproducer/TestDriverOptions.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/TestDriverOptions.test?rev=348186&r1=348185&r2=348186&view=diff
==
--- lldb/trunk/lit/Reproducer/TestDriverOptions.test (original)
+++ lldb/trunk/lit/Reproducer/TestDriverOptions.test Mon Dec  3 12:36:21 2018
@@ -1,3 +1,6 @@
+# FIXME: Find out why this fails on Windows.
+# UNSUPPORTED: system-windows
+
 # Check that errors are propagated to the driver.
 
 # RUN: not %lldb --capture /bogus 2>&1 | FileCheck %s --check-prefix CAPTURE


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

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D55038#1317099 , @stella.stamenova 
wrote:

> One of these tests fail on Windows (TestDriverOptions):
>
> http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1848/steps/test/logs/stdio


Thanks, I've disabled the test in r348186 while I try to figure out why it 
fails.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55038



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

[Lldb-commits] [PATCH] D55214: Introduce ObjectFileBreakpad

[Mark Mentovai via Phabricator via lldb-commits]

markmentovai added a comment.

Very excited to see this work beginning!




Comment at: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp:66-84
+  if (os == llvm::Triple::Win32) {
+// In binary form, the module id should have 20 bytes: 16 bytes for UUID,
+// and 4 bytes for the "age". However, in the textual format, the 4 bytes 
of
+// age are printed via %x, which can lead to shorter strings. So, we pad 
the
+// string with zeroes after the 16 bytes, to obtain a string of appropriate
+// size.
+if (token.size() < 33 || token.size() > 40)

labath wrote:
> @lemo: Does this part make sense? It seems that on linux the breakpad files 
> have the `INFO CODE_ID` section, which contains the UUID without the funny 
> trailing zero. So I could try fetching the UUID from there instead, but only 
> on linux, as that section is not present mac (and on windows it contains 
> something completely different). Right now I compute the UUID on linux by 
> chopping off the trailing zero (as I have to do that anyway for mac), but I 
> could do something different is there's any advantage to that.
INFO CODE_ID, if present, is a better thing to use than what you find in 
MODULE, except on Windows, where it’s absolutely the wrong thing to use but 
MODULE is fine.

So, suggested logic:

  if has_code_id and not is_win:
id = code_id
  else:
id = module_id

Aside from special-casing Windows against using INFO CODE_ID, I don’t think you 
should hard-code any OS checks here. There’s no reason Mac dump_syms couldn’t 
emit INFO CODE_ID, even though it doesn’t currently.

(In fact, you don’t even need to special-case for Windows. You could just 
detect the presence of a filename token after the ID in INFO CODE_ID. As your 
test data shows, Windows dump_syms always puts the module filename here, as in 
“INFO CODE_ID 5C01672A4000 a.exe”, but other dump_syms will only have the 
uncorrupted debug ID.


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

https://reviews.llvm.org/D55214



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

Re: [Lldb-commits] [lldb] r348186 - Skip TestDriverOptions on Windows

[Zachary Turner via lldb-commits]

Perhaps use %t.bogus? instead of /bogus?

On Mon, Dec 3, 2018 at 12:39 PM Jonas Devlieghere via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: jdevlieghere
> Date: Mon Dec  3 12:36:21 2018
> New Revision: 348186
>
> URL: http://llvm.org/viewvc/llvm-project?rev=348186&view=rev
> Log:
> Skip TestDriverOptions on Windows
>
> It's not clear to me why this is failing on Windows. Maybe it has
> something to do with the path?
>
> Modified:
> lldb/trunk/lit/Reproducer/TestDriverOptions.test
>
> Modified: lldb/trunk/lit/Reproducer/TestDriverOptions.test
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/TestDriverOptions.test?rev=348186&r1=348185&r2=348186&view=diff
>
> ==
> --- lldb/trunk/lit/Reproducer/TestDriverOptions.test (original)
> +++ lldb/trunk/lit/Reproducer/TestDriverOptions.test Mon Dec  3 12:36:21
> 2018
> @@ -1,3 +1,6 @@
> +# FIXME: Find out why this fails on Windows.
> +# UNSUPPORTED: system-windows
> +
>  # Check that errors are propagated to the driver.
>
>  # RUN: not %lldb --capture /bogus 2>&1 | FileCheck %s --check-prefix
> CAPTURE
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55230: [lit] Multiple build outputs and default target bitness

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

What is the reason we aren't using cmake + ninja for this kind of stuff? Or is 
it using it under the covers?


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

https://reviews.llvm.org/D55230



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

Re: [Lldb-commits] [lldb] r348186 - Skip TestDriverOptions on Windows

[Jonas Devlieghere via lldb-commits]

That wouldn't work because we try to create the directory which would
succeeded in the temp dir. I'd have to be something you don't have access
to, like the root or some network drive.

On Mon, Dec 3, 2018 at 12:53 PM Zachary Turner  wrote:

> Perhaps use %t.bogus? instead of /bogus?
>
> On Mon, Dec 3, 2018 at 12:39 PM Jonas Devlieghere via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> Author: jdevlieghere
>> Date: Mon Dec  3 12:36:21 2018
>> New Revision: 348186
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=348186&view=rev
>> Log:
>> Skip TestDriverOptions on Windows
>>
>> It's not clear to me why this is failing on Windows. Maybe it has
>> something to do with the path?
>>
>> Modified:
>> lldb/trunk/lit/Reproducer/TestDriverOptions.test
>>
>> Modified: lldb/trunk/lit/Reproducer/TestDriverOptions.test
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/TestDriverOptions.test?rev=348186&r1=348185&r2=348186&view=diff
>>
>> ==
>> --- lldb/trunk/lit/Reproducer/TestDriverOptions.test (original)
>> +++ lldb/trunk/lit/Reproducer/TestDriverOptions.test Mon Dec  3 12:36:21
>> 2018
>> @@ -1,3 +1,6 @@
>> +# FIXME: Find out why this fails on Windows.
>> +# UNSUPPORTED: system-windows
>> +
>>  # Check that errors are propagated to the driver.
>>
>>  # RUN: not %lldb --capture /bogus 2>&1 | FileCheck %s --check-prefix
>> CAPTURE
>>
>>
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55230: [lit] Multiple build outputs and default target bitness

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In D55230#1317320 , @clayborg wrote:

> What is the reason we aren't using cmake + ninja for this kind of stuff? Or 
> is it using it under the covers?


CMake gives you a nice static configuration for how you want to build your host 
toolchain, but that doesn't always match up with how you want to build your 
test inferiors.  You might even have a situation where two test inferiors need 
to be built with different toolchains than each other.  I think dotest has a 
lot of the same issues here, that's why we use the Makefile system over there.

Eventually, we will want to be able to decouple the test suite from the local 
build entirely, so that you could invoke it from the command line similar to 
how you can currently invoke dotest from the command line.  This would allow 
running the test suite multiple times with different toolchains or settings, 
which wouldn't be possible if there's a single upfront configuration done at 
CMake time.

It's also nice for test reproducibility to be able to look at a test and see 
the extact reproducer, e.g. (run this command line to build, then run lldb in 
this way and it repros).


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

https://reviews.llvm.org/D55230



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

Re: [Lldb-commits] [lldb] r348186 - Skip TestDriverOptions on Windows

[Zachary Turner via lldb-commits]

I will just add that once upon a time, there was a test in LLVM that worked
similar to this, and so it would try to open a file like /foo/foo.txt, and
expect it to fail (meaning that if it succeeded, the test would fail).  I
had once created that file on my hard drive by pure coincidence, and so
this test was failing for me.

So, I think we should try to find a way to specify "inaccessible directory"
that doesn't involve potentially flakiness or system-dependence.

Off the top of my head, we could allow LLDB to recognize a hidden /
undocumented environment variable like LLDB_REPRODUCER_NO_CREATE_DIRECTORY,
and we could set that variable before running the test.  Another option
would be to create the directory beforehand using a certain permission mask
that would not cause this test to fail (e.g. you can't read from it or
write to it but you can still delete it).  We'd need a tool that can do
that though.

On Mon, Dec 3, 2018 at 12:55 PM Jonas Devlieghere 
wrote:

> That wouldn't work because we try to create the directory which would
> succeeded in the temp dir. I'd have to be something you don't have access
> to, like the root or some network drive.
>
> On Mon, Dec 3, 2018 at 12:53 PM Zachary Turner  wrote:
>
>> Perhaps use %t.bogus? instead of /bogus?
>>
>> On Mon, Dec 3, 2018 at 12:39 PM Jonas Devlieghere via lldb-commits <
>> lldb-commits@lists.llvm.org> wrote:
>>
>>> Author: jdevlieghere
>>> Date: Mon Dec  3 12:36:21 2018
>>> New Revision: 348186
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=348186&view=rev
>>> Log:
>>> Skip TestDriverOptions on Windows
>>>
>>> It's not clear to me why this is failing on Windows. Maybe it has
>>> something to do with the path?
>>>
>>> Modified:
>>> lldb/trunk/lit/Reproducer/TestDriverOptions.test
>>>
>>> Modified: lldb/trunk/lit/Reproducer/TestDriverOptions.test
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/TestDriverOptions.test?rev=348186&r1=348185&r2=348186&view=diff
>>>
>>> ==
>>> --- lldb/trunk/lit/Reproducer/TestDriverOptions.test (original)
>>> +++ lldb/trunk/lit/Reproducer/TestDriverOptions.test Mon Dec  3 12:36:21
>>> 2018
>>> @@ -1,3 +1,6 @@
>>> +# FIXME: Find out why this fails on Windows.
>>> +# UNSUPPORTED: system-windows
>>> +
>>>  # Check that errors are propagated to the driver.
>>>
>>>  # RUN: not %lldb --capture /bogus 2>&1 | FileCheck %s --check-prefix
>>> CAPTURE
>>>
>>>
>>> ___
>>> lldb-commits mailing list
>>> lldb-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r348207 - [FileSystem] Migrate MonitoringProcessLauncher

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Dec  3 14:41:32 2018
New Revision: 348207

URL: http://llvm.org/viewvc/llvm-project?rev=348207&view=rev
Log:
[FileSystem] Migrate MonitoringProcessLauncher

Use the FileSystem helpers instead of using the file system directly.

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

Modified: lldb/trunk/source/Host/common/MonitoringProcessLauncher.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/MonitoringProcessLauncher.cpp?rev=348207&r1=348206&r2=348207&view=diff
==
--- lldb/trunk/source/Host/common/MonitoringProcessLauncher.cpp (original)
+++ lldb/trunk/source/Host/common/MonitoringProcessLauncher.cpp Mon Dec  3 
14:41:32 2018
@@ -12,7 +12,6 @@
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
 #include "lldb/Utility/Log.h"
-#include "lldb/Utility/Status.h"
 
 #include "llvm/Support/FileSystem.h"
 
@@ -30,20 +29,16 @@ MonitoringProcessLauncher::LaunchProcess
 
   error.Clear();
 
+  FileSystem &fs = FileSystem::Instance();
   FileSpec exe_spec(resolved_info.GetExecutableFile());
 
-  llvm::sys::fs::file_status stats;
-  status(exe_spec.GetPath(), stats);
-  if (!exists(stats)) {
+  if (!fs.Exists(exe_spec))
 FileSystem::Instance().Resolve(exe_spec);
-status(exe_spec.GetPath(), stats);
-  }
-  if (!exists(stats)) {
+
+  if (!fs.Exists(exe_spec))
 FileSystem::Instance().ResolveExecutableLocation(exe_spec);
-status(exe_spec.GetPath(), stats);
-  }
 
-  if (!exists(stats)) {
+  if (!fs.Exists(exe_spec)) {
 error.SetErrorStringWithFormatv("executable doesn't exist: '{0}'",
 exe_spec);
 return HostProcess();


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

[Lldb-commits] [PATCH] D55230: [lit] Multiple build outputs and default target bitness

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added inline comments.



Comment at: lldb/lit/helper/build.py:664
+if args.output and args.mode == 'compile' and len(args.inputs) > 1:
+raise ValueError('Cannot specify -o with mode=compile and multiple 
source files.  Use --outdir instead.')
+

Why not? It makes sense that the test might still need to know and/or specify 
the name of the binary that was produced. The script could require that both -o 
and --outdir are specified, but I think -o is still needed.


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

https://reviews.llvm.org/D55230



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

Re: [Lldb-commits] [PATCH] D55230: [lit] Multiple build outputs and default target bitness

[Zachary Turner via lldb-commits]

If you have -o and multiple input files, which output file are you
specifying? It seems easier to just say that if multiple input files are
present, the output file names are automatically calculated
On Mon, Dec 3, 2018 at 2:48 PM Stella Stamenova via Phabricator <
revi...@reviews.llvm.org> wrote:

> stella.stamenova added inline comments.
>
>
> 
> Comment at: lldb/lit/helper/build.py:664
> +if args.output and args.mode == 'compile' and len(args.inputs) > 1:
> +raise ValueError('Cannot specify -o with mode=compile and
> multiple source files.  Use --outdir instead.')
> +
> 
> Why not? It makes sense that the test might still need to know and/or
> specify the name of the binary that was produced. The script could require
> that both -o and --outdir are specified, but I think -o is still needed.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55230/new/
>
> https://reviews.llvm.org/D55230
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D55230: [lit] Multiple build outputs and default target bitness

[Stella Stamenova via lldb-commits]

Are there no cases where you want all of the input files to be compiled into a 
single executable?

From: Zachary Turner 
Sent: Monday, December 3, 2018 3:36 PM
To: reviews+d55230+public+74273d963dffb...@reviews.llvm.org
Cc: pa...@labath.sk; Stella Stamenova ; 
aleksandr.ura...@jetbrains.com; clayb...@gmail.com; mgo...@gentoo.org; 
lldb-commits@lists.llvm.org; l...@inglorion.net
Subject: Re: [PATCH] D55230: [lit] Multiple build outputs and default target 
bitness

If you have -o and multiple input files, which output file are you specifying? 
It seems easier to just say that if multiple input files are present, the 
output file names are automatically calculated
On Mon, Dec 3, 2018 at 2:48 PM Stella Stamenova via Phabricator 
mailto:revi...@reviews.llvm.org>> wrote:
stella.stamenova added inline comments.



Comment at: lldb/lit/helper/build.py:664
+if args.output and args.mode == 'compile' and len(args.inputs) > 1:
+raise ValueError('Cannot specify -o with mode=compile and multiple 
source files.  Use --outdir instead.')
+

Why not? It makes sense that the test might still need to know and/or specify 
the name of the binary that was produced. The script could require that both -o 
and --outdir are specified, but I think -o is still needed.


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

https://reviews.llvm.org/D55230


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

[Lldb-commits] [PATCH] D55240: [FileSystem] Migrate CommandCompletions

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: davide, labath.
JDevlieghere added a project: LLDB.

Make use of the convenience helpers from FileSystem.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D55240

Files:
  include/lldb/Host/FileSystem.h
  source/Commands/CommandCompletions.cpp
  source/Host/common/FileSystem.cpp

Index: source/Host/common/FileSystem.cpp
===
--- source/Host/common/FileSystem.cpp
+++ source/Host/common/FileSystem.cpp
@@ -63,6 +63,25 @@
   return g_fs;
 }
 
+vfs::directory_iterator FileSystem::DirBegin(const FileSpec &file_spec,
+ std::error_code &ec) {
+  return DirBegin(file_spec.GetPath(), ec);
+}
+
+vfs::directory_iterator FileSystem::DirBegin(const Twine &dir,
+ std::error_code &ec) {
+  return m_fs->dir_begin(dir, ec);
+}
+
+llvm::ErrorOr
+FileSystem::GetStatus(const FileSpec &file_spec) const {
+  return GetStatus(file_spec.GetPath());
+}
+
+llvm::ErrorOr FileSystem::GetStatus(const Twine &path) const {
+  return m_fs->status(path);
+}
+
 sys::TimePoint<>
 FileSystem::GetModificationTime(const FileSpec &file_spec) const {
   return GetModificationTime(file_spec.GetPath());
Index: source/Commands/CommandCompletions.cpp
===
--- source/Commands/CommandCompletions.cpp
+++ source/Commands/CommandCompletions.cpp
@@ -101,7 +101,6 @@
   if (CompletionBuffer.size() >= PATH_MAX)
 return matches.GetSize();
 
-  namespace fs = llvm::sys::fs;
   namespace path = llvm::sys::path;
 
   llvm::StringRef SearchDir;
@@ -178,11 +177,16 @@
   // SearchDir now contains the directory to search in, and Prefix contains the
   // text we want to match against items in that directory.
 
+  FileSystem fs = FileSystem::Instance();
   std::error_code EC;
-  fs::directory_iterator Iter(SearchDir, EC, false);
-  fs::directory_iterator End;
+  llvm::vfs::directory_iterator Iter = fs.DirBegin(SearchDir, EC);
+  llvm::vfs::directory_iterator End;
   for (; Iter != End && !EC; Iter.increment(EC)) {
 auto &Entry = *Iter;
+llvm::ErrorOr Status = fs.GetStatus(Entry.path());
+
+if (!Status)
+  continue;
 
 auto Name = path::filename(Entry.path());
 
@@ -190,20 +194,18 @@
 if (Name == "." || Name == ".." || !Name.startswith(PartialItem))
   continue;
 
-// We have a match.
-
-llvm::ErrorOr st = Entry.status();
-if (!st)
-  continue;
+bool is_dir = Status->isDirectory();
 
 // If it's a symlink, then we treat it as a directory as long as the target
 // is a directory.
-bool is_dir = fs::is_directory(*st);
-if (fs::is_symlink_file(*st)) {
-  fs::file_status target_st;
-  if (!fs::status(Entry.path(), target_st))
-is_dir = fs::is_directory(target_st);
+if (Status->isSymlink()) {
+  FileSpec symlink_filespec(Entry.path());
+  FileSpec resolved_filespec;
+  auto error = fs.ResolveSymbolicLink(symlink_filespec, resolved_filespec);
+  if (error.Success())
+is_dir = fs.IsDirectory(symlink_filespec);
 }
+
 if (only_directories && !is_dir)
   continue;
 
Index: include/lldb/Host/FileSystem.h
===
--- include/lldb/Host/FileSystem.h
+++ include/lldb/Host/FileSystem.h
@@ -54,6 +54,20 @@
   Status Open(File &File, const FileSpec &file_spec, uint32_t options,
   uint32_t permissions = lldb::eFilePermissionsFileDefault);
 
+  /// Get a directory iterator.
+  /// @{
+  llvm::vfs::directory_iterator DirBegin(const FileSpec &file_spec,
+ std::error_code &ec);
+  llvm::vfs::directory_iterator DirBegin(const llvm::Twine &dir,
+ std::error_code &ec);
+  /// @}
+
+  /// Returns the Status object for the given file.
+  /// @{
+  llvm::ErrorOr GetStatus(const FileSpec &file_spec) const;
+  llvm::ErrorOr GetStatus(const llvm::Twine &path) const;
+  /// @}
+
   /// Returns the modification time of the given file.
   /// @{
   llvm::sys::TimePoint<> GetModificationTime(const FileSpec &file_spec) const;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55240: [FileSystem] Migrate CommandCompletions

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

No objections from me, but I would appreciate if @labath can take a look.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55240



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

Re: [Lldb-commits] [PATCH] D55230: [lit] Multiple build outputs and default target bitness

[Zachary Turner via lldb-commits]

If that's the case, you specify -mode="compile-and-link" and you can
specify the output.  That will work fine.  The condition in the code only
checks for -mode='compile'

On Mon, Dec 3, 2018 at 3:38 PM Stella Stamenova 
wrote:

> Are there no cases where you want all of the input files to be compiled
> into a single executable?
>
>
>
> *From:* Zachary Turner 
> *Sent:* Monday, December 3, 2018 3:36 PM
> *To:* reviews+d55230+public+74273d963dffb...@reviews.llvm.org
> *Cc:* pa...@labath.sk; Stella Stamenova ;
> aleksandr.ura...@jetbrains.com; clayb...@gmail.com; mgo...@gentoo.org;
> lldb-commits@lists.llvm.org; l...@inglorion.net
> *Subject:* Re: [PATCH] D55230: [lit] Multiple build outputs and default
> target bitness
>
>
>
> If you have -o and multiple input files, which output file are you
> specifying? It seems easier to just say that if multiple input files are
> present, the output file names are automatically calculated
>
> On Mon, Dec 3, 2018 at 2:48 PM Stella Stamenova via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
> stella.stamenova added inline comments.
>
>
> 
> Comment at: lldb/lit/helper/build.py:664
> +if args.output and args.mode == 'compile' and len(args.inputs) > 1:
> +raise ValueError('Cannot specify -o with mode=compile and
> multiple source files.  Use --outdir instead.')
> +
> 
> Why not? It makes sense that the test might still need to know and/or
> specify the name of the binary that was produced. The script could require
> that both -o and --outdir are specified, but I think -o is still needed.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55230/new/
> 
>
> https://reviews.llvm.org/D55230
> 
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D55230: [lit] Multiple build outputs and default target bitness

[Zachary Turner via lldb-commits]

The idea is that if you specify -mode=compile, then no link happens and the
output is potentially many object files.  If there's only one object file,
then we allow the user to specify the name of that output.  But if there's
multiple, then it's hard to specify that with a single command line
invocation.  The user could always write multiple run lines each with
-mode=compile and an explicit output file.

When you link, there's always exactly one output file, the executable (we
don't count the PDB or DWP here, since we don't need to yet.  Maybe in the
future if we need this someday we can revisit).  So for -mode=compile or
-mode=compile-and-link, this is never an issue, since we only ever have 1
output.

It only comes up when you're compiling but *not* linking, *and* you specify
multiple inputs (and hence multiple outputs).  We could always just forbid
multiple inputs entirely with -mode=compile and require the user to write
multiple RUN lines, but it seems likely that all of the options will be the
same across all invocations, so this would be a nice shortcut.

On Mon, Dec 3, 2018 at 3:59 PM Zachary Turner  wrote:

> If that's the case, you specify -mode="compile-and-link" and you can
> specify the output.  That will work fine.  The condition in the code only
> checks for -mode='compile'
>
> On Mon, Dec 3, 2018 at 3:38 PM Stella Stamenova 
> wrote:
>
>> Are there no cases where you want all of the input files to be compiled
>> into a single executable?
>>
>>
>>
>> *From:* Zachary Turner 
>> *Sent:* Monday, December 3, 2018 3:36 PM
>> *To:* reviews+d55230+public+74273d963dffb...@reviews.llvm.org
>> *Cc:* pa...@labath.sk; Stella Stamenova ;
>> aleksandr.ura...@jetbrains.com; clayb...@gmail.com; mgo...@gentoo.org;
>> lldb-commits@lists.llvm.org; l...@inglorion.net
>> *Subject:* Re: [PATCH] D55230: [lit] Multiple build outputs and default
>> target bitness
>>
>>
>>
>> If you have -o and multiple input files, which output file are you
>> specifying? It seems easier to just say that if multiple input files are
>> present, the output file names are automatically calculated
>>
>> On Mon, Dec 3, 2018 at 2:48 PM Stella Stamenova via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>> stella.stamenova added inline comments.
>>
>>
>> 
>> Comment at: lldb/lit/helper/build.py:664
>> +if args.output and args.mode == 'compile' and len(args.inputs) > 1:
>> +raise ValueError('Cannot specify -o with mode=compile and
>> multiple source files.  Use --outdir instead.')
>> +
>> 
>> Why not? It makes sense that the test might still need to know and/or
>> specify the name of the binary that was produced. The script could require
>> that both -o and --outdir are specified, but I think -o is still needed.
>>
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D55230/new/
>> 
>>
>> https://reviews.llvm.org/D55230
>> 
>>
>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55240: [FileSystem] Migrate CommandCompletions

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 176525.
JDevlieghere added a comment.

Initialize the FS in the unit test.


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

https://reviews.llvm.org/D55240

Files:
  include/lldb/Host/FileSystem.h
  source/Commands/CommandCompletions.cpp
  source/Host/common/FileSystem.cpp
  unittests/Interpreter/TestCompletion.cpp

Index: unittests/Interpreter/TestCompletion.cpp
===
--- unittests/Interpreter/TestCompletion.cpp
+++ unittests/Interpreter/TestCompletion.cpp
@@ -7,9 +7,11 @@
 //
 //===--===//
 
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/CommandCompletions.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/TildeExpressionResolver.h"
+
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -65,6 +67,8 @@
   SmallString<128> FileBaz;
 
   void SetUp() override {
+FileSystem::Initialize();
+
 // chdir back into the original working dir this test binary started with.
 // A previous test may have have changed the working dir.
 ASSERT_NO_ERROR(fs::set_current_path(OriginalWorkingDir));
@@ -105,7 +109,10 @@
 ASSERT_NO_ERROR(fs::current_path(OriginalWorkingDir));
   }
 
-  void TearDown() override { ASSERT_NO_ERROR(fs::remove_directories(BaseDir)); }
+  void TearDown() override {
+ASSERT_NO_ERROR(fs::remove_directories(BaseDir));
+FileSystem::Terminate();
+  }
 
   static bool HasEquivalentFile(const Twine &Path, const StringList &Paths) {
 for (size_t I = 0; I < Paths.GetSize(); ++I) {
@@ -140,7 +147,7 @@
 };
 
 SmallString<128> CompletionTest::OriginalWorkingDir;
-}
+} // namespace
 
 static std::vector toVector(const StringList &SL) {
   std::vector Result;
@@ -170,8 +177,8 @@
   ASSERT_EQ(Count, Results.GetSize());
   EXPECT_TRUE(HasEquivalentFile(DirFooA, Results));
 
-  Count =
-CommandCompletions::DiskDirectories(Twine(BaseDir) + "/.", Results, Resolver);
+  Count = CommandCompletions::DiskDirectories(Twine(BaseDir) + "/.", Results,
+  Resolver);
   ASSERT_EQ(0u, Count);
   ASSERT_EQ(Count, Results.GetSize());
 
Index: source/Host/common/FileSystem.cpp
===
--- source/Host/common/FileSystem.cpp
+++ source/Host/common/FileSystem.cpp
@@ -63,6 +63,25 @@
   return g_fs;
 }
 
+vfs::directory_iterator FileSystem::DirBegin(const FileSpec &file_spec,
+ std::error_code &ec) {
+  return DirBegin(file_spec.GetPath(), ec);
+}
+
+vfs::directory_iterator FileSystem::DirBegin(const Twine &dir,
+ std::error_code &ec) {
+  return m_fs->dir_begin(dir, ec);
+}
+
+llvm::ErrorOr
+FileSystem::GetStatus(const FileSpec &file_spec) const {
+  return GetStatus(file_spec.GetPath());
+}
+
+llvm::ErrorOr FileSystem::GetStatus(const Twine &path) const {
+  return m_fs->status(path);
+}
+
 sys::TimePoint<>
 FileSystem::GetModificationTime(const FileSpec &file_spec) const {
   return GetModificationTime(file_spec.GetPath());
Index: source/Commands/CommandCompletions.cpp
===
--- source/Commands/CommandCompletions.cpp
+++ source/Commands/CommandCompletions.cpp
@@ -101,7 +101,6 @@
   if (CompletionBuffer.size() >= PATH_MAX)
 return matches.GetSize();
 
-  namespace fs = llvm::sys::fs;
   namespace path = llvm::sys::path;
 
   llvm::StringRef SearchDir;
@@ -178,11 +177,16 @@
   // SearchDir now contains the directory to search in, and Prefix contains the
   // text we want to match against items in that directory.
 
+  FileSystem fs = FileSystem::Instance();
   std::error_code EC;
-  fs::directory_iterator Iter(SearchDir, EC, false);
-  fs::directory_iterator End;
+  llvm::vfs::directory_iterator Iter = fs.DirBegin(SearchDir, EC);
+  llvm::vfs::directory_iterator End;
   for (; Iter != End && !EC; Iter.increment(EC)) {
 auto &Entry = *Iter;
+llvm::ErrorOr Status = fs.GetStatus(Entry.path());
+
+if (!Status)
+  continue;
 
 auto Name = path::filename(Entry.path());
 
@@ -190,20 +194,18 @@
 if (Name == "." || Name == ".." || !Name.startswith(PartialItem))
   continue;
 
-// We have a match.
-
-llvm::ErrorOr st = Entry.status();
-if (!st)
-  continue;
+bool is_dir = Status->isDirectory();
 
 // If it's a symlink, then we treat it as a directory as long as the target
 // is a directory.
-bool is_dir = fs::is_directory(*st);
-if (fs::is_symlink_file(*st)) {
-  fs::file_status target_st;
-  if (!fs::status(Entry.path(), target_st))
-is_dir = fs::is_directory(target_st);
+if (Status->isSymlink()) {
+  FileSpec symlink_filespec(Entry.path());
+  FileSpec resolved_filespec;
+  auto error = fs.ResolveSymbolicLink(symlink_filespec, resolved_fil

[Lldb-commits] [lldb] r348232 - [PlatformDarwin] Simplify logic and use FileSystem

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Dec  3 18:23:16 2018
New Revision: 348232

URL: http://llvm.org/viewvc/llvm-project?rev=348232&view=rev
Log:
[PlatformDarwin] Simplify logic and use FileSystem

Simplify code path by using the FileSystem.

Modified:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp?rev=348232&r1=348231&r2=348232&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp Mon Dec  3 
18:23:16 2018
@@ -190,25 +190,12 @@ FileSpecList PlatformDarwin::LocateExecu
 Status PlatformDarwin::ResolveSymbolFile(Target &target,
  const ModuleSpec &sym_spec,
  FileSpec &sym_file) {
-  Status error;
   sym_file = sym_spec.GetSymbolFileSpec();
-
-  llvm::sys::fs::file_status st;
-  if (status(sym_file.GetPath(), st, false)) {
-error.SetErrorString("Could not stat file!");
-return error;
-  }
-
-  if (exists(st)) {
-if (is_directory(st)) {
-  sym_file = Symbols::FindSymbolFileInBundle(
-  sym_file, sym_spec.GetUUIDPtr(), sym_spec.GetArchitecturePtr());
-}
-  } else {
-if (sym_spec.GetUUID().IsValid()) {
-}
+  if (FileSystem::Instance().IsDirectory(sym_file)) {
+sym_file = Symbols::FindSymbolFileInBundle(sym_file, sym_spec.GetUUIDPtr(),
+   sym_spec.GetArchitecturePtr());
   }
-  return error;
+  return {};
 }
 
 static lldb_private::Status


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

[Lldb-commits] [PATCH] D54843: [Expr] Check the language before ignoring Objective C keywords

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.

I think the ideal behavior would be "if we don't have debug info for the frame, 
choose ObjC++".  But if you are in frame with debug info, obey that frame's 
language (except that we have to use C++ because the expression parser uses C++ 
references to play some of the games it plays.)  The reasoning behind that is 
because in the first case, you won' have local names and are calling out to 
global objects and having the widest language available is the most convenient. 
 If we did it that way,  we could also make the user's choice of language 
always hold (except again you can't use "class" anywhere because of the use of 
C++...).

But I don't see an easy way to implement that behavior.  You'd have to plumb 
through another bit - has debug info - which is orthogonal to the language, but 
have that affect the language choice.

However, this change is strictly better for non-objc programs, and preserves 
the same behavior as currently holds when there is ObjC.  So I think this 
change is fine for now.


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

https://reviews.llvm.org/D54843



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