date:20190111

[Lldb-commits] [PATCH] D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages

2019-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: cmake/modules/LLDBStandalone.cmake:9
+  find_package(LLVM REQUIRED CONFIG
+HINTS "${LLDB_PATH_TO_LLVM_BUILD}" NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
+  find_package(Clang REQUIRED CONFIG

xiaobai wrote:
> labath wrote:
> > Why do you put `NO_DEFAULT_PATH` here? IIUC, the user will now have to 
> > specify `LLDB_PATH_TO_LLVM_BUILD` in order to build this, whereas 
> > previously llvm-config would be found on the path if it happened to be 
> > there (e.g. because it was already installed).
> > 
> > Would it not make sense to keep this behavior?
> In situations where you have multiple LLVM builds where each might be a 
> different version of LLVM, I think it is better to force the user to specify 
> which LLVM build they want to use instead of having them implicitly rely on 
> whichever llvm-config happens to be in their path.
> 
> That being said, I would be willing to remove `NO_DEFAULT_PATH` here. I can 
> understand if people find this behavior valuable or if the scenario I 
> described is not very common.
I don't actually use the standalone build, so I don't care about this too much. 
I just mentioned this because this is the default behavior when searching for 
packages (as well as the previous behavior when we searched for llvm-config). 
However, it is true that we are version-locked much more tightly with llvm than 
with any of the other packages we search for with find_package.

The other thing that bugs me about the LLDB_PATH_TO_(LLVM|CLANG)_BUILD 
variables is that they seem to imply that you should point them to the build 
tree of llvm/clang. However, it should be possible to build lldb against an 
already-installed llvm, right (in which case they will likely have the same 
value)? In either case, I think it would be nice to explicitly declare these as 
a cache variable, if only so we can provide a docstring for them.


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

https://reviews.llvm.org/D56531



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

[Lldb-commits] [PATCH] D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages

2019-01-11 Thread Michał Górny via Phabricator via lldb-commits

mgorny added inline comments.



Comment at: cmake/modules/LLDBStandalone.cmake:9
+  find_package(LLVM REQUIRED CONFIG
+HINTS "${LLDB_PATH_TO_LLVM_BUILD}" NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
+  find_package(Clang REQUIRED CONFIG

labath wrote:
> xiaobai wrote:
> > labath wrote:
> > > Why do you put `NO_DEFAULT_PATH` here? IIUC, the user will now have to 
> > > specify `LLDB_PATH_TO_LLVM_BUILD` in order to build this, whereas 
> > > previously llvm-config would be found on the path if it happened to be 
> > > there (e.g. because it was already installed).
> > > 
> > > Would it not make sense to keep this behavior?
> > In situations where you have multiple LLVM builds where each might be a 
> > different version of LLVM, I think it is better to force the user to 
> > specify which LLVM build they want to use instead of having them implicitly 
> > rely on whichever llvm-config happens to be in their path.
> > 
> > That being said, I would be willing to remove `NO_DEFAULT_PATH` here. I can 
> > understand if people find this behavior valuable or if the scenario I 
> > described is not very common.
> I don't actually use the standalone build, so I don't care about this too 
> much. I just mentioned this because this is the default behavior when 
> searching for packages (as well as the previous behavior when we searched for 
> llvm-config). However, it is true that we are version-locked much more 
> tightly with llvm than with any of the other packages we search for with 
> find_package.
> 
> The other thing that bugs me about the LLDB_PATH_TO_(LLVM|CLANG)_BUILD 
> variables is that they seem to imply that you should point them to the build 
> tree of llvm/clang. However, it should be possible to build lldb against an 
> already-installed llvm, right (in which case they will likely have the same 
> value)? In either case, I think it would be nice to explicitly declare these 
> as a cache variable, if only so we can provide a docstring for them.
In situations when you have multiple versions of LLVM in PATH, you usually set 
PATH in the order you want it to be used. And you really don't like when 
projects try to second-guess you.


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

https://reviews.llvm.org/D56531



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

lldb-commits@lists.llvm.org

2019-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.

looks good to me.




Comment at: lldb/include/lldb/Symbol/SymbolFile.h:135
+ FileSpecList &support_files) = 0;
+  virtual size_t ParseTypes(CompileUnit &comp_unit) = 0;
+  virtual bool ParseIsOptimized(lldb_private::CompileUnit &comp_unit) {

You're not consistent in the namespace qualification of CompileUnit. I'd 
suggest removing the namespace qualification, since you're already in that 
namespace anyway...


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

https://reviews.llvm.org/D56564



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

[Lldb-commits] [PATCH] D56014: [lldb] - Fix crash when listing the history with the key up.

2019-01-11 Thread George Rimar via Phabricator via lldb-commits

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

In D56014#1341129 , @labath wrote:

> I guess it should be possible to trigger this from dotest tests via pexpect. 
> I know we try to avoid it, but given that this is specifically testing 
> terminal interaction, using it here seems appropriate.


Thanks for the suggestion. I'll try to investigate it and return with the 
results during the next week I guess.


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

https://reviews.llvm.org/D56014



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

[Lldb-commits] [lldb] r350923 - ELF: Fix base address computation code for files generated by yaml2obj

2019-01-11 Thread Pavel Labath via lldb-commits

Author: labath
Date: Fri Jan 11 02:18:40 2019
New Revision: 350923

URL: http://llvm.org/viewvc/llvm-project?rev=350923&view=rev
Log:
ELF: Fix base address computation code for files generated by yaml2obj

The code was assuming that the elf file will have a PT_LOAD segment
starting from the first byte of the file. While this is true for files
generated by most linkers (it's a way of saving space), it is not a
requirement. And files not satisfying this constraint can still be
perfectly executable. yaml2obj is one of the tools which produces files
like this.

This patch relaxes the check in ObjectFileELF to take the address of the
first PT_LOAD segment as the base address of the object (instead of the
one with the offset 0). Since the PT_LOAD segments are supposed to be
sorted according to the VM address, this entry will also be the one with
the lowest VM address.

If we ever run into files which don't have the PT_LOAD segments sorted,
we can easily change this code to return the lowest VM address as the
base address (if that is the correct thing to do for these files).

Added:
lldb/trunk/lit/Modules/ELF/base-address.yaml
Modified:
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Added: lldb/trunk/lit/Modules/ELF/base-address.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/ELF/base-address.yaml?rev=350923&view=auto
==
--- lldb/trunk/lit/Modules/ELF/base-address.yaml (added)
+++ lldb/trunk/lit/Modules/ELF/base-address.yaml Fri Jan 11 02:18:40 2019
@@ -0,0 +1,34 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Base VM address: 0x40
+
+--- !ELF
+FileHeader:  
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x00400078
+Sections:
+  - Name:.pad
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC ]
+Address: 0x0040
+AddressAlign:0x1000
+Size:0x78
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x00400078
+AddressAlign:0x0001
+Content: 48C7C0E70048C7C72F000F05CC
+ProgramHeaders:
+  - Type: PT_LOAD
+Flags: [ PF_X, PF_R ]
+VAddr: 0x40
+Align: 0x20
+Sections:
+  - Section: .pad
+  - Section: .text
+...

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=350923&r1=350922&r2=350923&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Fri Jan 11 
02:18:40 2019
@@ -1051,7 +1051,7 @@ lldb_private::Address ObjectFileELF::Get
 Address ObjectFileELF::GetBaseAddress() {
   for (const auto &EnumPHdr : llvm::enumerate(ProgramHeaders())) {
 const ELFProgramHeader &H = EnumPHdr.value();
-if (H.p_type != PT_LOAD || H.p_offset != 0)
+if (H.p_type != PT_LOAD)
   continue;
 
 return Address(


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

[Lldb-commits] [lldb] r350924 - Introduce SymbolFileBreakpad and use it to fill symtab

2019-01-11 Thread Pavel Labath via lldb-commits

Author: labath
Date: Fri Jan 11 03:17:51 2019
New Revision: 350924

URL: http://llvm.org/viewvc/llvm-project?rev=350924&view=rev
Log:
Introduce SymbolFileBreakpad and use it to fill symtab

Summary:
This commit adds the glue code necessary to integrate the
SymbolFileBreakpad into the plugin system. Most of the methods are
stubbed out. The only method implemented method is AddSymbols, which
parses the PUBLIC "section" of the breakpad "object file", and fills out
the Module's symtab.

To enable testing this, I've made two additional changes:
- dump Symtab from the SymbolVendor class. The symtab was already being
  dumped as a part of the object file dump, but that happened before
  symbol vendor kicked in, so it did not reflect any symbols added
  there.
- add ability to explicitly specify the external symbol file in
  lldb-test (so that the object file could be linked with the breakpad
  symbol file). To make things simpler, I've changed lldb-test from
  consuming multiple inputs (and dumping their symbols) to having it
  just process a single file per invocation. This was not a problem
  since everyone was using it that way already.

Reviewers: clayborg, zturner, lemo, markmentovai, amccarth

Subscribers: mgorny, lldb-commits

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

Added:
lldb/trunk/lit/SymbolFile/Breakpad/
lldb/trunk/lit/SymbolFile/Breakpad/Inputs/
lldb/trunk/lit/SymbolFile/Breakpad/Inputs/basic-elf.yaml
lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms
lldb/trunk/lit/SymbolFile/Breakpad/symtab.test
lldb/trunk/source/Plugins/SymbolFile/Breakpad/
lldb/trunk/source/Plugins/SymbolFile/Breakpad/CMakeLists.txt
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
Modified:
lldb/trunk/source/API/SystemInitializerFull.cpp
lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt
lldb/trunk/source/Symbol/SymbolVendor.cpp
lldb/trunk/tools/lldb-test/SystemInitializerTest.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp

Added: lldb/trunk/lit/SymbolFile/Breakpad/Inputs/basic-elf.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Breakpad/Inputs/basic-elf.yaml?rev=350924&view=auto
==
--- lldb/trunk/lit/SymbolFile/Breakpad/Inputs/basic-elf.yaml (added)
+++ lldb/trunk/lit/SymbolFile/Breakpad/Inputs/basic-elf.yaml Fri Jan 11 
03:17:51 2019
@@ -0,0 +1,33 @@
+# A very basic ELF file to serve as a recipient of breakpad info
+
+--- !ELF
+FileHeader:  
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x004000D0
+Sections:
+  - Name:.text1
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x0040
+AddressAlign:0x1000
+Size:0xb0
+  - Name:.text2
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x004000B0
+AddressAlign:0x0010
+Size:0x42
+Symbols: 
+DynamicSymbols:  
+ProgramHeaders:
+  - Type: PT_LOAD
+Flags: [ PF_X, PF_R ]
+VAddr: 0x40
+Align: 0x1000
+Sections:
+  - Section: .text1
+  - Section: .text2
+...

Added: lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms?rev=350924&view=auto
==
--- lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms (added)
+++ lldb/trunk/lit/SymbolFile/Breakpad/Inputs/symtab.syms Fri Jan 11 03:17:51 
2019
@@ -0,0 +1,7 @@
+MODULE Linux x86_64 761550E08086333960A9074A9CE2895C0 a.out
+INFO CODE_ID E05015768680393360A9074A9CE2895C
+FILE 0 /tmp/a.c
+PUBLIC b0 0 f1
+PUBLIC m c0 0 f2
+PUBLIC d0 0 _start
+PUBLIC ff 0 _out_of_range_ignored

Added: lldb/trunk/lit/SymbolFile/Breakpad/symtab.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Breakpad/symtab.test?rev=350924&view=auto
==
--- lldb/trunk/lit/SymbolFile/Breakpad/symtab.test (added)
+++ lldb/trunk/lit/SymbolFile/Breakpad/symtab.test Fri Jan 11 03:17:51 2019
@@ -0,0 +1,23 @@
+# RUN: yaml2obj %S/Inputs/basic-elf.yaml > %T/symtab.out
+# RUN: %lldb %T/symtab.out -o "target symbols add -s symtab.out 
%S/Inputs/symtab.syms" \
+# RUN:   -s %s | FileCheck %s
+
+# CHECK-LABEL: (lldb) image dump symtab symtab.out
+# CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 3:
+# CHECK: Index   UserID DSX TypeFile Address/Value Load Address
   Size   Flags  Name
+# CHECK: [0]  0   X Code0x004000b0 
   0x0

[Lldb-commits] [PATCH] D56173: Introduce SymbolFileBreakpad and use it to fill symtab

2019-01-11 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB350924: Introduce SymbolFileBreakpad and use it to fill 
symtab (authored by labath, committed by ).
Herald added a subscriber: abidh.

Changed prior to commit:
  https://reviews.llvm.org/D56173?vs=180240&id=181233#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56173

Files:
  lit/SymbolFile/Breakpad/Inputs/basic-elf.yaml
  lit/SymbolFile/Breakpad/Inputs/symtab.syms
  lit/SymbolFile/Breakpad/symtab.test
  source/API/SystemInitializerFull.cpp
  source/Plugins/SymbolFile/Breakpad/CMakeLists.txt
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  source/Plugins/SymbolFile/CMakeLists.txt
  source/Symbol/SymbolVendor.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
@@ -100,10 +100,14 @@
 } // namespace object
 
 namespace symbols {
-static cl::list InputFilenames(cl::Positional,
-cl::desc(""),
-cl::OneOrMore,
-cl::sub(SymbolsSubcommand));
+static cl::opt InputFile(cl::Positional, cl::desc(""),
+  cl::Required, cl::sub(SymbolsSubcommand));
+
+static cl::opt
+SymbolPath("symbol-file",
+   cl::desc("The file from which to fetch symbol information."),
+   cl::value_desc("file"), cl::sub(SymbolsSubcommand));
+
 enum class FindType {
   None,
   Function,
@@ -692,28 +696,24 @@
   }
   auto Action = *ActionOr;
 
-  int HadErrors = 0;
-  for (const auto &File : InputFilenames) {
-outs() << "Module: " << File << "\n";
-ModuleSpec Spec{FileSpec(File)};
-Spec.GetSymbolFileSpec().SetFile(File, FileSpec::Style::native);
-
-auto ModulePtr = std::make_shared(Spec);
-SymbolVendor *Vendor = ModulePtr->GetSymbolVendor();
-if (!Vendor) {
-  WithColor::error() << "Module has no symbol vendor.\n";
-  HadErrors = 1;
-  continue;
-}
-
-if (Error E = Action(*ModulePtr)) {
-  WithColor::error() << toString(std::move(E)) << "\n";
-  HadErrors = 1;
-}
+  outs() << "Module: " << InputFile << "\n";
+  ModuleSpec Spec{FileSpec(InputFile)};
+  StringRef Symbols = SymbolPath.empty() ? InputFile : SymbolPath;
+  Spec.GetSymbolFileSpec().SetFile(Symbols, FileSpec::Style::native);
+
+  auto ModulePtr = std::make_shared(Spec);
+  SymbolVendor *Vendor = ModulePtr->GetSymbolVendor();
+  if (!Vendor) {
+WithColor::error() << "Module has no symbol vendor.\n";
+return 1;
+  }
 
-outs().flush();
+  if (Error E = Action(*ModulePtr)) {
+WithColor::error() << toString(std::move(E)) << "\n";
+return 1;
   }
-  return HadErrors;
+
+  return 0;
 }
 
 static void dumpSectionList(LinePrinter &Printer, const SectionList &List, bool is_subsection) {
Index: tools/lldb-test/SystemInitializerTest.cpp
===
--- tools/lldb-test/SystemInitializerTest.cpp
+++ tools/lldb-test/SystemInitializerTest.cpp
@@ -70,6 +70,7 @@
 #include "Plugins/Process/gdb-remote/ProcessGDBRemote.h"
 #include "Plugins/Process/minidump/ProcessMinidump.h"
 #include "Plugins/ScriptInterpreter/None/ScriptInterpreterNone.h"
+#include "Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h"
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
@@ -176,6 +177,7 @@
   MainThreadCheckerRuntime::Initialize();
 
   SymbolVendorELF::Initialize();
+  breakpad::SymbolFileBreakpad::Initialize();
   SymbolFileDWARF::Initialize();
   SymbolFilePDB::Initialize();
   SymbolFileSymtab::Initialize();
@@ -274,6 +276,7 @@
   UndefinedBehaviorSanitizerRuntime::Terminate();
   MainThreadCheckerRuntime::Terminate();
   SymbolVendorELF::Terminate();
+  breakpad::SymbolFileBreakpad::Terminate();
   SymbolFileDWARF::Terminate();
   SymbolFilePDB::Terminate();
   SymbolFileSymtab::Terminate();
Index: source/API/SystemInitializerFull.cpp
===
--- source/API/SystemInitializerFull.cpp
+++ source/API/SystemInitializerFull.cpp
@@ -83,6 +83,7 @@
 #include "Plugins/Process/mach-core/ProcessMachCore.h"
 #include "Plugins/Process/minidump/ProcessMinidump.h"
 #include "Plugins/ScriptInterpreter/None/ScriptInterpreterNone.h"
+#include "Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h"
 #include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
@@ -343,6 +344,7 @@
   MainThreadCheckerRuntime::In

[Lldb-commits] [PATCH] D56589: Teach the default symbol vendor to respect module.GetSymbolFileFileSpec()

2019-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: clayborg, zturner, lemo.

Adding a breakpad symbol file to an existing MachO module with "target symbols
add" currently works only if one's host platform is a mac. This is
because SymbolVendorMacOSX (which is the one responsible for loading
symbols for MachO files) is conditionally compiled for the mac platform.

While we will sooner or later have a special symbol vendor for breakpad
files (to enable more advanced searching), and so this flow could be
made to work through that, it's not clear to me whether this should be a
requirement for the "target symbols add" flow to work. After all, since
the user has explicitly specified the symbol file to use, the symbol
vendor plugin's job is pretty much done.

This patch teaches the default symbol vendor to respect module's symbol
file spec, and load the symbol from that file if it is specified (and no
plugin requests any special handling).


https://reviews.llvm.org/D56589

Files:
  lit/SymbolFile/Breakpad/Inputs/basic-macho.yaml
  lit/SymbolFile/Breakpad/Inputs/symtab-macho.syms
  lit/SymbolFile/Breakpad/symtab-macho.test
  source/Symbol/SymbolVendor.cpp

Index: source/Symbol/SymbolVendor.cpp
===
--- source/Symbol/SymbolVendor.cpp
+++ source/Symbol/SymbolVendor.cpp
@@ -43,12 +43,19 @@
   }
   // The default implementation just tries to create debug information using
   // the file representation for the module.
-  instance_ap.reset(new SymbolVendor(module_sp));
-  if (instance_ap.get()) {
-ObjectFile *objfile = module_sp->GetObjectFile();
-if (objfile)
-  instance_ap->AddSymbolFileRepresentation(objfile->shared_from_this());
+  ObjectFileSP sym_objfile_sp;
+  FileSpec sym_spec = module_sp->GetSymbolFileFileSpec();
+  if (sym_spec && sym_spec != module_sp->GetObjectFile()->GetFileSpec()) {
+DataBufferSP data_sp;
+offset_t data_offset = 0;
+sym_objfile_sp = ObjectFile::FindPlugin(
+module_sp, &sym_spec, 0, FileSystem::Instance().GetByteSize(sym_spec),
+data_sp, data_offset);
   }
+  if (!sym_objfile_sp)
+sym_objfile_sp = module_sp->GetObjectFile()->shared_from_this();
+  instance_ap.reset(new SymbolVendor(module_sp));
+  instance_ap->AddSymbolFileRepresentation(sym_objfile_sp);
   return instance_ap.release();
 }
 
Index: lit/SymbolFile/Breakpad/symtab-macho.test
===
--- /dev/null
+++ lit/SymbolFile/Breakpad/symtab-macho.test
@@ -0,0 +1,21 @@
+# RUN: yaml2obj %S/Inputs/basic-macho.yaml > %T/symtab-macho.out
+# RUN: %lldb %T/symtab-macho.out -o "target symbols add -s symtab-macho.out %S/Inputs/symtab-macho.syms" \
+# RUN:   -s %s | FileCheck %s
+
+image dump symtab symtab-macho.out
+# CHECK-LABEL: (lldb) image dump symtab symtab-macho.out
+# CHECK: Symtab, file = {{.*}}symtab-macho.out, num_symbols = 1:
+# CHECK: Index   UserID DSX TypeFile Address/Value Load Address   Size   Flags  Name
+# CHECK: [0]  0   X Code0x00010ff00x0006 0x _start
+
+# CHECK-LABEL: (lldb) image lookup -a 0x10ff0 -v
+# CHECK: Address: symtab-macho.out[0x00010ff0] (symtab-macho.out.__TEXT.__text + 0)
+# CHECK: Symbol: id = {0x}, range = [0x00010ff0-0x00010ff6), name="_start"
+
+# CHECK-LABEL: (lldb) image lookup -n _start -v
+# CHECK: Address: symtab-macho.out[0x00010ff0] (symtab-macho.out.__TEXT.__text + 0)
+# CHECK: Symbol: id = {0x}, range = [0x00010ff0-0x00010ff6), name="_start"
+
+image lookup -a 0x10ff0 -v
+image lookup -n _start -v
+exit
Index: lit/SymbolFile/Breakpad/Inputs/symtab-macho.syms
===
--- /dev/null
+++ lit/SymbolFile/Breakpad/Inputs/symtab-macho.syms
@@ -0,0 +1,2 @@
+MODULE mac x86_64 601705B3B1227B7D39F9240E077D625B0 mac.out
+PUBLIC ff0 0 _start
Index: lit/SymbolFile/Breakpad/Inputs/basic-macho.yaml
===
--- /dev/null
+++ lit/SymbolFile/Breakpad/Inputs/basic-macho.yaml
@@ -0,0 +1,47 @@
+--- !mach-o
+FileHeader:  
+  magic:   0xFEEDFACF
+  cputype: 0x0107
+  cpusubtype:  0x0003
+  filetype:0x0002
+  ncmds:   9
+  sizeofcmds:  520
+  flags:   0x0085
+  reserved:0x
+LoadCommands:
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 152
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  4096
+fileoff: 0
+filesize:4096
+ma

[Lldb-commits] [PATCH] D56590: breakpad: Add FUNC records to the symtab

2019-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: zturner, lemo, clayborg, markmentovai.
Herald added a subscriber: aprantl.

This patch extends SymbolFileBreakpad::AddSymbols to include the symbols
from the FUNC records too. These symbols come from the debug info and
have a size associated with them, so they are given preference in case
there is a PUBLIC record for the same address.

To achieve this, I first pre-process the symbols into a temporary
DenseMap, and then insert the uniqued symbols into the module's symtab.


https://reviews.llvm.org/D56590

Files:
  lit/SymbolFile/Breakpad/Inputs/symtab.syms
  lit/SymbolFile/Breakpad/symtab.test
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -26,8 +26,9 @@
 class LineIterator {
 public:
   // begin iterator for sections of given type
-  LineIterator(ObjectFile &obj, ConstString section_type)
-  : m_obj(&obj), m_section_type(section_type), m_next_section_idx(0) {
+  LineIterator(ObjectFile &obj, Token section_type)
+  : m_obj(&obj), m_section_type(toString(section_type)),
+m_next_section_idx(0) {
 ++*this;
   }
 
@@ -77,7 +78,7 @@
 }
 
 static llvm::iterator_range lines(ObjectFile &obj,
-ConstString section_type) {
+Token section_type) {
   return llvm::make_range(LineIterator(obj, section_type), LineIterator(obj));
 }
 
@@ -181,10 +182,11 @@
   }
 
   const SectionList &list = *module.GetSectionList();
-  for (llvm::StringRef line : lines(*m_obj_file, ConstString("PUBLIC"))) {
-// PUBLIC [m] address param_size name
-// skip PUBLIC keyword
-line = getToken(line).second;
+  llvm::DenseMap symbols;
+
+  auto parse = [&](llvm::StringRef line, bool is_func) {
+// [m] address {size} param_size name
+// {size} is present in FUNC records.
 llvm::StringRef token;
 std::tie(token, line) = getToken(line);
 if (token == "m")
@@ -192,9 +194,18 @@
 
 addr_t address;
 if (!to_integer(token, address, 16))
-  continue;
+  return;
 address += base;
 
+addr_t size = 0;
+bool size_is_valid = false;
+if (is_func) {
+  std::tie(token, line) = getToken(line);
+  if (!to_integer(token, size, 16))
+return;
+  size_is_valid = true;
+}
+
 // skip param_size
 line = getToken(line).second;
 
@@ -206,19 +217,36 @@
"Ignoring symbol {0}, whose address ({1}) is outside of the "
"object file. Mismatched symbol file?",
name, address);
-  continue;
+  return;
 }
 
-symtab.AddSymbol(Symbol(
-/*symID*/ 0, Mangled(name, /*is_mangled*/ false), eSymbolTypeCode,
+symbols.try_emplace(
+address, /*symID*/ 0, Mangled(name, /*is_mangled*/ false),
+eSymbolTypeCode,
 /*is_global*/ true, /*is_debug*/ false, /*is_trampoline*/ false,
 /*is_artificial*/ false,
-AddressRange(section_sp, address - section_sp->GetFileAddress(), 0),
-/*size_is_valid*/ 0, /*contains_linker_annotations*/ false,
-/*flags*/ 0));
+AddressRange(section_sp, address - section_sp->GetFileAddress(), size),
+size_is_valid, /*contains_linker_annotations*/ false,
+/*flags*/ 0);
+  };
+  for (llvm::StringRef line: lines(*m_obj_file, Token::Func)) {
+// Here we can get either FUNC records (starting with FUNC), or line records
+// (starting with a hex number).
+llvm::StringRef token_str;
+std::tie(token_str, line) = getToken(line);
+if (toToken(token_str) != Token::Func)
+  continue; // Skip line records.
+
+parse(line, true);
   }
 
-  // TODO: Process FUNC records as well.
+  for (llvm::StringRef line : lines(*m_obj_file, Token::Public)) {
+// PUBLIC [m] address param_size name
+// skip PUBLIC keyword
+parse(getToken(line).second, false);
+  }
 
+  for (auto &KV : symbols)
+symtab.AddSymbol(std::move(KV.second));
   symtab.CalculateSymbolSizes();
 }
Index: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
===
--- source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
+++ source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
@@ -17,6 +17,11 @@
 namespace lldb_private {
 namespace breakpad {
 
+enum class Token { Unknown, Module, Info, File, Func, Public, Stack };
+
+Token toToken(llvm::StringRef str);
+llvm::StringRef toString(Token t);
+
 class ObjectFileBreakpad : public ObjectFile {
 public:
   //--
Index: source/Pl

[Lldb-commits] [PATCH] D56595: SymbolFileBreakpad: Add line table support

2019-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: clayborg, zturner, lemo, markmentovai.

This patch teaches SymbolFileBreakpad to parse the line information in
breakpad files and present it to lldb.

The trickiest question here was what kind of "compile units" to present
to lldb, as there really isn't enough information in breakpad files to
correctly reconstruct those.

The two options I considered were:

- have the entire file be one compile unit
- have one compile unit for each FILE record

The drawbacks of the first approach are that the compile unit created
this way will be huge, and there isn't a good way to name it (I decided
to name it after the object file).

The second approach will create mostly correct compile units for cpp
files, but it will still be wrong for headers. However, the biggest
drawback here seemed to be the fact that this can cause a compile unit
to change mid-function (for example when a function from another file is
inlined or another file is #included into a function). While I don't
know of any specific thing that would break in this case, it does sound
like a thing that we should avoid.

So, in the end, I chose the first approach, because it results in
simpler code, and having one compile unit doesn't seem so bad,
particularly as the second approach wouldn't result in correct compile
units either.


https://reviews.llvm.org/D56595

Files:
  include/lldb/Core/FileSpecList.h
  lit/SymbolFile/Breakpad/Inputs/line-table.syms
  lit/SymbolFile/Breakpad/line-table.test
  source/Core/FileSpecList.cpp
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h

Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
===
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
@@ -43,7 +43,7 @@
 
   uint32_t CalculateAbilities() override;
 
-  void InitializeObject() override {}
+  void InitializeObject() override;
 
   //--
   // Compile Unit function calls
@@ -67,9 +67,7 @@
   }
 
   bool ParseCompileUnitSupportFiles(const SymbolContext &sc,
-FileSpecList &support_files) override {
-return false;
-  }
+FileSpecList &support_files) override;
 
   bool
   ParseImportedModules(const SymbolContext &sc,
@@ -98,10 +96,16 @@
   }
 
   bool CompleteType(CompilerType &compiler_type) override { return false; }
+
   uint32_t ResolveSymbolContext(const Address &so_addr,
 lldb::SymbolContextItem resolve_scope,
 SymbolContext &sc) override;
 
+  uint32_t ResolveSymbolContext(const FileSpec &file_spec, uint32_t line,
+bool check_inlines,
+lldb::SymbolContextItem resolve_scope,
+SymbolContextList &sc_list) override;
+
   size_t GetTypes(SymbolContextScope *sc_scope, lldb::TypeClass type_mask,
   TypeList &type_list) override {
 return 0;
@@ -141,6 +145,7 @@
   uint32_t GetPluginVersion() override { return 1; }
 
 private:
+  lldb::CompUnitSP m_comp_unit_sp;
 };
 
 } // namespace breakpad
Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -13,7 +13,10 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Host/FileSystem.h"
+#include "lldb/Symbol/CompileUnit.h"
+#include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeMap.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/ADT/StringExtras.h"
@@ -103,17 +106,23 @@
   if (m_obj_file->GetPluginName() != ObjectFileBreakpad::GetPluginNameStatic())
 return 0;
 
-  return CompileUnits | Functions;
+  return CompileUnits | Functions | LineTables;
 }
 
-uint32_t SymbolFileBreakpad::GetNumCompileUnits() {
-  // TODO
-  return 0;
+void SymbolFileBreakpad::InitializeObject() {
+  m_comp_unit_sp = std::make_shared(
+  m_obj_file->GetModule(), /*user_data*/ nullptr,
+  m_obj_file->GetModule()->GetObjectFile()->GetFileSpec(),
+  /*uid*/ 0, eLanguageTypeUnknown, /*is_optimized*/ eLazyBoolNo);
 }
 
+uint32_t SymbolFileBreakpad::GetNumCompileUnits() { return 1; }
+
 CompUnitSP SymbolFileBreakpad::ParseCompileUnitAtIndex(uint32_t index) {
-  // TODO
-  return nullptr;
+  assert(index == 0);
+  m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(
+  0, m_comp_unit_sp);
+  return m_comp_unit_sp;
 }
 
 size_t SymbolFileBreakpad::ParseCompileUnitFunctions(const SymbolContext &sc) {
@@ -122,16 +131,131 @@
 }
 
 bool Sy

[Lldb-commits] [PATCH] D56595: SymbolFileBreakpad: Add line table support

2019-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath marked an inline comment as done.
labath added inline comments.



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:150-151
+line_table_up->AppendLineEntryToSequence(
+line_seq_up.get(), *next_addr, /*line*/ 0, /*column*/ 0,
+/*file_idx*/ 0, /*is_start_of_statement*/ false,
+/*is_start_of_basic_block*/ false, /*is_prologue_end*/ false,

Note that here i set `file=column=line=0` for the terminal entry, which isn't 
consistent with the dwarf plugin for instance (it puts there whatever falls out 
of the state automaton, which most likely means the values from the previous 
entry). AFAICT, this shouldn't be a problem, because the terminal entry is 
there to just determine the range of the last real entry.


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

https://reviews.llvm.org/D56595



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

[Lldb-commits] [PATCH] D56543: DWARF: Add some support for non-native directory separators

2019-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath updated this revision to Diff 181261.
labath added a comment.

- move the comp_dir resolution logic from SymbolFileDWARF to DWARFUnit
- this required the addition on an accessor to the "comp_dir_symlinks" setting, 
which is used in the full comp_dir resolution
- add FileSpec::MakeAbsolute() to simplify bits of code
- determine the path style from the DW_AT_name field if DW_AT_comp_dir is 
missing (I recall that it is possible to get a CU without a comp_dir in some 
circumstances, but I wasn't able to get my compiler to do that).


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

https://reviews.llvm.org/D56543

Files:
  include/lldb/Utility/FileSpec.h
  lit/SymbolFile/DWARF/Inputs/dir-separator-posix.lldbinit
  lit/SymbolFile/DWARF/Inputs/dir-separator-windows.lldbinit
  lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s
  lit/SymbolFile/DWARF/dir-separator-posix.s
  lit/SymbolFile/DWARF/dir-separator-windows.s
  source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Utility/FileSpec.cpp

Index: source/Utility/FileSpec.cpp
===
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -557,6 +557,11 @@
   return llvm::sys::path::is_absolute(current_path, m_style);
 }
 
+void FileSpec::MakeAbsolute(const FileSpec &dir) {
+  if (IsRelative())
+PrependPathComponent(dir);
+}
+
 void llvm::format_provider::format(const FileSpec &F,
  raw_ostream &Stream,
  StringRef Style) {
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -86,6 +86,8 @@
   static lldb_private::SymbolFile *
   CreateInstance(lldb_private::ObjectFile *obj_file);
 
+  static const lldb_private::FileSpecList &GetSymlinkPaths();
+
   //--
   // Constructors and Destructors
   //--
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -158,64 +158,8 @@
 
 } // anonymous namespace end
 
-static const char *removeHostnameFromPathname(const char *path_from_dwarf) {
-  if (!path_from_dwarf || !path_from_dwarf[0]) {
-return path_from_dwarf;
-  }
-
-  const char *colon_pos = strchr(path_from_dwarf, ':');
-  if (nullptr == colon_pos) {
-return path_from_dwarf;
-  }
-
-  const char *slash_pos = strchr(path_from_dwarf, '/');
-  if (slash_pos && (slash_pos < colon_pos)) {
-return path_from_dwarf;
-  }
-
-  // check whether we have a windows path, and so the first character is a
-  // drive-letter not a hostname.
-  if (colon_pos == path_from_dwarf + 1 && isalpha(*path_from_dwarf) &&
-  strlen(path_from_dwarf) > 2 && '\\' == path_from_dwarf[2]) {
-return path_from_dwarf;
-  }
-
-  return colon_pos + 1;
-}
-
-static FileSpec resolveCompDir(const char *path_from_dwarf) {
-  if (!path_from_dwarf)
-return FileSpec();
-
-  // DWARF2/3 suggests the form hostname:pathname for compilation directory.
-  // Remove the host part if present.
-  const char *local_path = removeHostnameFromPathname(path_from_dwarf);
-  if (!local_path)
-return FileSpec();
-
-  bool is_symlink = false;
-  // Always normalize our compile unit directory to get rid of redundant
-  // slashes and other path anomalies before we use it for path prepending
-  FileSpec local_spec(local_path);
-  const auto &file_specs = GetGlobalPluginProperties()->GetSymLinkPaths();
-  for (size_t i = 0; i < file_specs.GetSize() && !is_symlink; ++i)
-is_symlink = FileSpec::Equal(file_specs.GetFileSpecAtIndex(i),
- local_spec, true);
-
-  if (!is_symlink)
-return local_spec;
-
-  namespace fs = llvm::sys::fs;
-  if (fs::get_file_type(local_spec.GetPath(), false) !=
-  fs::file_type::symlink_file)
-return local_spec;
-
-  FileSpec resolved_symlink;
-  const auto error = FileSystem::Instance().Readlink(local_spec, resolved_symlink);
-  if (error.Success())
-return resolved_symlink;
-
-  return local_spec;
+const FileSpecList &SymbolFileDWARF::GetSymlinkPaths() {
+  return GetGlobalPluginProperties()->GetSymLinkPaths();
 }
 
 DWARFUnit *SymbolFileDWARF::GetBaseCompileUnit() {
@@ -810,17 +754,12 @@
 if (module_sp) {
   const DWARFDIE cu_die = dwarf_cu->DIE();
   if (cu_die) {
-Fil

[Lldb-commits] [PATCH] D56543: DWARF: Add some support for non-native directory separators

2019-01-11 Thread Pavel Labath via Phabricator via lldb-commits

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



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:775-787
+void DWARFUnit::ComputePathStyle() {
+  m_path_style = FileSpec::Style::native;
+  const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly();
+  if (!die)
+return;
+  llvm::StringRef comp_dir =
+  die->GetAttributeValueAsString(m_dwarf, this, DW_AT_comp_dir, NULL);

clayborg wrote:
> I would suggest adding a "FileSpec DWARFUnit::GetFileSpec()" and moving code 
> from SymbolFileDWARF::ParseCompileUnit into this function:
> ```
> FileSpec cu_file_spec(cu_die.GetName());
> if (cu_file_spec) {
>   // If we have a full path to the compile unit, we don't need to
>   // resolve the file.  This can be expensive e.g. when the source
>   // files are
>   // NFS mounted.
>   if (cu_file_spec.IsRelative()) {
> const char *cu_comp_dir{
> cu_die.GetAttributeValueAsString(DW_AT_comp_dir, 
> nullptr)};
> 
> cu_file_spec.PrependPathComponent(resolveCompDir(cu_comp_dir));
>   }
> ```
> 
> We might even cache the FileSpec object inside DWARFUnit so it doesn't have 
> to be recomputed? 
> 
> Then use this resolved path. DW_AT_comp_dir isn't always there, sometimes the 
> DW_AT_name is a full path only. So we should centralize this in DWARFUnit. We 
> might want to make a function like:
> 
> ```
> void DWARFUnit::ResolveCompileUnitDirectoryRelativeFile(FileSpec &spec);
> ```
> 
> And then have "FileSpec DWARFUnit::GetFileSpec()" call it. I was looking at 
> the other uses of DW_AT_comp_dir in the source, and the line tables tend to 
> need to resolve relative paths. A few other locations. Might be nice to get 
> the DWARFUnit and resolve the path using that? We can add "FileSpec 
> DWARFDie::GetPath()" accessor, could even add a "void 
> DWARFDie::ResolveRelativePath(FileSpec &)"
I think this version mostly implements what you had in mind. I added a 
`DWARFUnit::GetCompilationDirectory()` which returns the DW_AT_comp_dir 
attribute (I would expect that `DWARFUnit::GetFileSpec()` would return 
DW_AT_name, which is not useful for relative path resolution, except as a hint 
for the path style).

I've also added `FileSpec::MakeAbsolute` to shorten the `if(!spec.relative()) 
spec.prepend(dir))` pattern we had in some places. I didn't add any special 
FileSpec accessor to DWARFDie, as it didn't seem particularly useful (at this 
point). The only place where that's used now is for the computation of the name 
of the compile unit, and that's pretty concise now anyway.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:255
 
+  void ComputePathStyle();
+

zturner wrote:
> aprantl wrote:
> > I would probably call this DetectPathStyle() since it's more a heuristic?
> Maybe even `Guess` since `Compute` implies absolute certainty.
With all the other changes, this is now called 
`ComputeCompDirAndGuessPathStyle`. Note that this is just a private helper 
function and the public accessor is still called `GetPathStyle()`


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

https://reviews.llvm.org/D56543



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

[Lldb-commits] [PATCH] D56602: Move FileAction, ProcessInfo and ProcessLaunchInfo from Target to Host

2019-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: zturner, clayborg, jingham, davide, teemperor.
Herald added subscribers: mgorny, emaste.

These classes describe the details of the process we are about to
launch, and so they are naturally used by the launching code in the Host
module. Previously they were present in Target because that is the most
important (but by far not the only) user of the launching code.

Since the launching code has other customers, must of which do not care
about Targets, it makes sense to move these classes to the Host layer,
next to the launching code.

This move reduces the number of times that Target is included from host
to 8 (it used to be 14).


https://reviews.llvm.org/D56602

Files:
  include/lldb/Host/FileAction.h
  include/lldb/Host/ProcessInfo.h
  include/lldb/Host/ProcessLaunchInfo.h
  include/lldb/Target/FileAction.h
  include/lldb/Target/Process.h
  include/lldb/Target/ProcessInfo.h
  include/lldb/Target/ProcessLaunchInfo.h
  include/lldb/Target/Target.h
  include/lldb/module.modulemap
  source/API/SBLaunchInfo.cpp
  source/Host/CMakeLists.txt
  source/Host/common/FileAction.cpp
  source/Host/common/Host.cpp
  source/Host/common/MonitoringProcessLauncher.cpp
  source/Host/common/ProcessInfo.cpp
  source/Host/common/ProcessLaunchInfo.cpp
  source/Host/macosx/objcxx/Host.mm
  source/Host/posix/ProcessLauncherPosixFork.cpp
  source/Host/windows/ProcessLauncherWindows.cpp
  
source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
  source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  source/Plugins/Process/Linux/NativeProcessLinux.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  source/Target/CMakeLists.txt
  source/Target/FileAction.cpp
  source/Target/ProcessInfo.cpp
  source/Target/ProcessLaunchInfo.cpp
  unittests/Host/CMakeLists.txt
  unittests/Host/FileActionTest.cpp
  unittests/Host/ProcessInfoTest.cpp
  unittests/Host/ProcessLaunchInfoTest.cpp
  unittests/tools/lldb-server/tests/TestClient.cpp
  unittests/tools/lldb-server/tests/TestClient.h

Index: unittests/tools/lldb-server/tests/TestClient.h
===
--- unittests/tools/lldb-server/tests/TestClient.h
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -12,7 +12,7 @@
 
 #include "MessageObjects.h"
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
-#include "lldb/Target/ProcessLaunchInfo.h"
+#include "lldb/Host/ProcessLaunchInfo.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Connection.h"
 #include "llvm/ADT/Optional.h"
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -11,7 +11,6 @@
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
-#include "lldb/Target/ProcessLaunchInfo.h"
 #include "lldb/Utility/Args.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Path.h"
Index: unittests/Host/ProcessLaunchInfoTest.cpp
===
--- /dev/null
+++ unittests/Host/ProcessLaunchInfoTest.cpp
@@ -0,0 +1,28 @@
+//===-- ProcessLaunchInfoTest.cpp ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Host/ProcessLaunchInfo.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+TEST(ProcessLaunchInfoTest, Constructor) {
+  ProcessLaunchInfo Info(FileSpec("/stdin"), FileSpec("/stdout"),
+ FileSpec("/stderr"), FileSpec("/wd"),
+ eLaunchFlagStopAtEntry);
+  EXPECT_EQ(FileSpec("/stdin"),
+Info.GetFileActionForFD(STDIN_FILENO)->GetFileSpec());
+  EXPECT_EQ(FileSpec("/stdout"),
+Info.GetFileActionForFD(STDOUT_FILENO)->GetFileSpec());
+  EXPECT_EQ(FileSpec("/stderr"),
+Info.GetFileActionForFD(STDERR_FILENO)->GetFileSpec());
+  EXPECT_EQ(FileSpec("/wd"), Info.GetWorkingDirectory());
+  EXPECT_EQ(eLaunchFlagStopAtEntry, Info.GetFlags().Get());
+}
Index: unittests/Host/ProcessInfoTest.cpp
===
--- /dev/null
+++ unittests/Host/ProcessInfoTest.cpp
@@ -0,0 +1,20 @@
+//===-- ProcessInfoTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Il

[Lldb-commits] [PATCH] D56606: [CMake] Export utility targets to the build/install tree depending on LLVM_BUILD/INSTALL_UTILS

2019-01-11 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz created this revision.
sgraenitz added reviewers: chapuni, gottesmm, beanz.
Herald added a subscriber: mgorny.

Allow external projects to import test-related targets like FileCheck, count, 
not etc. and query binary paths, properties, etc.
This would be useful for LLDB, because it reduces the difference between 
in-tree vs. standalone builds and simplifies CMake logic.


Repository:
  rL LLVM

https://reviews.llvm.org/D56606

Files:
  cmake/modules/AddLLVM.cmake


Index: cmake/modules/AddLLVM.cmake
===
--- cmake/modules/AddLLVM.cmake
+++ cmake/modules/AddLLVM.cmake
@@ -920,6 +920,9 @@
DEPENDS ${name}
COMPONENT ${name})
 endif()
+set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
+  elseif( LLVM_BUILD_UTILS )
+set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS_BUILDTREE_ONLY ${name})
   endif()
 endmacro(add_llvm_utility name)
 


Index: cmake/modules/AddLLVM.cmake
===
--- cmake/modules/AddLLVM.cmake
+++ cmake/modules/AddLLVM.cmake
@@ -920,6 +920,9 @@
DEPENDS ${name}
COMPONENT ${name})
 endif()
+set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
+  elseif( LLVM_BUILD_UTILS )
+set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS_BUILDTREE_ONLY ${name})
   endif()
 endmacro(add_llvm_utility name)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56400: [CMake] Some cleanup around test preparations

2019-01-11 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz abandoned this revision.
sgraenitz added a comment.

I will split off the "dead code elimination" part and close both reviews in 
favor of a more comprehensive fix.




Comment at: CMakeLists.txt:49
   set(LLDB_DEFAULT_TEST_FILECHECK 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/FileCheck${CMAKE_EXECUTABLE_SUFFIX}")
-
-  if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER AND TARGET clang)
-set(LLDB_DEFAULT_TEST_C_COMPILER 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
-  else()
-set(LLDB_DEFAULT_TEST_C_COMPILER "")
-  endif()
-
-  if (NOT LLDB_TEST_USE_CUSTOM_CXX_COMPILER AND TARGET clang)
-set(LLDB_DEFAULT_TEST_CXX_COMPILER 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang++${CMAKE_EXECUTABLE_SUFFIX}")
-  else()
-set(LLDB_DEFAULT_TEST_CXX_COMPILER "")
+  if(TARGET clang)
+set(LLDB_DEFAULT_TEST_COMPILER 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")

stella.stamenova wrote:
> Nit: it looks like your if statements lack a space before the parenthesis, 
> while all the existing ones have a space. Please be consistent with the 
> existing convention.
That doesn't seem to be the most popular convention ;-)
BTW do we have documentation for CMake coding style?


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

https://reviews.llvm.org/D56400



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

[Lldb-commits] [PATCH] D56609: [CMake] Remove dead code and outdated comments

2019-01-11 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz created this revision.
sgraenitz added reviewers: stella.stamenova, labath, JDevlieghere.
Herald added a subscriber: mgorny.

All of these changes are NOPs.


https://reviews.llvm.org/D56609

Files:
  CMakeLists.txt
  cmake/modules/LLDBStandalone.cmake
  lit/CMakeLists.txt


Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -11,14 +11,6 @@
   set(LLDB_IS_64_BITS 1)
 endif()
 
-if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER 
${LLDB_TEST_C_COMPILER})
-endif ()
-
-if (NOT LLDB_TEST_USE_CUSTOM_CXX_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_CXX_COMPILER 
${LLDB_TEST_CXX_COMPILER})
-endif ()
-
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR})
@@ -38,17 +30,8 @@
 
 if(TARGET lld)
   list(APPEND LLDB_TEST_DEPS lld)
-  set(LLDB_HAVE_LLD 1)
-else()
-  set(LLDB_HAVE_LLD 0)
 endif()
 
-if(BUILD_SHARED_LIBS)
-  set(ENABLE_SHARED 1)
-else()
-  set(ENABLE_SHARED 0)
-endif(BUILD_SHARED_LIBS)
-
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
   LLDB_DISABLE_PYTHON
Index: cmake/modules/LLDBStandalone.cmake
===
--- cmake/modules/LLDBStandalone.cmake
+++ cmake/modules/LLDBStandalone.cmake
@@ -101,7 +101,6 @@
 
   # Import CMake library targets from LLVM and Clang.
   
include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm/LLVMConfig.cmake")
-  # cmake/clang/ClangConfig.cmake is not created when LLVM and Clang are built 
together.
   if (EXISTS 
"${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
 
include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
   endif()
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -41,13 +41,10 @@
 option(LLDB_TEST_USE_CUSTOM_CXX_COMPILER "Use the C++ compiler provided via 
LLDB_TEST_CXX_COMPILER for building test inferiors (instead of the just-built 
compiler). Defaults to OFF." OFF)
 if(LLDB_INCLUDE_TESTS)
 
-  # Set the path to the default lldb test executable. Make the path relative to
-  # LLVM_RUNTIME_OUTPUT_INTDIR: this will be correct even when LLVM and LLDB
-  # have separate binary directories.
+  # Set the path to the default lldb test executable.
   set(LLDB_DEFAULT_TEST_EXECUTABLE 
"${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}")
 
-  # Set the paths to default llvm tools. Make these paths relative to the LLVM
-  # binary directory.
+  # Set the paths to default llvm tools.
   set(LLDB_DEFAULT_TEST_DSYMUTIL 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/dsymutil${CMAKE_EXECUTABLE_SUFFIX}")
   set(LLDB_DEFAULT_TEST_FILECHECK 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/FileCheck${CMAKE_EXECUTABLE_SUFFIX}")
 


Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -11,14 +11,6 @@
   set(LLDB_IS_64_BITS 1)
 endif()
 
-if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER ${LLDB_TEST_C_COMPILER})
-endif ()
-
-if (NOT LLDB_TEST_USE_CUSTOM_CXX_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_CXX_COMPILER ${LLDB_TEST_CXX_COMPILER})
-endif ()
-
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
@@ -38,17 +30,8 @@
 
 if(TARGET lld)
   list(APPEND LLDB_TEST_DEPS lld)
-  set(LLDB_HAVE_LLD 1)
-else()
-  set(LLDB_HAVE_LLD 0)
 endif()
 
-if(BUILD_SHARED_LIBS)
-  set(ENABLE_SHARED 1)
-else()
-  set(ENABLE_SHARED 0)
-endif(BUILD_SHARED_LIBS)
-
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
   LLDB_DISABLE_PYTHON
Index: cmake/modules/LLDBStandalone.cmake
===
--- cmake/modules/LLDBStandalone.cmake
+++ cmake/modules/LLDBStandalone.cmake
@@ -101,7 +101,6 @@
 
   # Import CMake library targets from LLVM and Clang.
   include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm/LLVMConfig.cmake")
-  # cmake/clang/ClangConfig.cmake is not created when LLVM and Clang are built together.
   if (EXISTS "${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
 include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
   endif()
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -41,13 +41,10 @@
 option(LLDB_TEST_USE_CUSTOM_CXX_COMPILER "Use the C++ compiler provided via LLDB_TEST_CXX_COMPILER for building tes

[Lldb-commits] [PATCH] D56609: [CMake] Remove dead code and outdated comments

2019-01-11 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz marked 2 inline comments as done.
sgraenitz added a comment.

Simple cleanup parts of the previous reviews D56400 
 and D56440 




Comment at: lit/CMakeLists.txt:21
-endif ()
-
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)

This only changes the value of `LLDB_TEST_C/CXX_COMPILER` in the scope this 
file. It is unused since https://reviews.llvm.org/rC347216#change-H2HV4zA8ol05 
removed their usage from `lit/lit.site.cfg.py.in` (which gets configured 
below). Before this change the local value was passed to the config file like 
this:

```
config.cc = "@LLDB_TEST_C_COMPILER@"
config.cxx = "@LLDB_TEST_CXX_COMPILER@"
```

They seem to be inferred via `LLDB_LIT_TOOLS_DIR` now.




Comment at: lit/CMakeLists.txt:51
-endif(BUILD_SHARED_LIBS)
-
 # the value is not canonicalized within LLVM

`LLDB_HAVE_LLD` was only used between r331447 and r347216.
`ENABLE_SHARED` was only used between r232205 and r232227.


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

https://reviews.llvm.org/D56609



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

[Lldb-commits] [lldb] r350937 - [CMake] Remove dead code and outdated comments

2019-01-11 Thread Stefan Granitz via lldb-commits

Author: stefan.graenitz
Date: Fri Jan 11 09:51:33 2019
New Revision: 350937

URL: http://llvm.org/viewvc/llvm-project?rev=350937&view=rev
Log:
[CMake] Remove dead code and outdated comments

Summary: All of these changes are NOPs.

Reviewers: stella.stamenova, labath, JDevlieghere

Reviewed By: stella.stamenova

Subscribers: mgorny, lldb-commits, #lldb

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

Modified:
lldb/trunk/CMakeLists.txt
lldb/trunk/cmake/modules/LLDBStandalone.cmake
lldb/trunk/lit/CMakeLists.txt

Modified: lldb/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=350937&r1=350936&r2=350937&view=diff
==
--- lldb/trunk/CMakeLists.txt (original)
+++ lldb/trunk/CMakeLists.txt Fri Jan 11 09:51:33 2019
@@ -41,13 +41,10 @@ option(LLDB_TEST_USE_CUSTOM_C_COMPILER "
 option(LLDB_TEST_USE_CUSTOM_CXX_COMPILER "Use the C++ compiler provided via 
LLDB_TEST_CXX_COMPILER for building test inferiors (instead of the just-built 
compiler). Defaults to OFF." OFF)
 if(LLDB_INCLUDE_TESTS)
 
-  # Set the path to the default lldb test executable. Make the path relative to
-  # LLVM_RUNTIME_OUTPUT_INTDIR: this will be correct even when LLVM and LLDB
-  # have separate binary directories.
+  # Set the path to the default lldb test executable.
   set(LLDB_DEFAULT_TEST_EXECUTABLE 
"${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}")
 
-  # Set the paths to default llvm tools. Make these paths relative to the LLVM
-  # binary directory.
+  # Set the paths to default llvm tools.
   set(LLDB_DEFAULT_TEST_DSYMUTIL 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/dsymutil${CMAKE_EXECUTABLE_SUFFIX}")
   set(LLDB_DEFAULT_TEST_FILECHECK 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/FileCheck${CMAKE_EXECUTABLE_SUFFIX}")
 

Modified: lldb/trunk/cmake/modules/LLDBStandalone.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBStandalone.cmake?rev=350937&r1=350936&r2=350937&view=diff
==
--- lldb/trunk/cmake/modules/LLDBStandalone.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBStandalone.cmake Fri Jan 11 09:51:33 2019
@@ -101,7 +101,6 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
 
   # Import CMake library targets from LLVM and Clang.
   
include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm/LLVMConfig.cmake")
-  # cmake/clang/ClangConfig.cmake is not created when LLVM and Clang are built 
together.
   if (EXISTS 
"${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
 
include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
   endif()

Modified: lldb/trunk/lit/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/CMakeLists.txt?rev=350937&r1=350936&r2=350937&view=diff
==
--- lldb/trunk/lit/CMakeLists.txt (original)
+++ lldb/trunk/lit/CMakeLists.txt Fri Jan 11 09:51:33 2019
@@ -11,14 +11,6 @@ if (CMAKE_SIZEOF_VOID_P EQUAL 8)
   set(LLDB_IS_64_BITS 1)
 endif()
 
-if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER 
${LLDB_TEST_C_COMPILER})
-endif ()
-
-if (NOT LLDB_TEST_USE_CUSTOM_CXX_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_CXX_COMPILER 
${LLDB_TEST_CXX_COMPILER})
-endif ()
-
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR})
@@ -38,17 +30,8 @@ list(APPEND LLDB_TEST_DEPS
 
 if(TARGET lld)
   list(APPEND LLDB_TEST_DEPS lld)
-  set(LLDB_HAVE_LLD 1)
-else()
-  set(LLDB_HAVE_LLD 0)
 endif()
 
-if(BUILD_SHARED_LIBS)
-  set(ENABLE_SHARED 1)
-else()
-  set(ENABLE_SHARED 0)
-endif(BUILD_SHARED_LIBS)
-
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
   LLDB_DISABLE_PYTHON


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

[Lldb-commits] [PATCH] D56609: [CMake] Remove dead code and outdated comments

2019-01-11 Thread Phabricator via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350937: [CMake] Remove dead code and outdated comments 
(authored by stefan.graenitz, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56609

Files:
  lldb/trunk/CMakeLists.txt
  lldb/trunk/cmake/modules/LLDBStandalone.cmake
  lldb/trunk/lit/CMakeLists.txt


Index: lldb/trunk/cmake/modules/LLDBStandalone.cmake
===
--- lldb/trunk/cmake/modules/LLDBStandalone.cmake
+++ lldb/trunk/cmake/modules/LLDBStandalone.cmake
@@ -101,7 +101,6 @@
 
   # Import CMake library targets from LLVM and Clang.
   
include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm/LLVMConfig.cmake")
-  # cmake/clang/ClangConfig.cmake is not created when LLVM and Clang are built 
together.
   if (EXISTS 
"${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
 
include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
   endif()
Index: lldb/trunk/CMakeLists.txt
===
--- lldb/trunk/CMakeLists.txt
+++ lldb/trunk/CMakeLists.txt
@@ -41,13 +41,10 @@
 option(LLDB_TEST_USE_CUSTOM_CXX_COMPILER "Use the C++ compiler provided via 
LLDB_TEST_CXX_COMPILER for building test inferiors (instead of the just-built 
compiler). Defaults to OFF." OFF)
 if(LLDB_INCLUDE_TESTS)
 
-  # Set the path to the default lldb test executable. Make the path relative to
-  # LLVM_RUNTIME_OUTPUT_INTDIR: this will be correct even when LLVM and LLDB
-  # have separate binary directories.
+  # Set the path to the default lldb test executable.
   set(LLDB_DEFAULT_TEST_EXECUTABLE 
"${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}")
 
-  # Set the paths to default llvm tools. Make these paths relative to the LLVM
-  # binary directory.
+  # Set the paths to default llvm tools.
   set(LLDB_DEFAULT_TEST_DSYMUTIL 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/dsymutil${CMAKE_EXECUTABLE_SUFFIX}")
   set(LLDB_DEFAULT_TEST_FILECHECK 
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/FileCheck${CMAKE_EXECUTABLE_SUFFIX}")
 
Index: lldb/trunk/lit/CMakeLists.txt
===
--- lldb/trunk/lit/CMakeLists.txt
+++ lldb/trunk/lit/CMakeLists.txt
@@ -11,14 +11,6 @@
   set(LLDB_IS_64_BITS 1)
 endif()
 
-if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER 
${LLDB_TEST_C_COMPILER})
-endif ()
-
-if (NOT LLDB_TEST_USE_CUSTOM_CXX_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_CXX_COMPILER 
${LLDB_TEST_CXX_COMPILER})
-endif ()
-
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR})
@@ -38,17 +30,8 @@
 
 if(TARGET lld)
   list(APPEND LLDB_TEST_DEPS lld)
-  set(LLDB_HAVE_LLD 1)
-else()
-  set(LLDB_HAVE_LLD 0)
 endif()
 
-if(BUILD_SHARED_LIBS)
-  set(ENABLE_SHARED 1)
-else()
-  set(ENABLE_SHARED 0)
-endif(BUILD_SHARED_LIBS)
-
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
   LLDB_DISABLE_PYTHON


Index: lldb/trunk/cmake/modules/LLDBStandalone.cmake
===
--- lldb/trunk/cmake/modules/LLDBStandalone.cmake
+++ lldb/trunk/cmake/modules/LLDBStandalone.cmake
@@ -101,7 +101,6 @@
 
   # Import CMake library targets from LLVM and Clang.
   include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm/LLVMConfig.cmake")
-  # cmake/clang/ClangConfig.cmake is not created when LLVM and Clang are built together.
   if (EXISTS "${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
 include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
   endif()
Index: lldb/trunk/CMakeLists.txt
===
--- lldb/trunk/CMakeLists.txt
+++ lldb/trunk/CMakeLists.txt
@@ -41,13 +41,10 @@
 option(LLDB_TEST_USE_CUSTOM_CXX_COMPILER "Use the C++ compiler provided via LLDB_TEST_CXX_COMPILER for building test inferiors (instead of the just-built compiler). Defaults to OFF." OFF)
 if(LLDB_INCLUDE_TESTS)
 
-  # Set the path to the default lldb test executable. Make the path relative to
-  # LLVM_RUNTIME_OUTPUT_INTDIR: this will be correct even when LLVM and LLDB
-  # have separate binary directories.
+  # Set the path to the default lldb test executable.
   set(LLDB_DEFAULT_TEST_EXECUTABLE "${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}")
 
-  # Set the paths to default llvm tools. Make these paths relative to the LLVM
-  # binary directory.
+  # Set the paths to default llvm tools.
   set(LLDB_DEFAULT_TEST_DSYMUTIL "${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/dsymutil${

lldb-commits@lists.llvm.org

2019-01-11 Thread Zachary Turner via lldb-commits

Author: zturner
Date: Fri Jan 11 10:03:20 2019
New Revision: 350943

URL: http://llvm.org/viewvc/llvm-project?rev=350943&view=rev
Log:
[SymbolFile] Make ParseCompileUnitXXX accept a CompileUnit&.

Previously all of these functions accepted a SymbolContext&.
While a CompileUnit is one member of a SymbolContext, there
are also many others, and by passing such a monolithic parameter
in this way it makes the requirements and assumptions of the
API unclear for both callers as well as implementors.

All these methods need is a CompileUnit.  By limiting the
parameter type in this way, we simplify the code as well as
make it self-documenting for both implementers and users.

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

Modified:
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/include/lldb/Symbol/SymbolVendor.h
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/SearchFilter.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
lldb/trunk/source/Symbol/CompileUnit.cpp
lldb/trunk/source/Symbol/SymbolVendor.cpp

Modified: lldb/trunk/include/lldb/Symbol/SymbolFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolFile.h?rev=350943&r1=350942&r2=350943&view=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolFile.h Fri Jan 11 10:03:20 2019
@@ -125,22 +125,19 @@ public:
   virtual uint32_t GetNumCompileUnits() = 0;
   virtual lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) = 0;
 
-  virtual lldb::LanguageType
-  ParseCompileUnitLanguage(const SymbolContext &sc) = 0;
-  virtual size_t ParseCompileUnitFunctions(const SymbolContext &sc) = 0;
-  virtual bool ParseCompileUnitLineTable(const SymbolContext &sc) = 0;
-  virtual bool ParseCompileUnitDebugMacros(const SymbolContext &sc) = 0;
-  virtual bool ParseCompileUnitSupportFiles(const SymbolContext &sc,
-FileSpecList &support_files) = 0;
-  virtual bool
-  ParseCompileUnitIsOptimized(const lldb_private::SymbolContext &sc) {
-return false;
-  }
+  virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
+  virtual size_t ParseFunctions(CompileUnit &comp_unit) = 0;
+  virtual bool ParseLineTable(CompileUnit &comp_unit) = 0;
+  virtual bool ParseDebugMacros(CompileUnit &comp_unit) = 0;
+  virtual bool ParseSupportFiles(CompileUnit &comp_unit,
+ FileSpecList &support_files) = 0;
+  virtual size_t ParseTypes(CompileUnit &comp_unit) = 0;
+  virtual bool ParseIsOptimized(CompileUnit &comp_unit) { return false; }
+
   virtual bool
   ParseImportedModules(const SymbolContext &sc,
std::vector &imported_modules) = 0;
   virtual size_t ParseFunctionBlocks(const SymbolContext &sc) = 0;
-  virtual size_t ParseTypesForCompileUnit(CompileUnit &comp_unit) = 0;
   virtual size_t ParseVariablesForContext(const SymbolContext &sc) = 0;
   virtual Type *ResolveTypeUID(lldb::user_id_t type_uid) = 0;
 

Modified: lldb/trunk/include/lldb/Symbol/SymbolVendor.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolVendor.h?rev=350943&r1=350942&r2=350943&view=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolVendor.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolVendor.h Fri Jan 11 10:03:20 2019
@@ -46,26 +46,26 @@ public:
 
   virtual void Dump(Stream *s);
 
-  virtual lldb::LanguageType ParseCompileUnitLanguage(const SymbolContext &sc);
+  virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit);
 
-  virtual size_t ParseCompileUnitFunctions(const SymbolContext &sc);
+  virtual size_t ParseFunctions(CompileUnit &comp_unit);
 
-  virtual bool ParseCompileUnitLineTable(const SymbolContext &sc);
+  virtual bool ParseLineTable(CompileUnit &comp_unit);
 
-  virtual bool ParseCompileUnitDebugMacros(const SymbolContext &sc);
+  virtual bool ParseDebugMacros(CompileUnit &comp_unit);
 
-  virtual bool ParseCompileUnitSupportFiles(const Symbo

lldb-commits@lists.llvm.org

2019-01-11 Thread Zachary Turner via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB350943: [SymbolFile] Make ParseCompileUnitXXX accept a 
CompileUnit&. (authored by zturner, committed by ).
Herald added subscribers: teemperor, abidh.

Changed prior to commit:
  https://reviews.llvm.org/D56564?vs=181166&id=181307#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56564

Files:
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/SymbolVendor.h
  source/Core/Module.cpp
  source/Core/SearchFilter.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  source/Symbol/CompileUnit.cpp
  source/Symbol/SymbolVendor.cpp

Index: include/lldb/Symbol/SymbolVendor.h
===
--- include/lldb/Symbol/SymbolVendor.h
+++ include/lldb/Symbol/SymbolVendor.h
@@ -46,26 +46,26 @@
 
   virtual void Dump(Stream *s);
 
-  virtual lldb::LanguageType ParseCompileUnitLanguage(const SymbolContext &sc);
+  virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit);
 
-  virtual size_t ParseCompileUnitFunctions(const SymbolContext &sc);
+  virtual size_t ParseFunctions(CompileUnit &comp_unit);
 
-  virtual bool ParseCompileUnitLineTable(const SymbolContext &sc);
+  virtual bool ParseLineTable(CompileUnit &comp_unit);
 
-  virtual bool ParseCompileUnitDebugMacros(const SymbolContext &sc);
+  virtual bool ParseDebugMacros(CompileUnit &comp_unit);
 
-  virtual bool ParseCompileUnitSupportFiles(const SymbolContext &sc,
-FileSpecList &support_files);
+  virtual bool ParseSupportFiles(CompileUnit &comp_unit,
+ FileSpecList &support_files);
 
-  virtual bool ParseCompileUnitIsOptimized(const SymbolContext &sc);
+  virtual bool ParseIsOptimized(CompileUnit &comp_unit);
+
+  virtual size_t ParseTypes(CompileUnit &comp_unit);
 
   virtual bool ParseImportedModules(const SymbolContext &sc,
 std::vector &imported_modules);
 
   virtual size_t ParseFunctionBlocks(const SymbolContext &sc);
 
-  virtual size_t ParseTypesForCompileUnit(CompileUnit &comp_unit);
-
   virtual size_t ParseVariablesForContext(const SymbolContext &sc);
 
   virtual Type *ResolveTypeUID(lldb::user_id_t type_uid);
Index: include/lldb/Symbol/SymbolFile.h
===
--- include/lldb/Symbol/SymbolFile.h
+++ include/lldb/Symbol/SymbolFile.h
@@ -125,22 +125,19 @@
   virtual uint32_t GetNumCompileUnits() = 0;
   virtual lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) = 0;
 
-  virtual lldb::LanguageType
-  ParseCompileUnitLanguage(const SymbolContext &sc) = 0;
-  virtual size_t ParseCompileUnitFunctions(const SymbolContext &sc) = 0;
-  virtual bool ParseCompileUnitLineTable(const SymbolContext &sc) = 0;
-  virtual bool ParseCompileUnitDebugMacros(const SymbolContext &sc) = 0;
-  virtual bool ParseCompileUnitSupportFiles(const SymbolContext &sc,
-FileSpecList &support_files) = 0;
-  virtual bool
-  ParseCompileUnitIsOptimized(const lldb_private::SymbolContext &sc) {
-return false;
-  }
+  virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
+  virtual size_t ParseFunctions(CompileUnit &comp_unit) = 0;
+  virtual bool ParseLineTable(CompileUnit &comp_unit) = 0;
+  virtual bool ParseDebugMacros(CompileUnit &comp_unit) = 0;
+  virtual bool ParseSupportFiles(CompileUnit &comp_unit,
+ FileSpecList &support_files) = 0;
+  virtual size_t ParseTypes(CompileUnit &comp_unit) = 0;
+  virtual bool ParseIsOptimized(CompileUnit &comp_unit) { return false; }
+
   virtual bool
   ParseImportedModules(const SymbolContext &sc,
std::vector &imported_modules) = 0;
   virtual size_t ParseFunctionBlocks(const SymbolContext &sc) = 0;
-  virtual size_t ParseTypesForCompileUnit(CompileUnit &comp_unit) = 0;
   virtual size_t ParseVariablesForContext(const SymbolContext &sc) = 0;
   virtual Type *ResolveTypeUID(lldb::user_id_t type_uid) = 0;
 
Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -365,23 +3

[Lldb-commits] [lldb] r350945 - [CMake] Include tests by default also in standalone builds

2019-01-11 Thread Stefan Granitz via lldb-commits

Author: stefan.graenitz
Date: Fri Jan 11 10:11:04 2019
New Revision: 350945

URL: http://llvm.org/viewvc/llvm-project?rev=350945&view=rev
Log:
[CMake] Include tests by default also in standalone builds

In-tree builds include tests by default. Standalone builds should behave the 
same.

Modified:
lldb/trunk/CMakeLists.txt
lldb/trunk/cmake/modules/LLDBStandalone.cmake

Modified: lldb/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=350945&r1=350944&r2=350945&view=diff
==
--- lldb/trunk/CMakeLists.txt (original)
+++ lldb/trunk/CMakeLists.txt Fri Jan 11 10:11:04 2019
@@ -35,8 +35,7 @@ endif ()
 add_subdirectory(source)
 add_subdirectory(tools)
 
-option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests."
-  ${LLVM_INCLUDE_TESTS})
+option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." 
${LLVM_INCLUDE_TESTS})
 option(LLDB_TEST_USE_CUSTOM_C_COMPILER "Use the C compiler provided via 
LLDB_TEST_C_COMPILER for building test inferiors (instead of the just-built 
compiler). Defaults to OFF." OFF)
 option(LLDB_TEST_USE_CUSTOM_CXX_COMPILER "Use the C++ compiler provided via 
LLDB_TEST_CXX_COMPILER for building test inferiors (instead of the just-built 
compiler). Defaults to OFF." OFF)
 if(LLDB_INCLUDE_TESTS)

Modified: lldb/trunk/cmake/modules/LLDBStandalone.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBStandalone.cmake?rev=350945&r1=350944&r2=350945&view=diff
==
--- lldb/trunk/cmake/modules/LLDBStandalone.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBStandalone.cmake Fri Jan 11 10:11:04 2019
@@ -106,6 +106,7 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
   endif()
 
   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
+  set(LLVM_INCLUDE_TESTS ON CACHE INTERNAL "")
 
   set(CMAKE_INCLUDE_CURRENT_DIR ON)
   include_directories("${CMAKE_BINARY_DIR}/include" "${LLVM_MAIN_INCLUDE_DIR}")


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

[Lldb-commits] [lldb] r350947 - Attempt to fix PDB tests broken by r350924

2019-01-11 Thread Pavel Labath via lldb-commits

Author: labath
Date: Fri Jan 11 10:24:17 2019
New Revision: 350947

URL: http://llvm.org/viewvc/llvm-project?rev=350947&view=rev
Log:
Attempt to fix PDB tests broken by r350924

The patch added the symbol plugin name to the lldb-test output. Update
the tests to account for that.

Modified:
lldb/trunk/lit/SymbolFile/PDB/class-layout.test
lldb/trunk/lit/SymbolFile/PDB/compilands.test
lldb/trunk/lit/SymbolFile/PDB/func-symbols.test
lldb/trunk/lit/SymbolFile/PDB/type-quals.test
lldb/trunk/lit/SymbolFile/PDB/typedefs.test
lldb/trunk/lit/SymbolFile/PDB/variables.test

Modified: lldb/trunk/lit/SymbolFile/PDB/class-layout.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/class-layout.test?rev=350947&r1=350946&r2=350947&view=diff
==
--- lldb/trunk/lit/SymbolFile/PDB/class-layout.test (original)
+++ lldb/trunk/lit/SymbolFile/PDB/class-layout.test Fri Jan 11 10:24:17 2019
@@ -13,7 +13,7 @@ RUN: lldb-test symbols %T/ClassLayoutTes
 RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck 
--check-prefix=CLASS %s
 
 CHECK: Module [[MOD:.*]]
-CHECK: {{^[0-9A-F]+}}: SymbolVendor ([[MOD]])
+CHECK: {{^[0-9A-F]+}}: SymbolVendor pdb ([[MOD]])
 CHECK: {{^[0-9A-F]+}}:   CompileUnit{{[{]0x[0-9a-f]+[}]}}, language = "c++", 
file = '{{.*}}\ClassLayoutTest.cpp'
 
 ENUM:  name = "Enum", size = 4,  decl = ClassLayoutTest.cpp:5

Modified: lldb/trunk/lit/SymbolFile/PDB/compilands.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/compilands.test?rev=350947&r1=350946&r2=350947&view=diff
==
--- lldb/trunk/lit/SymbolFile/PDB/compilands.test (original)
+++ lldb/trunk/lit/SymbolFile/PDB/compilands.test Fri Jan 11 10:24:17 2019
@@ -7,5 +7,5 @@ RUN: env LLDB_USE_NATIVE_PDB_READER=0 ll
 ; Link default libraries
 
 CHECK: Module [[CU:.*]]
-CHECK: {{^[0-9A-F]+}}: SymbolVendor ([[CU]])
+CHECK: {{^[0-9A-F]+}}: SymbolVendor pdb ([[CU]])
 CHECK: {{^[0-9A-F]+}}:   CompileUnit{{[{]0x[0-9a-f]+[}]}}, language = "c++", 
file = '{{.*}}\CompilandsTest.cpp'

Modified: lldb/trunk/lit/SymbolFile/PDB/func-symbols.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/func-symbols.test?rev=350947&r1=350946&r2=350947&view=diff
==
--- lldb/trunk/lit/SymbolFile/PDB/func-symbols.test (original)
+++ lldb/trunk/lit/SymbolFile/PDB/func-symbols.test Fri Jan 11 10:24:17 2019
@@ -7,7 +7,7 @@ RUN: lldb-test symbols %T/FuncSymbolsTes
 ; In this test, We don't check demangled name of a mangled function.
 
 CHECK-ONE: Module [[MD:.*]]
-CHECK-ONE-DAG: {{.*}}: SymbolVendor ([[MD]])
+CHECK-ONE-DAG: {{.*}}: SymbolVendor pdb ([[MD]])
 CHECK-ONE-DAG: [[TY0:.*]]:   Type{[[UID0:.*]]} , name = "Func_arg_array", decl 
= FuncSymbolsTestMain.cpp:3, compiler_type = {{.*}} int (int *)
 CHECK-ONE-DAG: [[TY1:.*]]:   Type{[[UID1:.*]]} , name = "Func_arg_void", decl 
= FuncSymbolsTestMain.cpp:4, compiler_type = {{.*}} void (void)
 CHECK-ONE-DAG: [[TY2:.*]]:   Type{[[UID2:.*]]} , name = "Func_arg_none", decl 
= FuncSymbolsTestMain.cpp:5, compiler_type = {{.*}} void (void)

Modified: lldb/trunk/lit/SymbolFile/PDB/type-quals.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/type-quals.test?rev=350947&r1=350946&r2=350947&view=diff
==
--- lldb/trunk/lit/SymbolFile/PDB/type-quals.test (original)
+++ lldb/trunk/lit/SymbolFile/PDB/type-quals.test Fri Jan 11 10:24:17 2019
@@ -4,7 +4,7 @@ RUN: %build --compiler=msvc --mode=link
 RUN: lldb-test symbols %T/TypeQualsTest.cpp.exe | FileCheck %s
 
 CHECK: Module [[MOD:.*]]
-CHECK-DAG: {{^[0-9A-F]+}}: SymbolVendor ([[MOD]])
+CHECK-DAG: {{^[0-9A-F]+}}: SymbolVendor pdb ([[MOD]])
 CHECK-DAG:  Type{{.*}} , name = "const int", size = 4, compiler_type = 
{{.*}} const int
 CHECK-DAG:  Type{{.*}} , size = 4, compiler_type = {{.*}} const int *
 CHECK-DAG:  Type{{.*}} , size = 4, compiler_type = {{.*}} const int **const

Modified: lldb/trunk/lit/SymbolFile/PDB/typedefs.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/typedefs.test?rev=350947&r1=350946&r2=350947&view=diff
==
--- lldb/trunk/lit/SymbolFile/PDB/typedefs.test (original)
+++ lldb/trunk/lit/SymbolFile/PDB/typedefs.test Fri Jan 11 10:24:17 2019
@@ -12,7 +12,7 @@ RUN: lldb-test symbols %T/SimpleTypesTes
 ; both of them is the same.
 
 CHECK: Module [[MOD:.*]]
-CHECK: {{^[0-9A-F]+}}: SymbolVendor ([[MOD]])
+CHECK: {{^[0-9A-F]+}}: SymbolVendor pdb ([[MOD]])
 CHECK-DAG: name = "char32_t", size = 4, compiler_type = {{.*}} char32_t
 CHECK-DAG: name = "char16_t", size = 2, compiler_type = {{.*}} char16_t
 CHECK-DAG: Type{{.*}} , name = "unsigned long", size = 4, compiler_ty

[Lldb-commits] [PATCH] D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER

2019-01-11 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz abandoned this revision.
sgraenitz marked 3 inline comments as done.
sgraenitz added a comment.

Closing this in favor of a more comprehensive fix.


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

https://reviews.llvm.org/D56440



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

[Lldb-commits] [lldb] r350950 - Fix build breaks after the ParseCompileUnit changes.

2019-01-11 Thread Zachary Turner via lldb-commits

Author: zturner
Date: Fri Jan 11 10:35:58 2019
New Revision: 350950

URL: http://llvm.org/viewvc/llvm-project?rev=350950&view=rev
Log:
Fix build breaks after the ParseCompileUnit changes.

The addition of SymbolFileBreakpad crossed paths with my change,
so this interface needs to be fixed up as well.

Modified:
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h

Modified: lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp?rev=350950&r1=350949&r2=350950&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp Fri 
Jan 11 10:35:58 2019
@@ -115,12 +115,12 @@ CompUnitSP SymbolFileBreakpad::ParseComp
   return nullptr;
 }
 
-size_t SymbolFileBreakpad::ParseCompileUnitFunctions(const SymbolContext &sc) {
+size_t SymbolFileBreakpad::ParseFunctions(CompileUnit &comp_unit) {
   // TODO
   return 0;
 }
 
-bool SymbolFileBreakpad::ParseCompileUnitLineTable(const SymbolContext &sc) {
+bool SymbolFileBreakpad::ParseLineTable(CompileUnit &comp_unit) {
   // TODO
   return 0;
 }

Modified: lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h?rev=350950&r1=350949&r2=350950&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h Fri Jan 
11 10:35:58 2019
@@ -53,23 +53,21 @@ public:
 
   lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
 
-  lldb::LanguageType
-  ParseCompileUnitLanguage(const SymbolContext &sc) override {
+  lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) override {
 return lldb::eLanguageTypeUnknown;
   }
 
-  size_t ParseCompileUnitFunctions(const SymbolContext &sc) override;
+  size_t ParseFunctions(CompileUnit &comp_unit) override;
 
-  bool ParseCompileUnitLineTable(const SymbolContext &sc) override;
+  bool ParseLineTable(CompileUnit &comp_unit) override;
 
-  bool ParseCompileUnitDebugMacros(const SymbolContext &sc) override {
-return false;
-  }
+  bool ParseDebugMacros(CompileUnit &comp_unit) override { return false; }
 
-  bool ParseCompileUnitSupportFiles(const SymbolContext &sc,
-FileSpecList &support_files) override {
+  bool ParseSupportFiles(CompileUnit &comp_unit,
+ FileSpecList &support_files) override {
 return false;
   }
+  size_t ParseTypes(CompileUnit &cu) override { return 0; }
 
   bool
   ParseImportedModules(const SymbolContext &sc,
@@ -86,7 +84,6 @@ public:
 return 0;
   }
 
-  size_t ParseTypesForCompileUnit(CompileUnit &cu) override { return 0; }
   size_t ParseVariablesForContext(const SymbolContext &sc) override {
 return 0;
   }


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

[Lldb-commits] [PATCH] D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages

2019-01-11 Thread Alex Langford via Phabricator via lldb-commits

xiaobai marked an inline comment as done.
xiaobai added inline comments.



Comment at: cmake/modules/LLDBStandalone.cmake:9
+  find_package(LLVM REQUIRED CONFIG
+HINTS "${LLDB_PATH_TO_LLVM_BUILD}" NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
+  find_package(Clang REQUIRED CONFIG

mgorny wrote:
> labath wrote:
> > xiaobai wrote:
> > > labath wrote:
> > > > Why do you put `NO_DEFAULT_PATH` here? IIUC, the user will now have to 
> > > > specify `LLDB_PATH_TO_LLVM_BUILD` in order to build this, whereas 
> > > > previously llvm-config would be found on the path if it happened to be 
> > > > there (e.g. because it was already installed).
> > > > 
> > > > Would it not make sense to keep this behavior?
> > > In situations where you have multiple LLVM builds where each might be a 
> > > different version of LLVM, I think it is better to force the user to 
> > > specify which LLVM build they want to use instead of having them 
> > > implicitly rely on whichever llvm-config happens to be in their path.
> > > 
> > > That being said, I would be willing to remove `NO_DEFAULT_PATH` here. I 
> > > can understand if people find this behavior valuable or if the scenario I 
> > > described is not very common.
> > I don't actually use the standalone build, so I don't care about this too 
> > much. I just mentioned this because this is the default behavior when 
> > searching for packages (as well as the previous behavior when we searched 
> > for llvm-config). However, it is true that we are version-locked much more 
> > tightly with llvm than with any of the other packages we search for with 
> > find_package.
> > 
> > The other thing that bugs me about the LLDB_PATH_TO_(LLVM|CLANG)_BUILD 
> > variables is that they seem to imply that you should point them to the 
> > build tree of llvm/clang. However, it should be possible to build lldb 
> > against an already-installed llvm, right (in which case they will likely 
> > have the same value)? In either case, I think it would be nice to 
> > explicitly declare these as a cache variable, if only so we can provide a 
> > docstring for them.
> In situations when you have multiple versions of LLVM in PATH, you usually 
> set PATH in the order you want it to be used. And you really don't like when 
> projects try to second-guess you.
@labath: When I wrote this I thought that it is possible to build against an 
installed LLVM, but I don't think that's currently possible. For example, 
`LLVM_MAIN_INCLUDE_DIR` should be set to the include directory in the LLVM 
source tree. TableGen.cmake adds the path in this variable to the include path 
when it invokes llvm-tablegen. This is exposed in the LLVM CMake package as 
`LLVM_BUILD_MAIN_INCLUDE_DIR` but only when you're using an LLVM build tree. 
The reason you need this is `tools/driver/Options.td` includes 
`"llvm/Option/OptParser.td"`, which does not get put into the include directory 
of your LLVM build/install.

Maybe I should rewrite part of this to make it clearer that you need a build 
tree and not an llvm install? Declaring the variables as cache variables is a 
good idea nonetheless.

@mgorny: It seems like you find the behavior valuable so I will remove 
`NO_DEFAULT_PATH`. CMake processes the `HINTS` before searching your `PATH` 
regardles, so if you set `LLDB_PATH_TO_${PROJECT}_BUILD` then it will use that 
instead of using whatever it finds in your `PATH`.


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

https://reviews.llvm.org/D56531



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

lldb-commits@lists.llvm.org

2019-01-11 Thread Zachary Turner via Phabricator via lldb-commits

zturner created this revision.
zturner added reviewers: clayborg, labath, davide.

This limits the API surface by providing only the necessary and sufficient 
information through the parameter list, while also making the expectations of 
the API self-documenting, since we include the word `Recursive` in the function 
name.


https://reviews.llvm.org/D56614

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolVendor.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/SymbolVendor.cpp

Index: lldb/source/Symbol/SymbolVendor.cpp
===
--- lldb/source/Symbol/SymbolVendor.cpp
+++ lldb/source/Symbol/SymbolVendor.cpp
@@ -189,12 +189,12 @@
   return false;
 }
 
-size_t SymbolVendor::ParseFunctionBlocks(const SymbolContext &sc) {
+size_t SymbolVendor::ParseBlocksRecursive(Function &func) {
   ModuleSP module_sp(GetModule());
   if (module_sp) {
 std::lock_guard guard(module_sp->GetMutex());
 if (m_sym_file_ap.get())
-  return m_sym_file_ap->ParseFunctionBlocks(sc);
+  return m_sym_file_ap->ParseBlocksRecursive(func);
   }
   return 0;
 }
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -286,14 +286,14 @@
 
 Block &Function::GetBlock(bool can_create) {
   if (!m_block.BlockInfoHasBeenParsed() && can_create) {
-SymbolContext sc;
-CalculateSymbolContext(&sc);
-if (sc.module_sp) {
-  sc.module_sp->GetSymbolVendor()->ParseFunctionBlocks(sc);
+ModuleSP module_sp = CalculateSymbolContextModule();
+if (module_sp) {
+  module_sp->GetSymbolVendor()->ParseBlocksRecursive(*this);
 } else {
-  Host::SystemLog(Host::eSystemLogError, "error: unable to find module "
- "shared pointer for function '%s' "
- "in %s\n",
+  Host::SystemLog(Host::eSystemLogError,
+  "error: unable to find module "
+  "shared pointer for function '%s' "
+  "in %s\n",
   GetName().GetCString(), m_comp_unit->GetPath().c_str());
 }
 m_block.SetBlockInfoHasBeenParsed(true, true);
Index: lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
===
--- lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
+++ lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
@@ -66,7 +66,7 @@
   const lldb_private::SymbolContext &sc,
   std::vector &imported_modules) override;
 
-  size_t ParseFunctionBlocks(const lldb_private::SymbolContext &sc) override;
+  size_t ParseBlocksRecursive(lldb_private::Function &func) override;
 
   size_t
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
Index: lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
===
--- lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
+++ lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
@@ -223,9 +223,7 @@
   return false;
 }
 
-size_t SymbolFileSymtab::ParseFunctionBlocks(const SymbolContext &sc) {
-  return 0;
-}
+size_t SymbolFileSymtab::ParseBlocksRecursive(Function &func) { return 0; }
 
 size_t SymbolFileSymtab::ParseVariablesForContext(const SymbolContext &sc) {
   return 0;
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -77,7 +77,7 @@
   const lldb_private::SymbolContext &sc,
   std::vector &imported_modules) override;
 
-  size_t ParseFunctionBlocks(const lldb_private::SymbolContext &sc) override;
+  size_t ParseBlocksRecursive(lldb_private::Function &func) override;
 
   size_t
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/P

[Lldb-commits] [PATCH] D56606: [CMake] Export utility targets to the build/install tree depending on LLVM_BUILD/INSTALL_UTILS

2019-01-11 Thread Chris Bieneman via Phabricator via lldb-commits

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

LGTM


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56606



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

[Lldb-commits] [PATCH] D56606: [CMake] Export utility targets to the build/install tree depending on LLVM_BUILD/INSTALL_UTILS

2019-01-11 Thread Phabricator via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350959: [CMake] Export utility targets to the build/install 
tree depending on… (authored by stefan.graenitz, committed by ).

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56606

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
@@ -920,6 +920,9 @@
DEPENDS ${name}
COMPONENT ${name})
 endif()
+set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
+  elseif( LLVM_BUILD_UTILS )
+set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS_BUILDTREE_ONLY ${name})
   endif()
 endmacro(add_llvm_utility name)
 


Index: llvm/trunk/cmake/modules/AddLLVM.cmake
===
--- llvm/trunk/cmake/modules/AddLLVM.cmake
+++ llvm/trunk/cmake/modules/AddLLVM.cmake
@@ -920,6 +920,9 @@
DEPENDS ${name}
COMPONENT ${name})
 endif()
+set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
+  elseif( LLVM_BUILD_UTILS )
+set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS_BUILDTREE_ONLY ${name})
   endif()
 endmacro(add_llvm_utility name)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56615: [SymbolFile] Remove the SymbolContext parameter from FindNamespace

2019-01-11 Thread Zachary Turner via Phabricator via lldb-commits

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

Every callsite was passing an empty SymbolContext, so this parameter had no 
effect.  Inside the DWARF implementation of this function, however, there was 
one codepath that checked members of the SymbolContext.  Since no call-sites 
actually ever used this functionality, it was essentially dead code, so I've 
deleted this code path as well.


https://reviews.llvm.org/D56615

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolVendor.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolVendor.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Index: lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -440,7 +440,6 @@
   SymbolFilePDB *symfile =
   static_cast(plugin->GetSymbolFile());
   llvm::pdb::IPDBSession &session = symfile->GetPDBSession();
-  SymbolContext sc;
   llvm::DenseSet searched_files;
   TypeMap results;
 
@@ -457,9 +456,10 @@
   symfile->ParseDeclsForContext(CompilerDeclContext(
   clang_ast_ctx, static_cast(tu)));
 
-  auto ns_namespace = symfile->FindNamespace(sc, ConstString("NS"), nullptr);
+  auto ns_namespace = symfile->FindNamespace(ConstString("NS"), nullptr);
   EXPECT_TRUE(ns_namespace.IsValid());
 
+  SymbolContext sc;
   EXPECT_EQ(1u, symfile->FindTypes(sc, ConstString("NSClass"), &ns_namespace,
false, 0, searched_files, results));
   EXPECT_EQ(1u, results.GetSize());
Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -441,9 +441,8 @@
   CompilerDeclContext *ContextPtr =
   ContextOr->IsValid() ? &*ContextOr : nullptr;
 
-  SymbolContext SC;
   CompilerDeclContext Result =
-  Vendor.FindNamespace(SC, ConstString(Name), ContextPtr);
+  Vendor.FindNamespace(ConstString(Name), ContextPtr);
   if (Result)
 outs() << "Found namespace: "
<< Result.GetScopeQualifiedName().GetStringRef() << "\n";
Index: lldb/source/Symbol/SymbolVendor.cpp
===
--- lldb/source/Symbol/SymbolVendor.cpp
+++ lldb/source/Symbol/SymbolVendor.cpp
@@ -352,15 +352,14 @@
 }
 
 CompilerDeclContext
-SymbolVendor::FindNamespace(const SymbolContext &sc, const ConstString &name,
+SymbolVendor::FindNamespace(const ConstString &name,
 const CompilerDeclContext *parent_decl_ctx) {
   CompilerDeclContext namespace_decl_ctx;
   ModuleSP module_sp(GetModule());
   if (module_sp) {
 std::lock_guard guard(module_sp->GetMutex());
 if (m_sym_file_ap.get())
-  namespace_decl_ctx =
-  m_sym_file_ap->FindNamespace(sc, name, parent_decl_ctx);
+  namespace_decl_ctx = m_sym_file_ap->FindNamespace(name, parent_decl_ctx);
   }
   return namespace_decl_ctx;
 }
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -160,7 +160,6 @@
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
   lldb_private::CompilerDeclContext FindNamespace(
-  const lldb_private::SymbolContext &sc,
   const lldb_private::ConstString &name,
   const lldb_private::CompilerDeclContext *parent_decl_ctx) override;
 
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1619,7 +1619,6 @@
 
 
 lldb_private::CompilerDeclContext SymbolFilePDB::FindNamespace(
-const lldb_private::SymbolContext &sc,
 const lldb_private::ConstString &name,
 const lldb_private::CompilerDeclContext *parent_decl_ctx) {
   auto type_system = GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
==

[Lldb-commits] [PATCH] D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages

2019-01-11 Thread Alex Langford via Phabricator via lldb-commits

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

Removed use of `NO_DEFAULT_PATH`
Made LLDB_PATH_TO_{LLVM,CLANG}_BUILD cache variables


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

https://reviews.llvm.org/D56531

Files:
  cmake/modules/LLDBStandalone.cmake

Index: cmake/modules/LLDBStandalone.cmake
===
--- cmake/modules/LLDBStandalone.cmake
+++ cmake/modules/LLDBStandalone.cmake
@@ -5,73 +5,26 @@
 
   option(LLVM_INSTALL_TOOLCHAIN_ONLY "Only include toolchain files in the 'install' target." OFF)
 
-  # Rely on llvm-config.
-  set(CONFIG_OUTPUT)
-  find_program(LLVM_CONFIG "llvm-config")
-  if(LLVM_CONFIG)
-message(STATUS "Found LLVM_CONFIG as ${LLVM_CONFIG}")
-set(CONFIG_COMMAND ${LLVM_CONFIG}
-  "--assertion-mode"
-  "--bindir"
-  "--libdir"
-  "--includedir"
-  "--prefix"
-  "--src-root"
-  "--cmakedir")
-execute_process(
-  COMMAND ${CONFIG_COMMAND}
-  RESULT_VARIABLE HAD_ERROR
-  OUTPUT_VARIABLE CONFIG_OUTPUT
-)
-if(NOT HAD_ERROR)
-  string(REGEX REPLACE
-"[ \t]*[\r\n]+[ \t]*" ";"
-CONFIG_OUTPUT ${CONFIG_OUTPUT})
+  set(LLDB_PATH_TO_LLVM_BUILD "" CACHE PATH "Path to LLVM build tree")
+  set(LLDB_PATH_TO_CLANG_BUILD "" CACHE PATH "Path to Clang build tree")
 
-else()
-  string(REPLACE ";" " " CONFIG_COMMAND_STR "${CONFIG_COMMAND}")
-  message(STATUS "${CONFIG_COMMAND_STR}")
-  message(FATAL_ERROR "llvm-config failed with status ${HAD_ERROR}")
-endif()
-  else()
-message(FATAL_ERROR "llvm-config not found -- ${LLVM_CONFIG}")
-  endif()
+  file(TO_CMAKE_PATH LLDB_PATH_TO_LLVM_BUILD "${LLDB_PATH_TO_LLVM_BUILD}")
+  file(TO_CMAKE_PATH LLDB_PATH_TO_CLANG_BUILD "${LLDB_PATH_TO_CLANG_BUILD}")
 
-  list(GET CONFIG_OUTPUT 0 ENABLE_ASSERTIONS)
-  list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 2 LIBRARY_DIR)
-  list(GET CONFIG_OUTPUT 3 INCLUDE_DIR)
-  list(GET CONFIG_OUTPUT 4 LLVM_OBJ_ROOT)
-  list(GET CONFIG_OUTPUT 5 MAIN_SRC_DIR)
-  list(GET CONFIG_OUTPUT 6 LLVM_CMAKE_PATH)
+  find_package(LLVM REQUIRED CONFIG
+HINTS "${LLDB_PATH_TO_LLVM_BUILD}" NO_CMAKE_FIND_ROOT_PATH)
+  find_package(Clang REQUIRED CONFIG
+HINTS "${LLDB_PATH_TO_CLANG_BUILD}" NO_CMAKE_FIND_ROOT_PATH)
 
-  if(NOT MSVC_IDE)
-set(LLVM_ENABLE_ASSERTIONS ${ENABLE_ASSERTIONS}
-  CACHE BOOL "Enable assertions")
-# Assertions should follow llvm-config's.
-mark_as_advanced(LLVM_ENABLE_ASSERTIONS)
-  endif()
+  # We set LLVM_MAIN_INCLUDE_DIR so that it gets included in TableGen flags.
+  set(LLVM_MAIN_INCLUDE_DIR "${LLVM_BUILD_MAIN_INCLUDE_DIR}" CACHE PATH "Path to LLVM's source include dir")
+  # We set LLVM_CMAKE_PATH so that GetSVN.cmake is found correctly when building SVNVersion.inc
+  set(LLVM_CMAKE_PATH ${LLVM_CMAKE_DIR} CACHE PATH "Path to LLVM CMake modules")
 
-  set(LLVM_TOOLS_BINARY_DIR ${TOOLS_BINARY_DIR} CACHE PATH "Path to llvm/bin")
-  set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
-  set(LLVM_MAIN_INCLUDE_DIR ${INCLUDE_DIR} CACHE PATH "Path to llvm/include")
-  set(LLVM_DIR ${LLVM_OBJ_ROOT}/cmake/modules/CMakeFiles CACHE PATH "Path to LLVM build tree CMake files")
-  set(LLVM_BINARY_DIR ${LLVM_OBJ_ROOT} CACHE PATH "Path to LLVM build tree")
-  set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
   set(LLVM_EXTERNAL_LIT ${LLVM_TOOLS_BINARY_DIR}/llvm-lit CACHE PATH "Path to llvm-lit")
-
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
 NO_DEFAULT_PATH)
 
-  set(LLVMCONFIG_FILE "${LLVM_CMAKE_PATH}/LLVMConfig.cmake")
-  if(EXISTS ${LLVMCONFIG_FILE})
-file(TO_CMAKE_PATH "${LLVM_CMAKE_PATH}" LLVM_CMAKE_PATH)
-list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_PATH}")
-include(${LLVMCONFIG_FILE})
-  else()
-message(FATAL_ERROR "Not found: ${LLVMCONFIG_FILE}")
-  endif()
-
   # They are used as destination of target generators.
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
   set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
@@ -82,6 +35,9 @@
 set(LLVM_SHLIB_OUTPUT_INTDIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
   endif()
 
+  # We append the directory in which LLVMConfig.cmake lives. We expect LLVM's
+  # CMake modules to be in that directory as well.
+  list(APPEND CMAKE_MODULE_PATH "${LLVM_DIR}")
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)
@@ -99,28 +55,15 @@
 message("-- Found PythonInterp: ${PYTHON_EXECUTABLE}")
   endif()
 
-  # Import CMake library targets from LLVM and Clang.
-  include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm/LLVMConfig.cmake")
-  if (EXISTS "${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
-include("${LLVM_OBJ_ROOT}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang/ClangConfig.cmake")
-  endif()
-
   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
   s

[Lldb-commits] [PATCH] D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages

2019-01-11 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz added a comment.

Thanks for the initiative! Would be great to have this part cleaned up as well.




Comment at: cmake/modules/LLDBStandalone.cmake:9
+  find_package(LLVM REQUIRED CONFIG
+HINTS "${LLDB_PATH_TO_LLVM_BUILD}" NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
+  find_package(Clang REQUIRED CONFIG

xiaobai wrote:
> mgorny wrote:
> > labath wrote:
> > > xiaobai wrote:
> > > > labath wrote:
> > > > > Why do you put `NO_DEFAULT_PATH` here? IIUC, the user will now have 
> > > > > to specify `LLDB_PATH_TO_LLVM_BUILD` in order to build this, whereas 
> > > > > previously llvm-config would be found on the path if it happened to 
> > > > > be there (e.g. because it was already installed).
> > > > > 
> > > > > Would it not make sense to keep this behavior?
> > > > In situations where you have multiple LLVM builds where each might be a 
> > > > different version of LLVM, I think it is better to force the user to 
> > > > specify which LLVM build they want to use instead of having them 
> > > > implicitly rely on whichever llvm-config happens to be in their path.
> > > > 
> > > > That being said, I would be willing to remove `NO_DEFAULT_PATH` here. I 
> > > > can understand if people find this behavior valuable or if the scenario 
> > > > I described is not very common.
> > > I don't actually use the standalone build, so I don't care about this too 
> > > much. I just mentioned this because this is the default behavior when 
> > > searching for packages (as well as the previous behavior when we searched 
> > > for llvm-config). However, it is true that we are version-locked much 
> > > more tightly with llvm than with any of the other packages we search for 
> > > with find_package.
> > > 
> > > The other thing that bugs me about the LLDB_PATH_TO_(LLVM|CLANG)_BUILD 
> > > variables is that they seem to imply that you should point them to the 
> > > build tree of llvm/clang. However, it should be possible to build lldb 
> > > against an already-installed llvm, right (in which case they will likely 
> > > have the same value)? In either case, I think it would be nice to 
> > > explicitly declare these as a cache variable, if only so we can provide a 
> > > docstring for them.
> > In situations when you have multiple versions of LLVM in PATH, you usually 
> > set PATH in the order you want it to be used. And you really don't like 
> > when projects try to second-guess you.
> @labath: When I wrote this I thought that it is possible to build against an 
> installed LLVM, but I don't think that's currently possible. For example, 
> `LLVM_MAIN_INCLUDE_DIR` should be set to the include directory in the LLVM 
> source tree. TableGen.cmake adds the path in this variable to the include 
> path when it invokes llvm-tablegen. This is exposed in the LLVM CMake package 
> as `LLVM_BUILD_MAIN_INCLUDE_DIR` but only when you're using an LLVM build 
> tree. The reason you need this is `tools/driver/Options.td` includes 
> `"llvm/Option/OptParser.td"`, which does not get put into the include 
> directory of your LLVM build/install.
> 
> Maybe I should rewrite part of this to make it clearer that you need a build 
> tree and not an llvm install? Declaring the variables as cache variables is a 
> good idea nonetheless.
> 
> @mgorny: It seems like you find the behavior valuable so I will remove 
> `NO_DEFAULT_PATH`. CMake processes the `HINTS` before searching your `PATH` 
> regardles, so if you set `LLDB_PATH_TO_${PROJECT}_BUILD` then it will use 
> that instead of using whatever it finds in your `PATH`.
Is it that instead of `-DLLVM_CONFIG=/path/to/llvm-build/bin/llvm-config` we 
would now pass `-DLLDB_PATH_TO_LLVM_BUILD=/path/to/llvm-build`?

> LLDB_PATH_TO_(LLVM|CLANG)_BUILD variables [...] they will likely have the 
> same value?

In my understanding this was always required. Did you try pointing 
`LLDB_PATH_TO_CLANG_BUILD` to a standalone build of Clang? Is there a good 
use-case for it? How do we detect that this Clang builds against the same LLVM 
build-tree?

When using LLVM/Clang/etc. as a dependency in external projects I usually 
followed the advice in 
https://www.llvm.org/docs/CMake.html#embedding-llvm-in-your-project:
> If LLVM is not installed or you wish to build directly against the LLVM build 
> tree you can use LLVM_DIR as previously mentioned.

That would be quite simple and `find_package(LLVM)` accepts `LLVM_DIR` as an 
input. Did you check how the other subproject do that?


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

https://reviews.llvm.org/D56531



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

[Lldb-commits] [PATCH] D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages

2019-01-11 Thread Alex Langford via Phabricator via lldb-commits

xiaobai marked an inline comment as done.
xiaobai added inline comments.



Comment at: cmake/modules/LLDBStandalone.cmake:9
+  find_package(LLVM REQUIRED CONFIG
+HINTS "${LLDB_PATH_TO_LLVM_BUILD}" NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
+  find_package(Clang REQUIRED CONFIG

sgraenitz wrote:
> xiaobai wrote:
> > mgorny wrote:
> > > labath wrote:
> > > > xiaobai wrote:
> > > > > labath wrote:
> > > > > > Why do you put `NO_DEFAULT_PATH` here? IIUC, the user will now have 
> > > > > > to specify `LLDB_PATH_TO_LLVM_BUILD` in order to build this, 
> > > > > > whereas previously llvm-config would be found on the path if it 
> > > > > > happened to be there (e.g. because it was already installed).
> > > > > > 
> > > > > > Would it not make sense to keep this behavior?
> > > > > In situations where you have multiple LLVM builds where each might be 
> > > > > a different version of LLVM, I think it is better to force the user 
> > > > > to specify which LLVM build they want to use instead of having them 
> > > > > implicitly rely on whichever llvm-config happens to be in their path.
> > > > > 
> > > > > That being said, I would be willing to remove `NO_DEFAULT_PATH` here. 
> > > > > I can understand if people find this behavior valuable or if the 
> > > > > scenario I described is not very common.
> > > > I don't actually use the standalone build, so I don't care about this 
> > > > too much. I just mentioned this because this is the default behavior 
> > > > when searching for packages (as well as the previous behavior when we 
> > > > searched for llvm-config). However, it is true that we are 
> > > > version-locked much more tightly with llvm than with any of the other 
> > > > packages we search for with find_package.
> > > > 
> > > > The other thing that bugs me about the LLDB_PATH_TO_(LLVM|CLANG)_BUILD 
> > > > variables is that they seem to imply that you should point them to the 
> > > > build tree of llvm/clang. However, it should be possible to build lldb 
> > > > against an already-installed llvm, right (in which case they will 
> > > > likely have the same value)? In either case, I think it would be nice 
> > > > to explicitly declare these as a cache variable, if only so we can 
> > > > provide a docstring for them.
> > > In situations when you have multiple versions of LLVM in PATH, you 
> > > usually set PATH in the order you want it to be used. And you really 
> > > don't like when projects try to second-guess you.
> > @labath: When I wrote this I thought that it is possible to build against 
> > an installed LLVM, but I don't think that's currently possible. For 
> > example, `LLVM_MAIN_INCLUDE_DIR` should be set to the include directory in 
> > the LLVM source tree. TableGen.cmake adds the path in this variable to the 
> > include path when it invokes llvm-tablegen. This is exposed in the LLVM 
> > CMake package as `LLVM_BUILD_MAIN_INCLUDE_DIR` but only when you're using 
> > an LLVM build tree. The reason you need this is `tools/driver/Options.td` 
> > includes `"llvm/Option/OptParser.td"`, which does not get put into the 
> > include directory of your LLVM build/install.
> > 
> > Maybe I should rewrite part of this to make it clearer that you need a 
> > build tree and not an llvm install? Declaring the variables as cache 
> > variables is a good idea nonetheless.
> > 
> > @mgorny: It seems like you find the behavior valuable so I will remove 
> > `NO_DEFAULT_PATH`. CMake processes the `HINTS` before searching your `PATH` 
> > regardles, so if you set `LLDB_PATH_TO_${PROJECT}_BUILD` then it will use 
> > that instead of using whatever it finds in your `PATH`.
> Is it that instead of `-DLLVM_CONFIG=/path/to/llvm-build/bin/llvm-config` we 
> would now pass `-DLLDB_PATH_TO_LLVM_BUILD=/path/to/llvm-build`?
> 
> > LLDB_PATH_TO_(LLVM|CLANG)_BUILD variables [...] they will likely have the 
> > same value?
> 
> In my understanding this was always required. Did you try pointing 
> `LLDB_PATH_TO_CLANG_BUILD` to a standalone build of Clang? Is there a good 
> use-case for it? How do we detect that this Clang builds against the same 
> LLVM build-tree?
> 
> When using LLVM/Clang/etc. as a dependency in external projects I usually 
> followed the advice in 
> https://www.llvm.org/docs/CMake.html#embedding-llvm-in-your-project:
> > If LLVM is not installed or you wish to build directly against the LLVM 
> > build tree you can use LLVM_DIR as previously mentioned.
> 
> That would be quite simple and `find_package(LLVM)` accepts `LLVM_DIR` as an 
> input. Did you check how the other subproject do that?
>Is it that instead of -DLLVM_CONFIG=/path/to/llvm-build/bin/llvm-config we 
>would now pass -DLLDB_PATH_TO_LLVM_BUILD=/path/to/llvm-build?

Correct.

>In my understanding this was always required. Did you try pointing 
>LLDB_PATH_TO_CLANG_BUILD to a standalone build of Clang? Is there a good 
>use-case for it? How do we detect that this Clang builds against the same LLVM 
>

[Lldb-commits] [PATCH] D56618: [SymbolFile] Remove the SymbolContext parameter from FindTypes

2019-01-11 Thread Zachary Turner via Phabricator via lldb-commits

zturner created this revision.
zturner added reviewers: labath, clayborg, davide.

This parameter was only ever used with the Module set, and since a SymbolFile 
is tied to a module, the parameter turns out to be entirely unnecessary.  
Furthermore, it doesn't make a lot of sense to ask a caller to ask SymbolFile 
which is tied to Module X to find types for Module Y, but that possibility was 
open with the previous interface.  By removing this parameter from the API, it 
makes it harder to use incorrectly as well as easier for an implementor to 
understand what it needs to do.

This change is still mostly mechanical, although the effects trickled out a bit 
higher than with other similar patches.  I did manually verify that every 
call-site was either setting nothing at all, or only setting the module field 
of the SymbolContext.


https://reviews.llvm.org/D56618

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Core/ModuleList.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolVendor.h
  lldb/source/API/SBModule.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/DataFormatters/TypeFormat.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  
lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolFile.cpp
  lldb/source/Symbol/SymbolVendor.cpp
  lldb/source/Target/Language.cpp
  lldb/source/Target/ObjCLanguageRuntime.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Index: lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -366,10 +366,9 @@
   SymbolFilePDB *symfile =
   static_cast(plugin->GetSymbolFile());
   llvm::pdb::IPDBSession &session = symfile->GetPDBSession();
-  SymbolContext sc;
   llvm::DenseSet searched_files;
   TypeMap results;
-  EXPECT_EQ(1u, symfile->FindTypes(sc, ConstString("Class"), nullptr, false, 0,
+  EXPECT_EQ(1u, symfile->FindTypes(ConstString("Class"), nullptr, false, 0,
searched_files, results));
   EXPECT_EQ(1u, results.GetSize());
   lldb::TypeSP udt_type = results.GetTypeAtIndex(0);
@@ -389,7 +388,6 @@
   SymbolFilePDB *symfile =
   static_cast(plugin->GetSymbolFile());
   llvm::pdb::IPDBSession &session = symfile->GetPDBSession();
-  SymbolContext sc;
   llvm::DenseSet searched_files;
   TypeMap results;
 
@@ -397,7 +395,7 @@
   symfile->GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus));
   EXPECT_NE(nullptr, clang_ast_ctx);
 
-  EXPECT_EQ(1u, symfile->FindTypes(sc, ConstString("Class"), nullptr, false, 0,
+  EXPECT_EQ(1u, symfile->FindTypes(ConstString("Class"), nullptr, false, 0,
searched_files, results));
   EXPECT_EQ(1u, results.GetSize());
 
@@ -416,7 +414,7 @@
   // compiler type for both, but `FindTypes` may return more than one type
   // (with the same compiler type) because the symbols have different IDs.
   auto ClassCompilerDeclCtx = CompilerDeclContext(clang_ast_ctx, ClassDeclCtx);
-  EXPECT_LE(1u, symfile->FindTypes(sc, ConstString("NestedClass"),
+  EXPECT_LE(1u, symfile->FindTypes(ConstString("NestedClass"),
&ClassCompilerDeclCtx, false, 0,
searched_files, results));
   EXPECT_LE(1u, results.GetSize());
@@ -459,9 +457,8 @@
   auto ns_namespace = symfile->FindNamespace(ConstString("NS"), nullptr);
   EXPECT_TRUE(ns_namespace.IsValid());
 
-  SymbolContext sc;
-  EXPECT_EQ(1u, symfile->FindTypes(sc, ConstString("NSClass"), &ns_namespace,
-   false, 0, searched_files, results));
+  EXPECT_EQ(1u, symfile->FindTypes(ConstString("NSClass"), &ns_namespace, false,
+   0, searched_files, results));
   EXPECT_EQ(1u, results.GetSize());
 
   lldb::TypeSP udt_type = results.GetTypeAtIndex(0);
@@ -483,12 +480,11 @@
   SymbolFilePDB *symfile =
   static_cast(plugin->

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

2019-01-11 Thread Hui Huang via Phabricator via lldb-commits

Hui added a comment.

I think the key problem here is to make sure the argument will be treated as a 
single argument to the process launcher.

To be specific to this case only, could we just provide a quote char to 
argument log file path and log channels on Windows?

The downside is one more #if is introduced.

#ifdef _WIN32
char quote_char= '"';
#else
char quote_char='\0';
#endif

   std::string env_debugserver_log_channels =
   host_env.lookup("LLDB_SERVER_LOG_CHANNELS");
   if (!env_debugserver_log_channels.empty()) {
 debugserver_args.AppendArgument(
 llvm::formatv("--log-channels={0}", env_debugserver_log_channels)
 .str(), quote_char);
  }

In D56230#1350986 , @labath wrote:

> > It is not that applicable for the windows process launcher to determine 
> > which entry in the args needs to be quoted unless given very specific flag 
> > or option.
>
> Why not? Given the argv parsing rules described here 
> https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments?view=vs-2017,
>  it sounds like it should be possible to create an algorithm doing the 
> reverse mapping.
>  Something like this ought to do the trick:
>
>   for(string: Args) {
> if (string.contains_either_of(" \t\"")  {
>   double_the_amount_of_backslashes_in_front_of_every_quote_char(string);
>   string = '"' + string '"';
> }
> cmdline += " " + string;
>   }
>





Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56230



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

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

2019-01-11 Thread Zachary Turner via Phabricator via lldb-commits

zturner added a comment.

In D56230#1354829 , @Hui wrote:

> I think the key problem here is to make sure the argument will be treated as 
> a single argument to the process launcher.
>
> To be specific to this case only, could we just provide a quote char to 
> argument log file path and log channels on Windows?
>
> The downside is one more #if is introduced.
>
> #ifdef _WIN32
>  char quote_char= '"';
>  #else
>  char quote_char='\0';
>  #endif
>
>std::string env_debugserver_log_channels =
>host_env.lookup("LLDB_SERVER_LOG_CHANNELS");
>if (!env_debugserver_log_channels.empty()) {
>  debugserver_args.AppendArgument(
>  llvm::formatv("--log-channels={0}", env_debugserver_log_channels)
>  .str(), quote_char);
>   }

I almost feel like the `Args` class could be made smarter here.  Because all of 
the proposed solutions will still not work correctly on Linux.  For example, 
why is this Windows specific?   `Args::GetCommandString()` is very dumb and 
just loops over each one appending to a string.  So if your log file name 
contains spaces on Linux, you would still have a problem no?

Currently the signature of `AppendArgument()` takes an argument string, and a 
quote char, which defaults to `'\0'` which means no quote.  But there is only 
one call-site of that entire function out of many hundreds of other calls to 
the function, most of which just pass nothing.  This means that all of those 
other call-sites are broken if their argument contains a space (many are string 
literals though, so it won't be a problem there).

But why don't we just split this into two functions:

  Args::QuoteAndAppendArgument(const Twine &arg);
  Args::AppendArgument(const Twine &arg);

and the first one will auto-detect the quoting style to use based on the host 
operating system.  This way we would fix the problem for all platforms, not 
just Windows, and we could also use the function to fix potentially many other 
bugs at all the callsites where we append unquoted arguments.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56230

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

[Lldb-commits] [lldb] r350978 - [lldbsuite] Skip TestExitDuringStep on Windows

2019-01-11 Thread Stella Stamenova via lldb-commits

Author: stella.stamenova
Date: Fri Jan 11 15:08:35 2019
New Revision: 350978

URL: http://llvm.org/viewvc/llvm-project?rev=350978&view=rev
Log:
[lldbsuite] Skip TestExitDuringStep on Windows

This test is flaky on Windows and will occasionally hang or fail.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/exit_during_step/TestExitDuringStep.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/exit_during_step/TestExitDuringStep.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/exit_during_step/TestExitDuringStep.py?rev=350978&r1=350977&r2=350978&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/exit_during_step/TestExitDuringStep.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/exit_during_step/TestExitDuringStep.py
 Fri Jan 11 15:08:35 2019
@@ -37,6 +37,7 @@ class ExitDuringStepTestCase(TestBase):
 False)
 
 @skipIfFreeBSD  # llvm.org/pr21411: test is hanging
+@skipIfWindows # This is flakey on Windows: llvm.org/pr38373
 def test_step_in(self):
 """Test thread exit during step-in handling."""
 self.build(dictionary=self.getBuildFlags())


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

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

2019-01-11 Thread Aaron Smith via Phabricator via lldb-commits

asmith updated this revision to Diff 181377.

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

https://reviews.llvm.org/D56230

Files:
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -126,14 +126,14 @@
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunication::SendPacketNoLock(llvm::StringRef payload) {
-StreamString packet(0, 4, eByteOrderBig);
-packet.PutChar('$');
-packet.Write(payload.data(), payload.size());
-packet.PutChar('#');
-packet.PutHex8(CalculcateChecksum(payload));
-std::string packet_str = packet.GetString();
-
-return SendRawPacketNoLock(packet_str);
+  StreamString packet(0, 4, eByteOrderBig);
+  packet.PutChar('$');
+  packet.Write(payload.data(), payload.size());
+  packet.PutChar('#');
+  packet.PutHex8(CalculcateChecksum(payload));
+  std::string packet_str = packet.GetString();
+
+  return SendRawPacketNoLock(packet_str);
 }
 
 GDBRemoteCommunication::PacketResult
@@ -952,16 +952,26 @@
   char debugserver_path[PATH_MAX];
   FileSpec &debugserver_file_spec = launch_info.GetExecutableFile();
 
+  Environment host_env = Host::GetEnvironment();
+
+#if defined(_WIN32)
+  // To ensure the Window's process launcher treats strings with spaces
+  // as a single argument
+  char quote_char = '"';
+#else
+  char quote_char = '\0';
+#endif
+
   // Always check to see if we have an environment override for the path to the
   // debugserver to use and use it if we do.
-  const char *env_debugserver_path = getenv("LLDB_DEBUGSERVER_PATH");
-  if (env_debugserver_path) {
+  std::string env_debugserver_path = host_env.lookup("LLDB_DEBUGSERVER_PATH");
+  if (!env_debugserver_path.empty()) {
 debugserver_file_spec.SetFile(env_debugserver_path,
   FileSpec::Style::native);
 if (log)
   log->Printf("GDBRemoteCommunication::%s() gdb-remote stub exe path set "
   "from environment variable: %s",
-  __FUNCTION__, env_debugserver_path);
+  __FUNCTION__, env_debugserver_path.c_str());
   } else
 debugserver_file_spec = g_debugserver_file_spec;
   bool debugserver_exists =
@@ -1004,7 +1014,6 @@
 
 Args &debugserver_args = launch_info.GetArguments();
 debugserver_args.Clear();
-char arg_cstr[PATH_MAX];
 
 // Start args with "debugserver /file/path -r --"
 debugserver_args.AppendArgument(llvm::StringRef(debugserver_path));
@@ -1114,29 +1123,29 @@
 }
   }
 }
-
-const char *env_debugserver_log_file = getenv("LLDB_DEBUGSERVER_LOG_FILE");
-if (env_debugserver_log_file) {
-  ::snprintf(arg_cstr, sizeof(arg_cstr), "--log-file=%s",
- env_debugserver_log_file);
-  debugserver_args.AppendArgument(llvm::StringRef(arg_cstr));
+std::string env_debugserver_log_file =
+host_env.lookup("LLDB_DEBUGSERVER_LOG_FILE");
+if (!env_debugserver_log_file.empty()) {
+  debugserver_args.AppendArgument(
+  llvm::formatv("--log-file={0}", env_debugserver_log_file).str(),
+  quote_char);
 }
 
 #if defined(__APPLE__)
 const char *env_debugserver_log_flags =
 getenv("LLDB_DEBUGSERVER_LOG_FLAGS");
 if (env_debugserver_log_flags) {
-  ::snprintf(arg_cstr, sizeof(arg_cstr), "--log-flags=%s",
- env_debugserver_log_flags);
-  debugserver_args.AppendArgument(llvm::StringRef(arg_cstr));
+  debugserver_args.AppendArgument(
+  llvm::formatv("--log-flags={0}", env_debugserver_log_flags).str());
 }
 #else
-const char *env_debugserver_log_channels =
-getenv("LLDB_SERVER_LOG_CHANNELS");
-if (env_debugserver_log_channels) {
-  ::snprintf(arg_cstr, sizeof(arg_cstr), "--log-channels=%s",
- env_debugserver_log_channels);
-  debugserver_args.AppendArgument(llvm::StringRef(arg_cstr));
+std::string env_debugserver_log_channels =
+host_env.lookup("LLDB_SERVER_LOG_CHANNELS");
+if (!env_debugserver_log_channels.empty()) {
+  debugserver_args.AppendArgument(
+  llvm::formatv("--log-channels={0}", env_debugserver_log_channels)
+  .str(),
+  quote_char);
 }
 #endif
 
@@ -1148,15 +1157,15 @@
   char env_var_name[64];
   snprintf(env_var_name, sizeof(env_var_name),
"LLDB_DEBUGSERVER_EXTRA_ARG_%" PRIu32, env_var_index++);
-  const char *extra_arg = getenv(env_var_name);
-  has_env_var = extra_arg != nullptr;
+  std::string extra_arg = host_env.lookup(env_var_name);
+  has_env_var = !extra_arg.empty();
 
   if (has_env_var) {
 debugserver_args.AppendArgument(llvm::StringRef(extra_arg));
 if (log)
   log->Printf("GDBRemoteCommunicat

[Lldb-commits] [PATCH] D56232: [lldb-server] Add remote platform capabilities for Windows

2019-01-11 Thread Hui Huang via Phabricator via lldb-commits

Hui added a comment.

In D56232#1349407 , @labath wrote:

> In D56232#1348514 , @zturner wrote:
>
> > Is that even necessary?  If a platform is not remote aware, `IsHost()` will 
> > always just return `true` by definition.  So we could put all of this in 
> > the existing `Platform` base class.
>
>
> I remember looking at this a while a go and concluding against it, but i'm 
> not sure if it was impossible of just I didn't like the result.
>
> The issue here is that PlatformWindows and PlatformPosix already have a 
> m_remote_platform member (which normally is an instance of 
> PlatformRemoteGDBServer). To move the common class into the base one, we'd 
> need to move this member too. That would mean that any platform has a 
> "remote" member, even those that already are "remote". That sounds a bit 
> weird.

Yes. I think the thing is the existing design makes PlatformWindows plugin play 
dual roles: a "host" and "remote-windows" platform which simplily is a 
pass-through to PlatformRemoteGDBServer.  Not quite sure about such intent of 
design.  Maybe to avoid creating new plugin for a remote platform or just to 
simplifying the init/release in plugin manager.  To break them apart, adding 
back the plugin, maybe PlatformRemoteWindows

PlatformWindows:  host implementation only. Most common parts will go to the 
platform base.

PlatformRemoteWindows: for communicating with remote only.  Could just be 
RemoteAwarePlatform

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56232

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

[Lldb-commits] [lldb] r350990 - Add SymbolFileBreakpad.

2019-01-11 Thread Jason Molenda via lldb-commits

Author: jmolenda
Date: Fri Jan 11 19:17:39 2019
New Revision: 350990

URL: http://llvm.org/viewvc/llvm-project?rev=350990&view=rev
Log:
Add SymbolFileBreakpad.

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=350990&r1=350989&r2=350990&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Fri Jan 11 19:17:39 2019
@@ -905,6 +905,7 @@
268900DD13353E6F00698AC0 /* Symbol.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 26BC7F1B10F1B8EC00F91463 /* Symbol.cpp */; };
268900DE13353E6F00698AC0 /* SymbolContext.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 26BC7F1C10F1B8EC00F91463 /* SymbolContext.cpp 
*/; };
268900DF13353E6F00698AC0 /* SymbolFile.cpp in Sources */ = {isa 
= PBXBuildFile; fileRef = 26BC7F1D10F1B8EC00F91463 /* SymbolFile.cpp */; };
+   AF97744721E9947E006876A7 /* SymbolFileBreakpad.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = AF97744621E9947B006876A7 /* 
SymbolFileBreakpad.cpp */; };
268900CA13353E5F00698AC0 /* SymbolFileDWARF.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 260C89D910F57C5600BB2B04 /* SymbolFileDWARF.cpp 
*/; };
268900CC13353E5F00698AC0 /* SymbolFileDWARFDebugMap.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = 260C89DB10F57C5600BB2B04 /* 
SymbolFileDWARFDebugMap.cpp */; };
6D95DC021B9DC057000E318A /* SymbolFileDWARFDwo.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 6D95DBFF1B9DC057000E318A /* 
SymbolFileDWARFDwo.cpp */; };
@@ -2975,6 +2976,8 @@
26BC7C6110F1B6E900F91463 /* SymbolContextScope.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
SymbolContextScope.h; path = include/lldb/Symbol/SymbolContextScope.h; 
sourceTree = ""; };
26BC7F1D10F1B8EC00F91463 /* SymbolFile.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = SymbolFile.cpp; path = source/Symbol/SymbolFile.cpp; sourceTree = 
""; };
26BC7C6210F1B6E900F91463 /* SymbolFile.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
SymbolFile.h; path = include/lldb/Symbol/SymbolFile.h; sourceTree = ""; 
};
+   AF97744621E9947B006876A7 /* SymbolFileBreakpad.cpp */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = 
SymbolFileBreakpad.cpp; path = 
source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp; sourceTree = 
SOURCE_ROOT; };
+   AF97744521E9947B006876A7 /* SymbolFileBreakpad.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = 
SymbolFileBreakpad.h; path = 
source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h; sourceTree = 
SOURCE_ROOT; };
260C89D910F57C5600BB2B04 /* SymbolFileDWARF.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = SymbolFileDWARF.cpp; sourceTree = ""; };
260C89DA10F57C5600BB2B04 /* SymbolFileDWARF.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
SymbolFileDWARF.h; sourceTree = ""; };
260C89DB10F57C5600BB2B04 /* SymbolFileDWARFDebugMap.cpp */ = 
{isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = 
sourcecode.cpp.cpp; path = SymbolFileDWARFDebugMap.cpp; sourceTree = ""; 
};
@@ -4024,6 +4027,7 @@
260C89B110F57C5600BB2B04 /* SymbolFile */ = {
isa = PBXGroup;
children = (
+   AF97744421E99467006876A7 /* Breakpad */,
AFD966B321714099006714AC /* NativePDB */,
AF6335DF1C87B20A00F7D554 /* PDB */,
260C89B210F57C5600BB2B04 /* DWARF */,
@@ -6804,6 +6808,16 @@
path = ../Disassembler;
sourceTree = "";
};
+   AF97744421E99467006876A7 /* Breakpad */ = {
+   isa = PBXGroup;
+   children = (
+   AF97744621E9947B006876A7 /* 
SymbolFileBreakpad.cpp */,
+   AF97744521E9947B006876A7 /* 
SymbolFileBreakpad.h */,
+   );
+   name = Breakpad;
+   path = "New Group";
+   sourceTree = "";
+   };
AFAFD8081E57E19E0017A14F /* Target */ = {
isa = PBXGroup;
children = (
@@ -8328,6 +8342,7 @@
3FBA69E11B6067120008F44A /* 
ScriptInterpreterNone.cpp in Sources */,

41 matches

Mail list logo