[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Overall, I am pretty sure we will end up with some kind of a special `Optional` 
class sooner or later. However, it's not clear to me whether this is the place 
to start. What kind of space this is saving? I would expect that size matters 
when you store something as a data member in a class, because you can have 
zillion of instances of that class on the heap. However, all the uses of this I 
see are local (stack) variables, where the size does not matter that much, and 
so you might as well have used an `Optional`. Have I missed something?

Overall, I think a good way to keep things sane would be to keep these 
specialized Optional classes as implementation details of classes, and convert 
to a real `Optional` as soon as it goes out to the rest of the world. This 
would be more consistent with how the rest of llvm does this (see 
`llvm::VersionTuple` for instance, which also does some storage optimizations, 
though without a special class). This would allow us to use the richer 
`Optional` interface in most of the places (e.g. `Optional` has special 
overloads of operator==, which allow you to compare an `Optional` with plain 
`T`), while still keeping the size footprint in check.


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

https://reviews.llvm.org/D56688



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

[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

+1. Especially since our assumption that everything related to C++ is of 
non-zero size is also not entirely correct. Empty structs for example are 
actually size 0 for Clang until we reach code gen where we set their size to 1. 
That's why we have this bug where we can't call functions that return an empty 
struct.


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

https://reviews.llvm.org/D56688



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

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/Process/Windows/Common/ProcessWindows.cpp:858
 
+Status ProcessWindows::GetFileLoadAddress(const FileSpec &file, bool 
&is_loaded,
+  lldb::addr_t &load_addr) {

asmith wrote:
> clayborg wrote:
> > This entire function is doing the job of the dynamic loader and should be 
> > moved into the dynamic loader.
> when do you think the changes you mentioned for the loader and 
> lldb-server.exe would be available for review? should we abandon this and 
> wait for those?
I am saying the DynamicLoaderWindowsDYLD should be fixed, so that no matter how 
we connect to a windows process, we get correct DYLD functionality. The way 
this patch is currently coded, you require every process plug-in that connects 
to windows processes to have this work around. 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56237



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

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

[Leonid Mashinskiy via Phabricator via lldb-commits]

leonid.mashinskiy updated this revision to Diff 181792.
leonid.mashinskiy set the repository for this revision to rLLDB LLDB.
leonid.mashinskiy added a comment.
Herald added a subscriber: arphaman.

- Ported implementation to NativePDB plugin.
- Implemented GetVariableLocationInfo for local variables of 
S_DEFRANGE_FRAMEPOINTER_REL and S_DEFRANGE_REGISTER_REL type


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55122

Files:
  lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp
  lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script
  lit/SymbolFile/PDB/variables-locations.test
  source/Expression/DWARFExpression.cpp
  source/Plugins/SymbolFile/NativePDB/CMakeLists.txt
  source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.cpp
  source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.h
  source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.h
  source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
  source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.h
  source/Plugins/SymbolFile/NativePDB/PdbIndex.h
  source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  source/Plugins/SymbolFile/NativePDB/PdbUtil.h
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  source/Plugins/SymbolFile/PDB/CMakeLists.txt
  source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  unittests/SymbolFile/CMakeLists.txt
  unittests/SymbolFile/NativePDB/CMakeLists.txt
  unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp

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

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:769-774
+FileSpec::Style DWARFUnit::GetPathStyle() {
+  if (!m_comp_dir)
+ComputeCompDirAndGuessPathStyle();
+  return m_comp_dir->GetPathStyle();
+}
+

Remove?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:777
+  if (!m_comp_dir)
+ComputeCompDirAndGuessPathStyle();
+  return *m_comp_dir;

Just move contents of ComputeCompDirAndGuessPathStyle() to this function now 
that we don't have GetPathStyle?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:818
+
+void DWARFUnit::ComputeCompDirAndGuessPathStyle() {
+  m_comp_dir = FileSpec();

Move contents to DWARFUnit::GetCompilationDirectory?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:173
+  const lldb_private::FileSpec &GetCompilationDirectory();
+  lldb_private::FileSpec::Style GetPathStyle();
+

Is GetPathStyle() needed? Seems like we should be able to grab the style from:

```
auto style = unit->GetCompilationDirectory().GetPathStyle();
```




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

If we remove GetPathStyle above, should this be removed and just put into the 
code for GetCompilationDirectory() now that we don't have two accessors?



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:757
   if (cu_die) {
-FileSpec cu_file_spec(cu_die.GetName());
+FileSpec cu_file_spec(cu_die.GetName(), dwarf_cu->GetPathStyle());
 if (cu_file_spec) {

Not sure how we would resolve this one. What if dwarf_cu doesn't have a 
DW_AT_comp_dir? Maybe we add a FileSpec constructor that takes a string and a 
FileSpec &relative_dir so this would would just become:

```
FileSpec cu_fs(cu_die.GetName(), dwarf_cu->GetCompilationDirectory());
```

The proto would be something like:
```
explicit FileSpec::FileSpec(llvm::StringRef path, const FileSpec 
&relative_root);
```

Then the if statement below goes away.



Comment at: source/Utility/FileSpec.cpp:560
 
+void FileSpec::MakeAbsolute(const FileSpec &dir) {
+  if (IsRelative())

Should this be "bool FileSpec::ResolveRelativePath(const FileSpec &dir)"? 
Returns true if this path was relative and it was resolved, and false otherwise?


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] D56590: breakpad: Add FUNC records to the symtab

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp:29-48
+Token breakpad::toToken(llvm::StringRef str) {
   return llvm::StringSwitch(str)
   .Case("MODULE", Token::Module)
   .Case("INFO", Token::Info)
   .Case("FILE", Token::File)
   .Case("FUNC", Token::Func)
   .Case("PUBLIC", Token::Public)

Move to BreakpadLine.cpp/.h?



Comment at: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h:20-23
+enum class Token { Unknown, Module, Info, File, Func, Public, Stack };
+
+Token toToken(llvm::StringRef str);
+llvm::StringRef toString(Token t);

Should we have a "Line" class in lldb_private::breakpad::Line? All functions 
that parse a breakpad line could be in this new class? Seeing as we have symbol 
files, object files and the process plug-in might need to parse these lines, 
seems like it would be cleaner? Maybe in BreakpadLine.cpp/.h? 

```
namespace lldb_private {
namespace breakpad {
class Line {
  enum class Token { Unknown, Module, Info, File, Func, Public, Stack };
  Line(llvm::StringRef s);
  Token parseToken();
  static llvm::StringRef tokenToString(Token t);
};
```



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:26
 namespace {
 class LineIterator {
 public:

Move this functionality into llvm::breakpad::Line?



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:187
+
+  auto parse = [&](llvm::StringRef line, bool is_func) {
+// [m] address {size} param_size name

change "llvm::StringRef line" to "lldb_private::breakpad::Line" and remove 
second parameter as the "Line" class can know its token type?



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:233-240
+// 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.
+

If we make a Line class this code would become:

```
lldb_private::breakpad::Line bp_line(line);
if (bp_line.GetToken() != Token::Func)
  continue;
parse(bp_line);
```




Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:246
+// skip PUBLIC keyword
+parse(getToken(line).second, false);
+  }

If we make a Line class this code would become:

```
parse(bp_line);
```



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

https://reviews.llvm.org/D56590



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

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:232
+  };
+  for (llvm::StringRef line: lines(*m_obj_file, Token::Func)) {
+// Here we can get either FUNC records (starting with FUNC), or line 
records

Should the iterator for Token::Func just return "FUNC" objects only? Maybe we 
add a Token::Line to the Token enumeration and then add an optional second 
parameter to the iterator? So any code that would want a "FUNC" and its lines, 
would do:

```
for (llvm::StringRef line: lines(*m_obj_file, Token::Func, Token::Line)) {
}
```



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:240
+
+parse(line, true);
   }

If we modify the iterator this would become:
```
parse(lldb_private::breakpad::Line(line));
```


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

https://reviews.llvm.org/D56590



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

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

So LLDB treats compile units special depending on the inline strategy. Because 
of this, I outlined some examples of how and why we should create a compile 
unit per "FUNC" token. Let me know if anything above was unclear




Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:119
 
+uint32_t SymbolFileBreakpad::GetNumCompileUnits() { return 1; }
+

I would vote to make 1 compile unit for each "FUNC". The hard part will be to 
select the right source file for each function. I would just start by selecting 
the first line entry for the function. Compile units are expected to give a 
list of support files, and in this case, you would make a set of files that are 
in the line entries only. So if you had:

```
MODULE Linux x86_64 761550E08086333960A9074A9CE2895C0 a.out
INFO CODE_ID E05015768680393360A9074A9CE2895C
FILE 0 /tmp/a.c
FILE 1 /tmp/b.c
FILE 2 /usr/include/foo.h
FUNC b0 10 0 func1
b0 1 1 0
b1 1 2 0
b2 1 2 1
b4 1 3 1
FUNC b0 10 0 func2
b0 1 1 1
b1 1 2 1
b2 1 2 2
```
We would have a compile unit for "func1" with a cu for "/tmp/a.c" and with 
support files:
support_file[0] = "/tmp/a.c"
support_file[1] = "/usr/include/foo.h"

We would have a compile unit for "func2" with a cu for "/tmp/b.c" and with 
support files:

support_file[0] = "/tmp/b.c"
support_file[1] = "/usr/include/foo.h"

The main reason for make individual compile units, is LLDB treats a compile 
unit specially when settings breakpoints depending on if we ask for inline 
functions to be set.  If we set the following setting:
```
(lldb) settings set target.inline-breakpoint-strategy never
```
Then we will check the name of the compile unit to ensure it matches. So if we 
did:
```
(lldb) b a.c:12
```
This would only work if the actual lldb_private::CompileUnit has a FileSpec 
that matches "a.c". 



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:122
 CompUnitSP SymbolFileBreakpad::ParseCompileUnitAtIndex(uint32_t index) {
-  // TODO
-  return nullptr;
+  assert(index == 0);
+  m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(

return the count of the number of "FUNC" objects



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:123-125
+  m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(
+  0, m_comp_unit_sp);
+  return m_comp_unit_sp;

parse a compile unit per function. We might want to cache all "FILE" entries in 
a list inside the SymbolFileBreakpad so we can easily pull out the FileSpecs 
when creating each compile unit.

Also, each compile unit's ID can be the line number in the breakpad file to the 
"FUNC" entry. This allows easy access to each "FUNC" entry in the breakpad file 
when we are asked to parse more information about it (get compile unit support 
files, or any parsing of info for a compile unit.



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:158-179
+llvm::StringRef token_str;
+std::tie(token_str, line) = getToken(line);
+if (toToken(token_str) == Token::Func)
+  continue;
+
+// address size line filenum
+addr_t address;

It would be nice to put this parsing code into the 
lldb_private::breakpad::Line" class I talked about in the other patch?

It would be great if this code looked like:

```
lldb_private::breakpad::Line bp_line(line);
switch (bp_line.GetToken()) {
  case Token::Func:
// Create the compile unit and store into cu_sp
cu_sp = bp_line.CreateCompileUnit();
break;
  case Token::Line: {
addr_t address;
size_t size;
uint32_t line_num, file_num;
if (bp_line.ParseLineEntry(address, size, line_num, file_num)) {
  // Discontiguous entries. Finish off the previous sequence and reset.
  if (next_addr && *next_addr != address)
finish_sequence();
  line_table_up->AppendLineEntryToSequence(
  line_seq_up.get(), address, line_num, 0, file_num, true, false, 
false, false,  false);
}
```



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:199
+
+bool SymbolFileBreakpad::ParseCompileUnitSupportFiles(
+const SymbolContext &sc, FileSpecList &support_files) {

This function will need to populate support_files for a given FUNC as mentioned 
above



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:234
+  if (resolve_scope & eSymbolContextLineEntry) {
+if (sc.comp_unit->GetLineTable()->FindLineEntryByAddress(so_addr,
+ sc.line_entry)) {

We need to search each compile unit to see which compile unit contains the 
address now.



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h:99
   bool CompleteType(CompilerType &compiler_type) override { return false; }
+
   uint32_t Reso

[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

[Davide Italiano via Phabricator via lldb-commits]

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

I hit an incarnation of this problem, and I was able to get away with a 
localized fix in:
 [lldb] r344982 - [ValueObject] Stop assuming types are non-zero sized.

Of course, your approach is principled and it's IMHO clearly the right way 
forward.
I didn't check every single callsite, as the patch is mechanical enough, but 
the general idea looks good to me.


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

https://reviews.llvm.org/D56688



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

[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

Looks good to me


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

https://reviews.llvm.org/D56688



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

[Lldb-commits] [lldb] r351215 - Silence compiler warnings

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Jan 15 10:07:54 2019
New Revision: 351215

URL: http://llvm.org/viewvc/llvm-project?rev=351215&view=rev
Log:
Silence compiler warnings

Modified:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteiOS.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.cpp

Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp?rev=351215&r1=351214&r2=351215&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp 
(original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp Tue 
Jan 15 10:07:54 2019
@@ -89,7 +89,7 @@ PlatformSP PlatformAppleTVSimulator::Cre
   // Only accept "unknown" for the vendor if the host is Apple and it
   // "unknown" wasn't specified (it was just returned because it was NOT
   // specified)
-  case llvm::Triple::UnknownArch:
+  case llvm::Triple::UnknownVendor:
 create = !arch->TripleVendorWasSpecified();
 break;
 #endif

Modified: 
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp?rev=351215&r1=351214&r2=351215&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp 
(original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp 
Tue Jan 15 10:07:54 2019
@@ -88,7 +88,7 @@ PlatformSP PlatformAppleWatchSimulator::
   // Only accept "unknown" for the vendor if the host is Apple and it
   // "unknown" wasn't specified (it was just returned because it was NOT
   // specified)
-  case llvm::Triple::UnknownArch:
+  case llvm::Triple::UnknownVendor:
 create = !arch->TripleVendorWasSpecified();
 break;
 #endif

Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp?rev=351215&r1=351214&r2=351215&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp 
(original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp Tue Jan 
15 10:07:54 2019
@@ -111,7 +111,7 @@ PlatformSP PlatformDarwinKernel::CreateI
 
 // Only accept "unknown" for vendor if the host is Apple and it "unknown"
 // wasn't specified (it was just returned because it was NOT specified)
-case llvm::Triple::UnknownArch:
+case llvm::Triple::UnknownVendor:
   create = !arch->TripleVendorWasSpecified();
   break;
 default:

Modified: 
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp?rev=351215&r1=351214&r2=351215&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp 
(original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp Tue 
Jan 15 10:07:54 2019
@@ -94,7 +94,7 @@ PlatformSP PlatformRemoteAppleBridge::Cr
   // Only accept "unknown" for the vendor if the host is Apple and
   // it "unknown" wasn't specified (it was just returned because it
   // was NOT specified)
-  case llvm::Triple::UnknownArch:
+  case llvm::Triple::UnknownVendor:
 create = !arch->TripleVendorWasSpecified();
 break;
 

Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp?rev=351215&r1=351214&r2=351215&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp 
(original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp Tue Jan 
15 10:07:54 2019
@@ -97,7 +97,7 @@ PlatformSP PlatformRemoteAppleTV::Create
   // Only accept "unknown" for the vendor if the host is Apple and
   // "unknown" wasn't specified (it was just returned because i

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

[Hui Huang via Phabricator via lldb-commits]

Hui added a comment.

In D56230#1355248 , @labath wrote:

> In D56230#1355247 , @labath wrote:
>
> > For example, for a `Args` vector like `lldb-server`, `gdb-remote`, 
> > `--log-channels=foo\\\   \\\"""   '''`, `whatever`, `QuoteForCreateProcess` 
> > would return
> >  `lldb-server gdb-remote "--log-channels=foo\\\   \\\"\"\"   '''" 
> > whatever` (there are other ways to quote this too). Passing this string to 
> > CreateProcess will result in the original vector being available to the 
> > `main` function of lldb-server, which is what this code (and all other code 
> > that works with the `Args` class) expects.
>
>
> Btw, there is already code for doing this in llvm 
> (`llvm::sys::flattenWindowsCommandLine`), so we can just steal the 
> implementation from there.


What do you think of the following codes to be added in Args?

  bool Args::GetFlattenQuotedCommandString(std::string &command) const {
std::vector args_ref;
std::vector owner;
  
for (size_t i = 0; i < m_entries.size(); ++i) {
  if (m_entries[i].quote) {
std::string arg;
arg += m_entries[i].quote;
arg += m_entries[i].ref;
arg += m_entries[i].quote;
owner.push_back(arg);
args_ref.push_back(owner.back());
  } else {
args_ref.push_back(m_entries[i].ref);
  }
}
  
command = llvm::sys::flattenWindowsCommandLine(args_ref);
  
return !m_entries.empty();
  }


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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D56230#1358102 , @Hui wrote:

> What do you think of the following codes to be added in Args?
>
>   bool Args::GetFlattenQuotedCommandString(std::string &command) const {
> std::vector args_ref;
> std::vector owner;
>  
> for (size_t i = 0; i < m_entries.size(); ++i) {
>   if (m_entries[i].quote) {
> std::string arg;
> arg += m_entries[i].quote;
> arg += m_entries[i].ref;
> arg += m_entries[i].quote;
> owner.push_back(arg);
> args_ref.push_back(owner.back());
>   } else {
> args_ref.push_back(m_entries[i].ref);
>   }
> }
>  
> command = llvm::sys::flattenWindowsCommandLine(args_ref);
>  
> return !m_entries.empty();
>   }
>


Yes, that's more-or-less what I had in mind. I have some comments about the 
implementation, but none of them are fundamental, I think:

- add "windows" to the function name somewhere 
(`GetFlattenedWindowsCommandString`?, `GetWindowsCommandString`?, ...), as 
function uses quoting style which is specifically meant for windows, and is 
unlikely to be correct elsewhere
- drop the `if (m_entries[i].quote)` branch. You don't need it here, and I 
don't believe it will be correct anyway, because all it will do is cause 
`llvm::sys::flattenWindowsCommandLine` to add one more quote level around that 
and escape the quotes added by yourself


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] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

I had to stop commenting because there's too many occurrences, but please 
remove `auto` pretty much throughout the patch.  A reader of the code will not 
necessarily be aware or remember that the function returns an `Optional`, and 
they will read the code as "if the size is 0, report an error".  So I think the 
type should be spelled explicitly in all cases.




Comment at: include/lldb/Target/ProcessStructReader.h:70
 }
-size_t total_size = struct_type.GetByteSize(nullptr);
-lldb::DataBufferSP buffer_sp(new DataBufferHeap(total_size, 0));
+auto total_size = struct_type.GetByteSize(nullptr);
+if (!total_size)

I think we shouldn't use `auto` here, although this is a minor nit and I don't 
feel strongly.



Comment at: include/lldb/Target/ProcessStructReader.h:73
+  return;
+lldb::DataBufferSP buffer_sp(new DataBufferHeap(*total_size, 0));
 Status error;

Can we use `make_shared` here?



Comment at: source/API/SBType.cpp:107-108
+  if (IsValid())
+if (auto size = m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr))
+  return *size;
+  return 0;

`TypeImpl::GetCompilerType` needs to return an `Optional` also, otherwise you 
will have the same issue here with zero-size types.



Comment at: source/Commands/CommandObjectMemory.cpp:527-528
 
-  m_format_options.GetByteSizeValue() = 
clang_ast_type.GetByteSize(nullptr);
-
-  if (m_format_options.GetByteSizeValue() == 0) {
+  auto size = clang_ast_type.GetByteSize(nullptr);
+  if (!size) {
 result.AppendErrorWithFormat(

Can you remove `auto` here?  It's definitely not obvious that it's returning an 
`Optional` here, so I originally read this as "if the size is 0, return an 
error".



Comment at: source/Commands/CommandObjectMemory.cpp:650
 
-  bytes_read = clang_ast_type.GetByteSize(nullptr) *
-   m_format_options.GetCountValue().GetCurrentValue();
+  auto size = clang_ast_type.GetByteSize(nullptr);
+  if (!size) {

Same, please remove `auto`.



Comment at: source/Commands/CommandObjectMemory.cpp:1072
 uint64_t value = result_sp->GetValueAsUnsigned(0);
-switch (result_sp->GetCompilerType().GetByteSize(nullptr)) {
+auto size = result_sp->GetCompilerType().GetByteSize(nullptr);
+if (!size)

`auto`



Comment at: source/Core/Value.cpp:227-229
+  if (auto size = ast_type.GetByteSize(
+  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
+byte_size = *size;

If the ast type is valid (if-check just above), then is it actually possible 
for this method to fail?



Comment at: source/Core/Value.cpp:349-351
+  if (auto size = ast_type.GetByteSize(
+  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
+limit_byte_size = *size;

Same here.  Can this actually fail?



Comment at: source/Core/ValueObject.cpp:759-762
+  auto item_type_size = pointee_or_element_compiler_type.GetByteSize(
   exe_ctx.GetBestExecutionContextScope());
-  const uint64_t bytes = item_count * item_type_size;
-  const uint64_t offset = item_idx * item_type_size;
+  if (!item_type_size)
+return 0;

`auto`



Comment at: source/Core/ValueObject.cpp:827-828
 case eAddressTypeHost: {
-  const uint64_t max_bytes =
+  auto max_bytes =
   
GetCompilerType().GetByteSize(exe_ctx.GetBestExecutionContextScope());
+  if (max_bytes && *max_bytes > offset) {

`auto`



Comment at: source/Core/ValueObject.cpp:1826-1828
+  auto size = type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
+  if (!size)
+return {};

`auto`



Comment at: source/Core/ValueObject.cpp:1829-1831
+  ValueObjectChild *synthetic_child =
+  new ValueObjectChild(*this, type, name_const_str, *size, offset, 0, 0,
+   false, false, eAddressTypeInvalid, 0);

This should probably be `auto synthetic_child = 
std::make_shared(...);`


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56688



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

[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: include/lldb/Target/ProcessStructReader.h:70
 }
-size_t total_size = struct_type.GetByteSize(nullptr);
-lldb::DataBufferSP buffer_sp(new DataBufferHeap(total_size, 0));
+auto total_size = struct_type.GetByteSize(nullptr);
+if (!total_size)

zturner wrote:
> I think we shouldn't use `auto` here, although this is a minor nit and I 
> don't feel strongly.
I actually changed my mind here.  I didn't feel strongly at first because I 
thought this was a `uint64_t`.  As I read through more of the patch, I realized 
it was an `Optional`, which totally changes the semantics.  So now I 
do feel more strongly


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56688



___
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

[Hui Huang via Phabricator via lldb-commits]

Hui added a comment.



> - drop the `if (m_entries[i].quote)` branch. You don't need it here, and I 
> don't believe it will be correct anyway, because all it will do is cause 
> `llvm::sys::flattenWindowsCommandLine` to add one more quote level around 
> that and escape the quotes added by yourself

The quote char might be specified through Args::AppendArgument at user's will.


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] D56543: DWARF: Add some support for non-native directory separators

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:173
+  const lldb_private::FileSpec &GetCompilationDirectory();
+  lldb_private::FileSpec::Style GetPathStyle();
+

clayborg wrote:
> Is GetPathStyle() needed? Seems like we should be able to grab the style from:
> 
> ```
> auto style = unit->GetCompilationDirectory().GetPathStyle();
> ```
> 
I'd like to avoid that for two reasons:
- I think this documents the intent better. Logically, the "path style" is a 
property of the compile unit and the fact that the compilation directory (and 
everything else) uses this path style is a consequence of that. The fact that 
we actually get the path style from the compilation directory is an 
implementation detail that users should not care about and may in fact change 
in the future e.g. if DWARF develops an ability to explicitly signal this 
information.
- I'm not thrilled by the fact that we can have an empty (invalid) FileSpec, 
whose path style still has some significance (this happens if the CU does not 
have a comp_dir attribute, and we guess the style from the. In fact I even 
considered getting rid of that and having an explicit path style member). In 
the end I decided it's not too bad if it's internal to DWARFCompileUnit, but I 
would not want to expose this quirk to the outside world. So if anything, I'd 
actually go for making the path style a separate member.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:757
   if (cu_die) {
-FileSpec cu_file_spec(cu_die.GetName());
+FileSpec cu_file_spec(cu_die.GetName(), dwarf_cu->GetPathStyle());
 if (cu_file_spec) {

clayborg wrote:
> Not sure how we would resolve this one. What if dwarf_cu doesn't have a 
> DW_AT_comp_dir? Maybe we add a FileSpec constructor that takes a string and a 
> FileSpec &relative_dir so this would would just become:
> 
> ```
> FileSpec cu_fs(cu_die.GetName(), dwarf_cu->GetCompilationDirectory());
> ```
> 
> The proto would be something like:
> ```
> explicit FileSpec::FileSpec(llvm::StringRef path, const FileSpec 
> &relative_root);
> ```
> 
> Then the if statement below goes away.
That would work if we say that we do support having empty/invalid FileSpec 
objects with certain PathStyle. However, as I said above, I am not entirely 
thrilled by that. FWIW, I don't think having this if is bad, as it makes it 
clear that how you're intending to handle the case when the CU has no name.



Comment at: source/Utility/FileSpec.cpp:560
 
+void FileSpec::MakeAbsolute(const FileSpec &dir) {
+  if (IsRelative())

clayborg wrote:
> Should this be "bool FileSpec::ResolveRelativePath(const FileSpec &dir)"? 
> Returns true if this path was relative and it was resolved, and false 
> otherwise?
I called this `MakeAbsolute` because that's how the equivalent llvm function is 
called (`llvm::sys::fs::make_absolute`). I think it would be good to stay 
consistent here.


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] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D56230#1358269 , @Hui wrote:

> > - drop the `if (m_entries[i].quote)` branch. You don't need it here, and I 
> > don't believe it will be correct anyway, because all it will do is cause 
> > `llvm::sys::flattenWindowsCommandLine` to add one more quote level around 
> > that and escape the quotes added by yourself
>
> The quote char might be specified through Args::AppendArgument at user's will.


Yes, but do you know what the user meant when he set it? I don't, but I'm 
pretty sure he did not mean it to be passed verbatim to the launched process. 
It certainly isn't what will happen on posix systems as there we just ignore 
it. Maybe nobody knows, which is why nobody sets it? I think it would be best 
to be consistent with posix systems, and ignore it here too (at least until we 
have a good reason to do otherwise).


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] D56737: [SymbolFile] Split ParseVariablesForContext into two functions.

[Zachary Turner via Phabricator via lldb-commits]

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

This function was being used in 2 separate use cases.  The first is parsing 
variables in a function, and the second is parsing global variables.  To 
clarify the intent, I've split this into two functions, 
ParseLocalVariablesRecursive and ParseGlobalVariables.

In doing so, I found what may actually be a bug.   This bug was very subtle in 
the old implementation, and an example of exactly why I think it's important to 
clean up these interfaces.   There was a disconnect in what the caller expected 
to happen and what the implementation actually did.

Specifically, before when calling `Block::GetBlockVariableList` it was 
implemented this way:

  SymbolContext sc;
  CalculateSymbolContext(&sc);
  assert(sc.module_sp);
  sc.module_sp->GetSymbolVendor()->ParseVariablesForContext(sc);

This is logical, because it expects that `ParseVariablesForContext` will see 
that the block is set, then only parse variables for the given block.  However, 
the implementation of this function did not actually handle the case where 
`sc.m_block` was set.  Instead, this would fall to the `sc.m_function` code 
path, and parse all variables for the entire function, recursively, even though 
only 1 block was requested.

I expect this will be a performance problem if you have large function with 
many blocks and you put a breakpoint in one of the leaf-blocks and try to print 
a variable.  It would have to parse potentially thousands of other blocks' 
variables even though only 1 is needed.

After this patch, there is no functional change, but the bug is more obvious, 
because `Block::GetVariableList` is implemented like this:

  VariableListSP Block::GetBlockVariableList(bool can_create) {
if (!m_parsed_block_variables) {
  if (m_variable_list_sp.get() == nullptr && can_create) {
m_parsed_block_variables = true;
Function *func = CalculateSymbolContextFunction();
ModuleSP module = CalculateSymbolContextModule();
module->GetSymbolVendor()->ParseLocalVariablesRecursive(*func);
  }
}
return m_variable_list_sp;
  }

This way, a person so motivated can go through and add another overload such as 
`ParseVariablesForBlock` (or change `ParseVariablesForFunction` to 
`ParseVariablesForBlock(Block& block, bool recursive)` so that we can pass 
false there.`


https://reviews.llvm.org/D56737

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/Block.cpp
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/SymbolVendor.cpp

Index: lldb/source/Symbol/SymbolVendor.cpp
===
--- lldb/source/Symbol/SymbolVendor.cpp
+++ lldb/source/Symbol/SymbolVendor.cpp
@@ -209,12 +209,22 @@
   return 0;
 }
 
-size_t SymbolVendor::ParseVariablesForContext(const SymbolContext &sc) {
+size_t SymbolVendor::ParseGlobalVariables(CompileUnit &comp_unit) {
   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->ParseVariablesForContext(sc);
+  return m_sym_file_ap->ParseGlobalVariables(comp_unit);
+  }
+  return 0;
+}
+
+size_t SymbolVendor::ParseLocalVariablesRecursive(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->ParseLocalVariablesRecursive(func);
   }
   return 0;
 }
Index: lldb/source/Symbol/CompileUnit.cpp
===
--- lldb/source/Symbol/CompileUnit.cpp
+++ lldb/source/Symbol/CompileUnit.cpp
@@ -233,10 +233,8 @@
 
 VariableListSP CompileUnit::GetVariableList(bool can_create) {
   if (m_variables.get() == nullptr && can_create) {
-SymbolContext sc;
-CalculateSymbolContext(&sc);
-assert(sc.module_sp);
-sc.module_sp->GetSymbolVendor()->ParseVariablesForContext(sc);
+ModuleSP module = CalculateSymbolContextModule();
+GetModule()->GetSymbolVendor()->ParseGlobalVariables(*this);
   }
 
   return m_variables;
Index: lldb/source/Symbol/Block.cpp
===

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

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In D56230#1358328 , @labath wrote:

> In D56230#1358269 , @Hui wrote:
>
> > > - drop the `if (m_entries[i].quote)` branch. You don't need it here, and 
> > > I don't believe it will be correct anyway, because all it will do is 
> > > cause `llvm::sys::flattenWindowsCommandLine` to add one more quote level 
> > > around that and escape the quotes added by yourself
> >
> > The quote char might be specified through Args::AppendArgument at user's 
> > will.
>
>
> Yes, but do you know what the user meant when he set it? I don't, but I'm 
> pretty sure he did not mean it to be passed verbatim to the launched process. 
> It certainly isn't what will happen on posix systems as there we just ignore 
> it. Maybe nobody knows, which is why nobody sets it? I think it would be best 
> to be consistent with posix systems, and ignore it here too (at least until 
> we have a good reason to do otherwise).


I've always disliked this argument and hoped that someday someone would remove 
it entirely.  My recollection (which may be wrong) is that the only actual use 
of it is so that if someone types a command, and we later need to print the 
command back, we will print it with the same quote char.  It almost seems like 
we could just delete the argument and use a standardized quote char when 
flattening a command string.


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] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Sorry for the late review




Comment at: lldb/trunk/include/lldb/Target/ProcessStructReader.h:70
 }
-size_t total_size = struct_type.GetByteSize(nullptr);
-lldb::DataBufferSP buffer_sp(new DataBufferHeap(total_size, 0));
+auto total_size = struct_type.GetByteSize(nullptr);
+if (!total_size)

I will also second the sentiment on the `auto`, it does not improve readability.



Comment at: lldb/trunk/source/API/SBType.cpp:106
 uint64_t SBType::GetByteSize() {
-  if (!IsValid())
-return 0;
-
-  return m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr);
+  if (IsValid())
+if (auto size = m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr))

The early exit is more readable here and also has a lower mental load.



Comment at: lldb/trunk/source/Commands/CommandObjectMemory.cpp:647
+  if (!size) {
+result.AppendError("can't get size of type");
+return false;

This does not appear to be NFC



Comment at: lldb/trunk/source/Commands/CommandObjectMemory.cpp:1069
+if (!size)
+  return false;
+switch (*size) {

NFC?



Comment at: lldb/trunk/source/Core/Value.cpp:227
 if (ast_type.IsValid())
-  byte_size = ast_type.GetByteSize(
-  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
+  if (auto size = ast_type.GetByteSize(
+  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))

This might read better as two steps, get the value and then check it.



Comment at: lldb/trunk/source/Expression/Materializer.cpp:50
+  if (auto size = type.GetByteSize(nullptr))
+m_size = *size;
 

What value does `m_size` have otherwise?



Comment at: lldb/trunk/source/Expression/Materializer.cpp:800
+  if (!byte_size) {
+err.SetErrorString("can't get size of type");
+return;

NFC?



Comment at: lldb/trunk/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp:2372
   lldb::offset_t offset = 0;
-  if (byte_size == sizeof(float)) {
+  if (*byte_size == sizeof(float)) {
 value.GetScalar() = data.GetFloat(&offset);

switch?



Comment at: lldb/trunk/source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp:827
 CompilerType compiler_type(value->GetCompilerType());
-if (compiler_type) {
+auto bit_size = compiler_type.GetBitSize(&thread);
+if (bit_size) {

Looks like you chopped out the `if (compiler_type) ` check



Comment at: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:1720
   if (homogeneous_count > 0 && homogeneous_count <= 4) {
+auto base_byte_size = base_type.GetByteSize(nullptr);
 if (base_type.IsVectorType(nullptr, nullptr)) {

There are a lot of checks for `base_byte_size` so it is not clear what value 
`vfp_byte_size` will have if `base_byte_size` is empty.

The flow looks a lot different but hard to track what will be set and not set.



Comment at: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:1788
 
-DataBufferSP data_sp(new DataBufferHeap(byte_size, 0));
+DataBufferSP data_sp(new DataBufferHeap(*byte_size, 0));
 uint32_t data_offset = 0;

`make_shared`?



Comment at: lldb/trunk/source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp:2343
   lldb::offset_t offset = 0;
-  if (byte_size == sizeof(float)) {
+  if (*byte_size == sizeof(float)) {
 value.GetScalar() = data.GetFloat(&offset);

switch? 



Comment at: lldb/trunk/source/Plugins/ABI/SysV-s390x/ABISysV_s390x.cpp:379
 CompilerType compiler_type = value->GetCompilerType();
-if (!compiler_type)
+auto bit_size = compiler_type.GetBitSize(&thread);
+if (!bit_size)

Looks the `if (!compiler_type)` was removed.



Comment at: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp:228
   }
-  CompilerType 
pair_type(__i_->GetCompilerType().GetTypeTemplateArgument(0));
-  std::string name; uint64_t bit_offset_ptr; uint32_t 
bitfield_bit_size_ptr; bool is_bitfield_ptr;
-  pair_type = pair_type.GetFieldAtIndex(0, name, &bit_offset_ptr, 
&bitfield_bit_size_ptr, &is_bitfield_ptr);
+  CompilerType pair_type(
+  __i_->GetCompilerType().GetTypeTemplateArgument(0));

Unrelated formatting changes?



Comment at: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp:243
   m_pair_ptr = nullptr;
-  if (addr && addr!=LLDB_INVALID_ADDRESS) {
-ClangASTContext *ast_ctx = 
llvm::dyn_cast_or_null(pair_type.GetTypeSystem());
+  if (addr && addr != LLDB

[Lldb-commits] [lldb] r351237 - Replace auto -> llvm::Optional

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Jan 15 12:33:58 2019
New Revision: 351237

URL: http://llvm.org/viewvc/llvm-project?rev=351237&view=rev
Log:
Replace auto -> llvm::Optional

This addresses post-commit feedback for https://reviews.llvm.org/D56688

Modified:
lldb/trunk/source/API/SBType.cpp
lldb/trunk/source/Commands/CommandObjectMemory.cpp
lldb/trunk/source/Core/Value.cpp
lldb/trunk/source/Core/ValueObject.cpp
lldb/trunk/source/Core/ValueObjectMemory.cpp
lldb/trunk/source/Core/ValueObjectVariable.cpp
lldb/trunk/source/DataFormatters/TypeFormat.cpp
lldb/trunk/source/DataFormatters/VectorType.cpp
lldb/trunk/source/Expression/Materializer.cpp
lldb/trunk/source/Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.cpp
lldb/trunk/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
lldb/trunk/source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp
lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
lldb/trunk/source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp
lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
lldb/trunk/source/Plugins/ABI/SysV-mips/ABISysV_mips.cpp
lldb/trunk/source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
lldb/trunk/source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp
lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
lldb/trunk/source/Plugins/ABI/SysV-s390x/ABISysV_s390x.cpp
lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
lldb/trunk/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp

lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp
lldb/trunk/source/Symbol/CompilerType.cpp
lldb/trunk/source/Symbol/Type.cpp

Modified: lldb/trunk/source/API/SBType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBType.cpp?rev=351237&r1=351236&r2=351237&view=diff
==
--- lldb/trunk/source/API/SBType.cpp (original)
+++ lldb/trunk/source/API/SBType.cpp Tue Jan 15 12:33:58 2019
@@ -104,7 +104,8 @@ bool SBType::IsValid() const {
 
 uint64_t SBType::GetByteSize() {
   if (IsValid())
-if (auto size = m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr))
+if (llvm::Optional size =
+m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr))
   return *size;
   return 0;
 }

Modified: lldb/trunk/source/Commands/CommandObjectMemory.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectMemory.cpp?rev=351237&r1=351236&r2=351237&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectMemory.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectMemory.cpp Tue Jan 15 12:33:58 2019
@@ -519,7 +519,7 @@ protected:
 --pointer_count;
   }
 
-  auto size = clang_ast_type.GetByteSize(nullptr);
+  llvm::Optional size = clang_ast_type.GetByteSize(nullptr);
   if (!size) {
 result.AppendErrorWithFormat(
 "unable to get the byte size of the type '%s'\n",
@@ -642,7 +642,7 @@ protected:
   if (!m_format_options.GetFormatValue().OptionWasSet())
 m_format_options.GetFormatValue().SetCurrentValue(eFormatDefault);
 
-  auto size = clang_ast_type.GetByteSize(nullptr);
+  llvm::Optional size = clang_ast_type.GetByteSize(nullptr);
   if (!size) {
 result.AppendError("can't get size of type");
 return false;
@@ -1064,7 +1064,8 @@ protected:
m_memory_options.m_expr.GetStringValue(), frame, result_sp)) &&
   result_sp) {
 uint64_t value = result_sp->GetValueAsUnsigned(0);
-auto size = result_sp->GetCompilerType().GetByteSize(nullptr);
+llvm::Optional size =
+result_sp->GetCompilerType().GetByteSize(nullptr);
 if (!size)
   return false;
 switch (*size) {

Modified: lldb/trunk/source/Core/Value.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Value.cpp?rev=351237&r1=351236&r2=351237&view=diff
==
--- lldb/trunk/source/Core/Value.cpp (original)
+++ lldb/trunk/source/Core/Value.cpp Tue Jan 15 12:33:58 2019
@@ -224,7 +224,7 @@ uint64_t Value::GetValueByteSize(Status
   {
 const CompilerType &ast_type = GetCompilerType();
 if (ast_type.IsValid())
-

[Lldb-commits] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: zturner, compnerd, sgraenitz.
Herald added subscribers: mgorny, ki.stfu.

When building LLDB standalone on windows, lldb-mi fails to build
because it doesn't link against User32. This patch adds an explicit dependency
of lldb-mi on User32.


https://reviews.llvm.org/D56741

Files:
  tools/lldb-mi/CMakeLists.txt


Index: tools/lldb-mi/CMakeLists.txt
===
--- tools/lldb-mi/CMakeLists.txt
+++ tools/lldb-mi/CMakeLists.txt
@@ -7,6 +7,10 @@
   list(APPEND extra_libs pthread)
 endif ()
 
+if(CMAKE_SYSTEM_NAME MATCHES "Windows")
+  list(APPEND extra_libs User32)
+endif()
+
 # We need to include the llvm components we depend on manually, as liblldb does
 # not re-export those.
 set(LLVM_LINK_COMPONENTS Support)


Index: tools/lldb-mi/CMakeLists.txt
===
--- tools/lldb-mi/CMakeLists.txt
+++ tools/lldb-mi/CMakeLists.txt
@@ -7,6 +7,10 @@
   list(APPEND extra_libs pthread)
 endif ()
 
+if(CMAKE_SYSTEM_NAME MATCHES "Windows")
+  list(APPEND extra_libs User32)
+endif()
+
 # We need to include the llvm components we depend on manually, as liblldb does
 # not re-export those.
 set(LLVM_LINK_COMPONENTS Support)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

[Adrian Prantl via Phabricator via lldb-commits]

aprantl marked an inline comment as done.
aprantl added a comment.

I changed all the autos back to full types in r351237.




Comment at: lldb/trunk/source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp:827
 CompilerType compiler_type(value->GetCompilerType());
-if (compiler_type) {
+auto bit_size = compiler_type.GetBitSize(&thread);
+if (bit_size) {

shafik wrote:
> Looks like you chopped out the `if (compiler_type) ` check
That's intentional since GetByteSize also checks for IsValid().


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56688



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

[Lldb-commits] [lldb] r351243 - Add Doxygen comments.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Jan 15 13:04:18 2019
New Revision: 351243

URL: http://llvm.org/viewvc/llvm-project?rev=351243&view=rev
Log:
Add Doxygen comments.

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

Modified: lldb/trunk/include/lldb/Symbol/CompilerType.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/CompilerType.h?rev=351243&r1=351242&r2=351243&view=diff
==
--- lldb/trunk/include/lldb/Symbol/CompilerType.h (original)
+++ lldb/trunk/include/lldb/Symbol/CompilerType.h Tue Jan 15 13:04:18 2019
@@ -287,7 +287,9 @@ public:
 
   struct IntegralTemplateArgument;
 
+  /// Return the size of the type in bytes.
   llvm::Optional GetByteSize(ExecutionContextScope *exe_scope) const;
+  /// Return the size of the type in bits.
   llvm::Optional GetBitSize(ExecutionContextScope *exe_scope) const;
 
   lldb::Encoding GetEncoding(uint64_t &count) const;


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

[Lldb-commits] [lldb] r351244 - Simplify code

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Jan 15 13:04:19 2019
New Revision: 351244

URL: http://llvm.org/viewvc/llvm-project?rev=351244&view=rev
Log:
Simplify code

Modified:
lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=351244&r1=351243&r2=351244&view=diff
==
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Tue Jan 15 13:04:19 2019
@@ -4506,7 +4506,7 @@ ClangASTContext::GetArrayElementType(lld
 
 // TODO: the real stride will be >= this value.. find the real one!
 if (stride)
-  if (llvm::Optional size = element_type.GetByteSize(nullptr))
+  if (Optional size = element_type.GetByteSize(nullptr))
 *stride = *size;
 
 return element_type;
@@ -5529,7 +5529,7 @@ static bool ObjCDeclHasIVars(clang::ObjC
   return false;
 }
 
-static llvm::Optional
+static Optional
 GetDynamicArrayInfo(ClangASTContext &ast, SymbolFile *sym_file,
 clang::QualType qual_type,
 const ExecutionContext *exe_ctx) {
@@ -6596,6 +6596,10 @@ CompilerType ClangASTContext::GetChildCo
   if (!type)
 return CompilerType();
 
+  auto get_exe_scope = [&exe_ctx]() {
+return exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr;
+  };
+
   clang::QualType parent_qual_type(GetCanonicalQualType(type));
   const clang::Type::TypeClass parent_type_class =
   parent_qual_type->getTypeClass();
@@ -6684,8 +6688,8 @@ CompilerType ClangASTContext::GetChildCo
 CompilerType base_class_clang_type(getASTContext(),
base_class->getType());
 child_name = base_class_clang_type.GetTypeName().AsCString("");
-llvm::Optional size = base_class_clang_type.GetBitSize(
-exe_ctx ? exe_ctx->GetBestExecutionContextScope() : NULL);
+Optional size =
+base_class_clang_type.GetBitSize(get_exe_scope());
 if (!size)
   return {};
 uint64_t base_class_clang_type_bit_size = *size;
@@ -6716,8 +6720,8 @@ CompilerType ClangASTContext::GetChildCo
   // alignment (field_type_info.second) from the AST context.
   CompilerType field_clang_type(getASTContext(), field->getType());
   assert(field_idx < record_layout.getFieldCount());
-  llvm::Optional size = field_clang_type.GetByteSize(
-  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : NULL);
+  Optional size =
+  field_clang_type.GetByteSize(get_exe_scope());
   if (!size)
 return {};
   child_byte_size = *size;
@@ -6891,8 +6895,8 @@ CompilerType ClangASTContext::GetChildCo
 
 // We have a pointer to an simple type
 if (idx == 0 && pointee_clang_type.GetCompleteType()) {
-  if (llvm::Optional size = pointee_clang_type.GetByteSize(
-  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : NULL)) {
+  if (Optional size =
+  pointee_clang_type.GetByteSize(get_exe_scope())) {
 child_byte_size = *size;
 child_byte_offset = 0;
 return pointee_clang_type;
@@ -6914,8 +6918,8 @@ CompilerType ClangASTContext::GetChildCo
   ::snprintf(element_name, sizeof(element_name), "[%" PRIu64 "]",
  static_cast(idx));
   child_name.assign(element_name);
-  if (llvm::Optional size = element_type.GetByteSize(
-  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : NULL)) {
+  if (Optional size =
+  element_type.GetByteSize(get_exe_scope())) {
 child_byte_size = *size;
 child_byte_offset = (int32_t)idx * (int32_t)child_byte_size;
 return element_type;
@@ -6933,8 +6937,8 @@ CompilerType ClangASTContext::GetChildCo
 CompilerType element_type(getASTContext(), array->getElementType());
 if (element_type.GetCompleteType()) {
   child_name = llvm::formatv("[{0}]", idx);
-  if (llvm::Optional size = element_type.GetByteSize(
-  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : NULL)) {
+  if (Optional size =
+  element_type.GetByteSize(get_exe_scope())) {
 child_byte_size = *size;
 child_byte_offset = (int32_t)idx * (int32_t)child_byte_size;
 return element_type;
@@ -6972,8 +6976,8 @@ CompilerType ClangASTContext::GetChildCo
 
   // We have a pointer to an simple type
   if (idx == 0) {
-if (llvm::Optional size = pointee_clang_type.GetByteSize(
-exe_ctx ? exe_ctx->GetBestExecutionContextScope() : NULL)) {
+if (Optional size =
+pointee_clang_type.GetByteSize(get_exe_scope())) {
   ch

[Lldb-commits] [PATCH] D56322: SBReproducer prototype

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 181868.
JDevlieghere added a comment.
Herald added a subscriber: lldb-commits.

Prototype 2.0


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56322

Files:
  include/lldb/API/SBReproducer.h
  include/lldb/Utility/Reproducer.h
  source/API/CMakeLists.txt
  source/API/SBCommandInterpreter.cpp
  source/API/SBCommandReturnObject.cpp
  source/API/SBDebugger.cpp
  source/API/SBFileSpec.cpp
  source/API/SBHostOS.cpp
  source/API/SBReproducer.cpp
  source/API/SBReproducerPrivate.h
  tools/driver/Driver.cpp

Index: tools/driver/Driver.cpp
===
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -14,6 +14,7 @@
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBHostOS.h"
 #include "lldb/API/SBLanguageRuntime.h"
+#include "lldb/API/SBReproducer.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/API/SBStringList.h"
 
@@ -910,6 +911,11 @@
 return 1;
   }
 
+  SBReproducer reproducer;
+  if (reproducer.Replay()) {
+return 0;
+  }
+
   SBHostOS::ThreadCreated("");
 
   signal(SIGINT, sigint_handler);
Index: source/API/SBReproducerPrivate.h
===
--- /dev/null
+++ source/API/SBReproducerPrivate.h
@@ -0,0 +1,730 @@
+//===-- SBReproducerPrivate.h ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLDB_API_SBREPRODUCER_PRIVATE_H
+#define LLDB_API_SBREPRODUCER_PRIVATE_H
+
+#include "lldb/API/SBReproducer.h"
+
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Reproducer.h"
+
+#include "llvm/ADT/DenseMap.h"
+
+#include 
+
+namespace lldb_private {
+namespace repro {
+
+/// Mapping between serialized indices and their corresponding objects.
+///
+/// This class is used during replay to map indices back to in-memory objects.
+///
+/// When objects are constructed, they are added to this mapping using
+/// AddObjectForIndex.
+///
+/// When an object is passed to a function, its index is deserialized and
+/// AddObjectForIndex returns the corresponding object. If there is no object
+/// for the given index, a nullptr is returend. The latter is valid when custom
+/// replay code is in place and the actual object is ignored.
+class SBIndexToObject {
+public:
+  /// Returns an object as a pointer for the given index or nullptr if not
+  /// present in the map.
+  template  T *GetObjectForIndex(int idx) {
+assert(idx != 0 && "Cannot get object for sentinel");
+void *object = GetObjectForIndexImpl(idx);
+return static_cast::type *>(object);
+  }
+
+  /// Adds a pointer to an object to the mapping for the given index.
+  template  void AddObjectForIndex(int idx, T *object) {
+AddObjectForIndexImpl(
+idx, static_cast(
+ const_cast::type *>(object)));
+  }
+
+  /// Adds a reference to an object to the mapping for the given index.
+  template  void AddObjectForIndex(int idx, T &object) {
+AddObjectForIndexImpl(
+idx, static_cast(
+ const_cast::type *>(&object)));
+  }
+
+private:
+  /// Helper method that does the actual lookup. The void* result is later cast
+  /// by the caller.
+  void *GetObjectForIndexImpl(int idx) {
+auto it = m_mapping.find(idx);
+if (it == m_mapping.end()) {
+  return nullptr;
+}
+return m_mapping[idx];
+  }
+
+  /// Helper method that does the actual insertion.
+  void AddObjectForIndexImpl(int idx, void *object) {
+assert(idx != 0 && "Cannot add object for sentinel");
+m_mapping[idx] = object;
+  }
+
+  /// Keeps a mapping between indices and their corresponding object.
+  llvm::DenseMap m_mapping;
+};
+
+/// Base class for tag dispatch used in the SBDeserializer. Different tags are
+/// instantiated with different values.
+template  struct SBTag {};
+
+/// We need to differentiate between pointers to fundamental and
+/// non-fundamental types. See the corresponding SBDeserializer::Read method
+/// for the reason why.
+typedef SBTag<0> PointerTag;
+typedef SBTag<1> ReferenceTag;
+typedef SBTag<2> ValueTag;
+typedef SBTag<3> FundamentalPointerTag;
+typedef SBTag<4> FundamentalReferenceTag;
+
+/// Return the deserialization tag for the given type T.
+template  struct serializer_tag { typedef ValueTag type; };
+template  struct serializer_tag {
+  typedef
+  typename std::conditional::value,
+FundamentalPointerTag, PointerTag>::type type;
+};
+template  struct serializer_tag {
+  typedef typename std::conditional::value,
+FundamentalReferenceTag, ReferenceTag>::type
+  type;
+};
+

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Might be good to have a test where we have a relative DW_AT_name and not 
DW_AT_comp_dir? Just in case?


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] D56543: DWARF: Add some support for non-native directory separators

[Greg Clayton via Phabricator via lldb-commits]

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

Ok, thanks for explaining. Your reasoning makes sense.


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] [lldb] r351250 - Simplify Value::GetValueByteSize()

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Jan 15 13:26:03 2019
New Revision: 351250

URL: http://llvm.org/viewvc/llvm-project?rev=351250&view=rev
Log:
Simplify Value::GetValueByteSize()

Modified:
lldb/trunk/source/Core/Value.cpp

Modified: lldb/trunk/source/Core/Value.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Value.cpp?rev=351250&r1=351249&r2=351250&view=diff
==
--- lldb/trunk/source/Core/Value.cpp (original)
+++ lldb/trunk/source/Core/Value.cpp Tue Jan 15 13:26:03 2019
@@ -210,35 +210,31 @@ bool Value::ValueOf(ExecutionContext *ex
 }
 
 uint64_t Value::GetValueByteSize(Status *error_ptr, ExecutionContext *exe_ctx) 
{
-  uint64_t byte_size = 0;
-
   switch (m_context_type) {
   case eContextTypeRegisterInfo: // RegisterInfo *
-if (GetRegisterInfo())
-  byte_size = GetRegisterInfo()->byte_size;
+if (GetRegisterInfo()) {
+  if (error_ptr)
+error_ptr->Clear();
+  return GetRegisterInfo()->byte_size;
+}
 break;
 
   case eContextTypeInvalid:
   case eContextTypeLLDBType: // Type *
   case eContextTypeVariable: // Variable *
   {
-const CompilerType &ast_type = GetCompilerType();
-if (ast_type.IsValid())
-  if (llvm::Optional size = ast_type.GetByteSize(
-  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
-byte_size = *size;
-  } break;
-  }
-
-  if (error_ptr) {
-if (byte_size == 0) {
-  if (error_ptr->Success())
-error_ptr->SetErrorString("Unable to determine byte size.");
-} else {
-  error_ptr->Clear();
+auto *scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr;
+if (llvm::Optional size = GetCompilerType().GetByteSize(scope)) {
+  if (error_ptr)
+error_ptr->Clear();
+  return *size;
 }
+break;
+  }
   }
-  return byte_size;
+  if (error_ptr && error_ptr->Success())
+error_ptr->SetErrorString("Unable to determine byte size.");
+  return 0;
 }
 
 const CompilerType &Value::GetCompilerType() {


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

[Lldb-commits] [lldb] r351263 - [debugserver][CMake] Remove commented out line

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Tue Jan 15 14:27:59 2019
New Revision: 351263

URL: http://llvm.org/viewvc/llvm-project?rev=351263&view=rev
Log:
[debugserver][CMake] Remove commented out line

This has been commented out since rL300111
(commit d742d081f3a1e7412cc609765139ba32d597ac15). Looks like it was
committed as a commented out line, so I'm removing it.

Modified:
lldb/trunk/tools/debugserver/source/CMakeLists.txt

Modified: lldb/trunk/tools/debugserver/source/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/CMakeLists.txt?rev=351263&r1=351262&r2=351263&view=diff
==
--- lldb/trunk/tools/debugserver/source/CMakeLists.txt (original)
+++ lldb/trunk/tools/debugserver/source/CMakeLists.txt Tue Jan 15 14:27:59 2019
@@ -5,7 +5,6 @@ include_directories(${LLDB_SOURCE_DIR}/s
 include_directories(MacOSX/DarwinLog)
 
 include_directories(MacOSX)
-#include_directories(${CMAKE_CURRENT_BINARY_DIR}/MacOSX)
 
 set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -stdlib=libc++ 
-Wl,-sectcreate,__TEXT,__info_plist,${CMAKE_CURRENT_SOURCE_DIR}/../resources/lldb-debugserver-Info.plist")
 


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

[Lldb-commits] [lldb] r351264 - Simplify code by using Optional::getValueOr()

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Jan 15 14:30:01 2019
New Revision: 351264

URL: http://llvm.org/viewvc/llvm-project?rev=351264&view=rev
Log:
Simplify code by using Optional::getValueOr()

Modified:
lldb/trunk/source/Core/ValueObjectVariable.cpp
lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp

lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp

Modified: lldb/trunk/source/Core/ValueObjectVariable.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObjectVariable.cpp?rev=351264&r1=351263&r2=351264&view=diff
==
--- lldb/trunk/source/Core/ValueObjectVariable.cpp (original)
+++ lldb/trunk/source/Core/ValueObjectVariable.cpp Tue Jan 15 14:30:01 2019
@@ -112,9 +112,7 @@ uint64_t ValueObjectVariable::GetByteSiz
   if (!type.IsValid())
 return 0;
 
-  llvm::Optional size =
-  type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
-  return size ? *size : 0;
+  return 
type.GetByteSize(exe_ctx.GetBestExecutionContextScope()).getValueOr(0);
 }
 
 lldb::ValueType ValueObjectVariable::GetValueType() const {

Modified: lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp?rev=351264&r1=351263&r2=351264&view=diff
==
--- lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp (original)
+++ lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp Tue Jan 15 
14:30:01 2019
@@ -577,8 +577,7 @@ private:
   ReturnValueExtractor(Thread &thread, CompilerType &type,
RegisterContext *reg_ctx, ProcessSP process_sp)
   : m_thread(thread), m_type(type),
-m_byte_size(m_type.GetByteSize(nullptr) ? *m_type.GetByteSize(nullptr)
-: 0),
+m_byte_size(m_type.GetByteSize(nullptr).getValueOr(0)),
 m_data_ap(new DataBufferHeap(m_byte_size, 0)), m_reg_ctx(reg_ctx),
 m_process_sp(process_sp), m_byte_order(process_sp->GetByteOrder()),
 m_addr_size(

Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp?rev=351264&r1=351263&r2=351264&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp Tue Jan 15 
14:30:01 2019
@@ -337,9 +337,7 @@ bool IRForTarget::CreateResultVariable(l
   if (log)
 log->Printf("Creating a new result global: \"%s\" with size 0x%" PRIx64,
 m_result_name.GetCString(),
-m_result_type.GetByteSize(nullptr)
-? *m_result_type.GetByteSize(nullptr)
-: 0);
+m_result_type.GetByteSize(nullptr).getValueOr(0));
 
   // Construct a new result global and set up its metadata
 

Modified: 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp?rev=351264&r1=351263&r2=351264&view=diff
==
--- 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
 Tue Jan 15 14:30:01 2019
@@ -513,11 +513,11 @@ void ClassDescriptorV2::iVarsStorage::fi
 CompilerType ivar_type =
 encoding_to_type_sp->RealizeType(type, for_expression);
 if (ivar_type) {
-  llvm::Optional ivar_size = ivar_type.GetByteSize(nullptr);
   LLDB_LOGV(log,
 "name = {0}, encoding = {1}, offset_ptr = {2:x}, size = "
 "{3}, type_size = {4}",
-name, type, offset_ptr, size, ivar_size ? *ivar_size : 0);
+name, type, offset_ptr, size,
+ivar_type.GetByteSize(nullptr).getValueOr(0));
   Scalar offset_scalar;
   Status error;
   const int offset_ptr_size = 4;


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

[Lldb-commits] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

[Saleem Abdulrasool via Phabricator via lldb-commits]

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

I personally prefer the use of `target_link_libraries` rather than the custom 
stuff added in the `add_lldb_tool`.


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

https://reviews.llvm.org/D56741



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

[Lldb-commits] [PATCH] D54617: [Reproducers] Add file provider

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

With the LLVM changes now checked-in this is ready for review.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D54617



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

[Lldb-commits] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

Why do we need to link against user32?  What kind of unresolved external are 
you getting?


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

https://reviews.llvm.org/D56741



___
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.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision as: JDevlieghere.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Although I don't want to condone checking things in without a test case, given 
that the fix is obvious and that testing it is non-trivial, I think it's fine 
to check this in as is.


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] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

@zturner: `undefined symbol: __imp_MessageBoxA`. Looks like MessageBoxA is used 
in the file `tools/lldb-mi/MIUtilDebug.cpp`, specifically in the function 
`CMIUtilDebug::ShowDlgWaitForDbgAttach`.

In D56741#1358733 , @compnerd wrote:

> I personally prefer the use of `target_link_libraries` rather than the custom 
> stuff added in the `add_lldb_tool`.


Sure, but the mechanism is already in place to handle this kind of thing.


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

https://reviews.llvm.org/D56741



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

[Lldb-commits] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

It seems like a pretty bad idea to have lldb-mi show a dialog box :-/

I don't actually work on or have anything to do with lldb-mi though, but I 
don't know how well-maintained or supported it is these days anyway.  But if 
nobody chimes in and says why this is important to have, I would just assume 
delete the line.


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

https://reviews.llvm.org/D56741



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

[Lldb-commits] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D56741#1358804 , @zturner wrote:

> It seems like a pretty bad idea to have lldb-mi show a dialog box :-/
>
> I don't actually work on or have anything to do with lldb-mi though, but I 
> don't know how well-maintained or supported it is these days anyway.  But if 
> nobody chimes in and says why this is important to have, I would just assume 
> delete the line.


It looks like this functionality has been available since lldb-mi was 
committed. I'd rather fix the build than modify the functionality not knowing 
if somebody actually values this behavior. Based on commit logs it looks like 
there is some amount of work that was done on lldb-mi last year, but I'm not 
really sure how much people care about lldb-mi honestly.


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

https://reviews.llvm.org/D56741



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

[Lldb-commits] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

As far as I can tell, this dialog is only used when you build the lldb-mi with 
MICONFIG_DEBUG_SHOW_ATTACH_DBG_DLG, which you would only do if you wanted to 
debug lldb-mi.  The point is to stall the lldb-mi launch at an early point till 
you've gotten a debugger attached to it.  Then you can click the MessageBox OK 
to let lldb-mi proceed.  On all the other systems it does a while (var) sleep; 
type thing, so you attach and change the value of var with the expression 
parser and continue.

While the MessageBox implementation is cute, setting the var value should work 
just as well.  It certainly doesn't seem worth dragging UI into lldb-mi just 
for this purpose.


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

https://reviews.llvm.org/D56741



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

[Lldb-commits] [lldb] r351274 - Remove redundant check.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Jan 15 15:33:26 2019
New Revision: 351274

URL: http://llvm.org/viewvc/llvm-project?rev=351274&view=rev
Log:
Remove redundant check.

Modified:
lldb/trunk/source/Core/Value.cpp

Modified: lldb/trunk/source/Core/Value.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Value.cpp?rev=351274&r1=351273&r2=351274&view=diff
==
--- lldb/trunk/source/Core/Value.cpp (original)
+++ lldb/trunk/source/Core/Value.cpp Tue Jan 15 15:33:26 2019
@@ -341,11 +341,9 @@ Status Value::GetValueAsData(ExecutionCo
 
 uint32_t limit_byte_size = UINT32_MAX;
 
-if (ast_type.IsValid()) {
-  if (llvm::Optional size = ast_type.GetByteSize(
-  exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
-limit_byte_size = *size;
-}
+if (llvm::Optional size = ast_type.GetByteSize(
+exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
+  limit_byte_size = *size;
 
 if (limit_byte_size <= m_value.GetByteSize()) {
   if (m_value.GetData(data, limit_byte_size))


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

[Lldb-commits] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D56741#1358882 , @jingham wrote:

> As far as I can tell, this dialog is only used when you build the lldb-mi 
> with MICONFIG_DEBUG_SHOW_ATTACH_DBG_DLG, which you would only do if you 
> wanted to debug lldb-mi.  The point is to stall the lldb-mi launch at an 
> early point till you've gotten a debugger attached to it.  Then you can click 
> the MessageBox OK to let lldb-mi proceed.  On all the other systems it does a 
> while (var) sleep; type thing, so you attach and change the value of var with 
> the expression parser and continue.
>
> While the MessageBox implementation is cute, setting the var value should 
> work just as well.  It certainly doesn't seem worth dragging UI into lldb-mi 
> just for this purpose.


Okay, in that case I'll go ahead and remove it like Zach originally suggested 
then. Thanks for taking a look.


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

https://reviews.llvm.org/D56741



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

[Lldb-commits] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

That seems right.  , IME on a decently fast Linux or macOS machine "attach 
-w" will get lldb attached to the process way before it would have gotten this 
far, so I'm not sure how useful this convenience is altogether.  But maybe 
"attach -w" is slower on Windows.


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

https://reviews.llvm.org/D56741



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

[Lldb-commits] [PATCH] D56741: [CMake] Explicitly list User32 as dependency of lldb-mi

[Alex Langford via Phabricator via lldb-commits]

xiaobai abandoned this revision.
xiaobai added a comment.

Abandoned in favor of D56755 .


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

https://reviews.llvm.org/D56741



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

[Lldb-commits] [lldb] r351276 - [lldb-mi] Remove use of dialog box

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Tue Jan 15 16:09:50 2019
New Revision: 351276

URL: http://llvm.org/viewvc/llvm-project?rev=351276&view=rev
Log:
[lldb-mi] Remove use of dialog box

Summary:
This really is only implemented on Windows, and it requires us to pull
in User32. This was only useful when debugging on lldb-mi on Windows, and there
doesn't seem to be a good reason why using a dialog box is better than what
exists for other platforms.

Reviewers: zturner, jingham, compnerd

Subscribers: ki.stfu

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

Modified:
lldb/trunk/tools/lldb-mi/MIDriverMain.cpp
lldb/trunk/tools/lldb-mi/MIUtilDebug.cpp
lldb/trunk/tools/lldb-mi/MIUtilDebug.h

Modified: lldb/trunk/tools/lldb-mi/MIDriverMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIDriverMain.cpp?rev=351276&r1=351275&r2=351276&view=diff
==
--- lldb/trunk/tools/lldb-mi/MIDriverMain.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIDriverMain.cpp Tue Jan 15 16:09:50 2019
@@ -165,11 +165,7 @@ bool DriverSystemShutdown(const bool vbA
 //--
 int main(int argc, char const *argv[]) {
 #if MICONFIG_DEBUG_SHOW_ATTACH_DBG_DLG
-#ifdef _WIN32
-  CMIUtilDebug::ShowDlgWaitForDbgAttach();
-#else
   CMIUtilDebug::WaitForDbgAttachInfinteLoop();
-#endif //  _WIN32
 #endif // MICONFIG_DEBUG_SHOW_ATTACH_DBG_DLG
 
   llvm::StringRef ToolName = argv[0];

Modified: lldb/trunk/tools/lldb-mi/MIUtilDebug.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIUtilDebug.cpp?rev=351276&r1=351275&r2=351276&view=diff
==
--- lldb/trunk/tools/lldb-mi/MIUtilDebug.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIUtilDebug.cpp Tue Jan 15 16:09:50 2019
@@ -39,25 +39,6 @@ CMIUtilDebug::~CMIUtilDebug() {}
 
 //++
 
//
-// Details: Show a dialog to the process/application halts. It gives the
-// opportunity to
-//  attach a debugger.
-// Type:Static method.
-// Args:None.
-// Return:  None.
-// Throws:  None.
-//--
-void CMIUtilDebug::ShowDlgWaitForDbgAttach() {
-  const CMIUtilString strCaption(CMIDriver::Instance().GetAppNameShort());
-#ifdef _WIN32
-  ::MessageBoxA(NULL, "Attach your debugger now", strCaption.c_str(), MB_OK);
-#else
-// ToDo: Implement other platform version of an Ok to continue dialog box
-#endif // _WIN32
-}
-
-//++
-//
 // Details: Temporarily stall the process/application to give the programmer 
the
 //  opportunity to attach a debugger. How to use: Put a break in the
 //  programmer

Modified: lldb/trunk/tools/lldb-mi/MIUtilDebug.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIUtilDebug.h?rev=351276&r1=351275&r2=351276&view=diff
==
--- lldb/trunk/tools/lldb-mi/MIUtilDebug.h (original)
+++ lldb/trunk/tools/lldb-mi/MIUtilDebug.h Tue Jan 15 16:09:50 2019
@@ -24,7 +24,6 @@ class CMICmnLog;
 class CMIUtilDebug {
   // Statics:
 public:
-  static void ShowDlgWaitForDbgAttach();
   static void WaitForDbgAttachInfinteLoop();
 
   // Methods:


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

[Lldb-commits] [PATCH] D56763: [CMake] Prevent lldbDebugserverCommon from building if you disable debugserver builds

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: sgraenitz, davide, JDevlieghere, beanz, vsk, aprantl, 
labath.
Herald added subscribers: jfb, mgorny.

The flags `LLDB_USE_SYSTEM_DEBUGSERVER` and `LLDB_NO_DEBUGSERVER` were
introduced to the debugserver build. If one of these two flags are set, then we
do not build and sign debugserver. However I noticed that we were still building
the lldbDebugserverCommon and lldbDebugserverCommon_NonUI libraries regardless
of whether or not these flags were set. I don't believe we should be building
these libraries unless we are building and signing debugserver.


https://reviews.llvm.org/D56763

Files:
  tools/debugserver/source/CMakeLists.txt

Index: tools/debugserver/source/CMakeLists.txt
===
--- tools/debugserver/source/CMakeLists.txt
+++ tools/debugserver/source/CMakeLists.txt
@@ -30,69 +30,6 @@
 
 add_subdirectory(MacOSX)
 
-set(generated_mach_interfaces
-  ${CMAKE_CURRENT_BINARY_DIR}/mach_exc.h
-  ${CMAKE_CURRENT_BINARY_DIR}/mach_excServer.c
-  ${CMAKE_CURRENT_BINARY_DIR}/mach_excUser.c
-  )
-add_custom_command(OUTPUT ${generated_mach_interfaces}
-  COMMAND mig ${CMAKE_CURRENT_SOURCE_DIR}/MacOSX/dbgnub-mig.defs
-  DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/MacOSX/dbgnub-mig.defs
-  )
-
-set(DEBUGSERVER_VERS_GENERATED_FILE ${CMAKE_CURRENT_BINARY_DIR}/debugserver_vers.c)
-set_source_files_properties(${DEBUGSERVER_VERS_GENERATED_FILE} PROPERTIES GENERATED 1)
-
-add_custom_command(OUTPUT ${DEBUGSERVER_VERS_GENERATED_FILE}
-  COMMAND ${LLDB_SOURCE_DIR}/scripts/generate-vers.pl
-  ${LLDB_SOURCE_DIR}/lldb.xcodeproj/project.pbxproj debugserver
-  > ${DEBUGSERVER_VERS_GENERATED_FILE}
-  DEPENDS ${LLDB_SOURCE_DIR}/scripts/generate-vers.pl
-  ${LLDB_SOURCE_DIR}/lldb.xcodeproj/project.pbxproj
-  )
-
-set(lldbDebugserverCommonSources
-  DNBArch.cpp
-  DNBBreakpoint.cpp
-  DNB.cpp
-  DNBDataRef.cpp
-  DNBError.cpp
-  DNBLog.cpp
-  DNBRegisterInfo.cpp
-  DNBThreadResumeActions.cpp
-  JSON.cpp
-  StdStringExtractor.cpp
-  # JSON reader depends on the following LLDB-common files
-  ${LLDB_SOURCE_DIR}/source/Host/common/StringConvert.cpp
-  ${LLDB_SOURCE_DIR}/source/Host/common/SocketAddress.cpp
-  # end JSON reader dependencies
-  libdebugserver.cpp
-  PseudoTerminal.cpp
-  PThreadEvent.cpp
-  PThreadMutex.cpp
-  RNBContext.cpp
-  RNBRemote.cpp
-  RNBServices.cpp
-  RNBSocket.cpp
-  SysSignal.cpp
-  TTYState.cpp
-
-  MacOSX/CFBundle.cpp
-  MacOSX/CFString.cpp
-  MacOSX/Genealogy.cpp
-  MacOSX/MachException.cpp
-  MacOSX/MachProcess.mm
-  MacOSX/MachTask.mm
-  MacOSX/MachThread.cpp
-  MacOSX/MachThreadList.cpp
-  MacOSX/MachVMMemory.cpp
-  MacOSX/MachVMRegion.cpp
-  MacOSX/OsLogger.cpp
-  ${generated_mach_interfaces}
-  ${DEBUGSERVER_VERS_GENERATED_FILE})
-
-add_library(lldbDebugserverCommon ${lldbDebugserverCommonSources})
-
 # LLDB-specific identity, currently used for code signing debugserver.
 set(LLDB_CODESIGN_IDENTITY "" CACHE STRING
 "Override code sign identity for debugserver and for use in tests; falls back to LLVM_CODESIGNING_IDENTITY if set or lldb_codesign otherwise (Darwin only)")
@@ -241,6 +178,69 @@
 endif()
 
 if(build_and_sign_debugserver)
+  set(generated_mach_interfaces
+${CMAKE_CURRENT_BINARY_DIR}/mach_exc.h
+${CMAKE_CURRENT_BINARY_DIR}/mach_excServer.c
+${CMAKE_CURRENT_BINARY_DIR}/mach_excUser.c
+)
+  add_custom_command(OUTPUT ${generated_mach_interfaces}
+COMMAND mig ${CMAKE_CURRENT_SOURCE_DIR}/MacOSX/dbgnub-mig.defs
+DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/MacOSX/dbgnub-mig.defs
+)
+
+  set(DEBUGSERVER_VERS_GENERATED_FILE ${CMAKE_CURRENT_BINARY_DIR}/debugserver_vers.c)
+  set_source_files_properties(${DEBUGSERVER_VERS_GENERATED_FILE} PROPERTIES GENERATED 1)
+
+  add_custom_command(OUTPUT ${DEBUGSERVER_VERS_GENERATED_FILE}
+COMMAND ${LLDB_SOURCE_DIR}/scripts/generate-vers.pl
+${LLDB_SOURCE_DIR}/lldb.xcodeproj/project.pbxproj debugserver
+> ${DEBUGSERVER_VERS_GENERATED_FILE}
+DEPENDS ${LLDB_SOURCE_DIR}/scripts/generate-vers.pl
+${LLDB_SOURCE_DIR}/lldb.xcodeproj/project.pbxproj
+)
+
+  set(lldbDebugserverCommonSources
+DNBArch.cpp
+DNBBreakpoint.cpp
+DNB.cpp
+DNBDataRef.cpp
+DNBError.cpp
+DNBLog.cpp
+DNBRegisterInfo.cpp
+DNBThreadResumeActions.cpp
+JSON.cpp
+StdStringExtractor.cpp
+# JSON reader depends on the following LLDB-common files
+${LLDB_SOURCE_DIR}/source/Host/common/StringConvert.cpp
+${LLDB_SOURCE_DIR}/source/Host/common/SocketAddress.cpp
+# end JSON reader dependencies
+libdebugserver.cpp
+PseudoTerminal.cpp
+PThreadEvent.cpp
+PThreadMutex.cpp
+RNBContext.cpp
+RNBRemote.cpp
+RNBServices.cpp
+RNBSocket.cpp
+SysSignal.cpp
+TTYState.cpp
+
+MacOSX/CFBundle.cpp
+MacOSX/CFString.cpp
+MacOSX/Genealogy.cpp
+MacOSX/MachException.cpp
+MacOSX/MachProcess.mm
+MacOSX/MachTask.mm
+