[Lldb-commits] [PATCH] D62501: Implement GetSharedLibraryInfoAddress

2019-06-07 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

I think this is pretty good now. The only concern I have is about the placement 
NativeProcessTestUtils. The way you're including it now looks weird. I think we 
should put that in a separate place, as per the inline coment.

In D62501#1533483 , @aadsm wrote:

> Note about the tests: I only have one test for the GetELFImageInfoAddress 
> that follows the 32bit version and also makes sure we correctly read the load 
> bias. Tbh I'm not sure if this is enough but should be easy to add more cases 
> now.


The main advantage of this kind of tests I see is that they make it possible to 
test what happens if things don't go as expected. You don't have to go 
overboard on that (you've already done a lot more here than most other code 
reviews do), but it may be nice to include one test where this lookup fails 
(e.g. the DT_DEBUG entry is missing, or something).




Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:41
 #include "lldb/Utility/StringExtractor.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Support/Errno.h"

I guess this is no longer needed.



Comment at: lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp:9
+
+#include "../../Host/NativeProcessTestUtils.h"
+

This is the second time we've needed to share code between various unit tests. 
Let's just try start a convention for putting these into a separate place and 
see how it develops. I'd say we should put this into 
`unittests/TestingSupport/Host/NativeProcessTestUtils.h` (so, #includable as 
"TestingSupport/Host/NativeProcessTestUtils.h"), for the time being.



Comment at: lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp:35
+MockProcessELF &process,
+std::vector> auxv_data) {
+  auto addr_size = process.GetAddressByteSize();

s/vector/ArrayRef



Comment at: lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp:45-46
+  }
+  llvm::StringRef stringref((const char *)buffer_sp->GetBytes(),
+buffer_sp->GetByteSize());
+  return llvm::MemoryBuffer::getMemBufferCopy(stringref, "");

you could say `toStringRef(buffer_sp->GetData())`



Comment at: lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp:61-64
+  llvm::Optional maybe_phdr_addr =
+  process.GetAuxValue(AuxVector::AUXV_AT_PHDR);
+  ASSERT_NE(llvm::None, maybe_phdr_addr);
+  ASSERT_EQ(phdr_addr, *maybe_phdr_addr);

`Optional` has overloaded equality operators which "do the right thing", though 
they are a bit tricky to use with integral types, as they kick in only if the 
contained type matches exactly. If you changed the type of `phdr_addr` to 
`uint64_t`, then you could just write `ASSERT_EQ(phdr_addr, 
process.GetAuxValue(AuxVector::AUXV_AT_PHDR))` here.



Comment at: lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp:83-117
+  // We're going to set up a fake memory with 2 program headers and 1 entry in
+  // the dynamic section.
+  // For simplicity sake they will be consecutive in memory:
+  // ++
+  // | PT_PHDR|
+  // ++
+  // | PT_DYNAMIC |

What if we just defined a struct which described the final memory layout, and 
then gave that as an argument to the FakeMemory object?

I'm thinking of something like:
```
struct MemoryContents {
  Elf32_Phdr phdr_load;
  Elf32_Phdr phdr_dynamic;
  Elf32_Dyn dyn_debug;
} MC;
MC.phdr_load.p_type = PT_DYNAMIC;
...
FakeMemory M(&MC, sizeof MC, phdr_addr); // This assumes adding a (const void 
*, size_t) constructor to the class
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62501



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

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-07 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D62502#1533490 , @aadsm wrote:

> @labath that's a really good point. I've talked with @clayborg  and @xiaobai 
> about this and we decided it would be fine to do it manually as well so I 
> added a `XMLEncodeAttributeValue` function to make it happen. I'm not 100% 
> sure about its implementation.. Should I use a StreamString instead?


The implementation is fine, just make it take a StringRef instead of a char 
pointer.

The thing I'm not sure about is the location, as now this is the only function 
in XML.h that actually works without libxml support. Given that this is 
something we don't want to advertise too broadly, maybe you could just make it 
a static function GDBRemoteCommunicationServerLLGS? If this trick stays limited 
to lldb-server (which I think it should), then that could even be the right 
place for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502



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

[Lldb-commits] [lldb] r362784 - Fix some signed/unsigned comparison warnings

2019-06-07 Thread Pavel Labath via lldb-commits 
Author: labath
Date: Fri Jun  7 02:43:53 2019
New Revision: 362784

URL: http://llvm.org/viewvc/llvm-project?rev=362784&view=rev
Log:
Fix some signed/unsigned comparison warnings

Modified:
lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

Modified: lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp?rev=362784&r1=362783&r2=362784&view=diff
==
--- lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp (original)
+++ lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp Fri Jun  7 
02:43:53 2019
@@ -118,16 +118,16 @@ TEST_F(SymbolFileDWARFTests, TestAbbrevO
   EXPECT_FALSE(bool(error));
   // Make sure we have O(1) access to each abbreviation by making sure the
   // index offset is 1 and not UINT32_MAX
-  EXPECT_EQ(abbrev_set.GetIndexOffset(), 1);
+  EXPECT_EQ(abbrev_set.GetIndexOffset(), 1u);
   
   auto abbrev1 = abbrev_set.GetAbbreviationDeclaration(1);
   EXPECT_EQ(abbrev1->Tag(), DW_TAG_compile_unit);
   EXPECT_TRUE(abbrev1->HasChildren());
-  EXPECT_EQ(abbrev1->NumAttributes(), 1);
+  EXPECT_EQ(abbrev1->NumAttributes(), 1u);
   auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(2);
   EXPECT_EQ(abbrev2->Tag(), DW_TAG_subprogram);
   EXPECT_FALSE(abbrev2->HasChildren());
-  EXPECT_EQ(abbrev2->NumAttributes(), 1);
+  EXPECT_EQ(abbrev2->NumAttributes(), 1u);
 }
 
 TEST_F(SymbolFileDWARFTests, TestAbbrevOrder1Start5) {
@@ -163,16 +163,16 @@ TEST_F(SymbolFileDWARFTests, TestAbbrevO
   EXPECT_FALSE(bool(error));
   // Make sure we have O(1) access to each abbreviation by making sure the
   // index offset is 5 and not UINT32_MAX
-  EXPECT_EQ(abbrev_set.GetIndexOffset(), 5);
+  EXPECT_EQ(abbrev_set.GetIndexOffset(), 5u);
   
   auto abbrev1 = abbrev_set.GetAbbreviationDeclaration(5);
   EXPECT_EQ(abbrev1->Tag(), DW_TAG_compile_unit);
   EXPECT_TRUE(abbrev1->HasChildren());
-  EXPECT_EQ(abbrev1->NumAttributes(), 1);
+  EXPECT_EQ(abbrev1->NumAttributes(), 1u);
   auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(6);
   EXPECT_EQ(abbrev2->Tag(), DW_TAG_subprogram);
   EXPECT_FALSE(abbrev2->HasChildren());
-  EXPECT_EQ(abbrev2->NumAttributes(), 1);
+  EXPECT_EQ(abbrev2->NumAttributes(), 1u);
 }
 
 TEST_F(SymbolFileDWARFTests, TestAbbrevOutOfOrder) {
@@ -213,11 +213,11 @@ TEST_F(SymbolFileDWARFTests, TestAbbrevO
   auto abbrev1 = abbrev_set.GetAbbreviationDeclaration(2);
   EXPECT_EQ(abbrev1->Tag(), DW_TAG_compile_unit);
   EXPECT_TRUE(abbrev1->HasChildren());
-  EXPECT_EQ(abbrev1->NumAttributes(), 1);
+  EXPECT_EQ(abbrev1->NumAttributes(), 1u);
   auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(1);
   EXPECT_EQ(abbrev2->Tag(), DW_TAG_subprogram);
   EXPECT_FALSE(abbrev2->HasChildren());
-  EXPECT_EQ(abbrev2->NumAttributes(), 1);
+  EXPECT_EQ(abbrev2->NumAttributes(), 1u);
 }
 
 TEST_F(SymbolFileDWARFTests, TestAbbrevInvalidNULLTag) {


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

[Lldb-commits] [PATCH] D62943: DWARF: Simplify SymbolFileDWARF::GetDWARFCompileUnit

2019-06-07 Thread Pavel Labath via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362783: DWARF: Simplify SymbolFileDWARF::GetDWARFCompileUnit 
(authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62943

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -608,15 +608,7 @@
   if (!comp_unit)
 return nullptr;
 
-  DWARFDebugInfo *info = DebugInfo();
-  if (info) {
-// The compile unit ID is the index of the DWARF unit.
-DWARFUnit *dwarf_cu = info->GetUnitAtIndex(comp_unit->GetID());
-if (dwarf_cu && dwarf_cu->GetUserData() == nullptr)
-  dwarf_cu->SetUserData(comp_unit);
-return dwarf_cu;
-  }
-  return nullptr;
+  return static_cast(comp_unit->GetUserData());
 }
 
 DWARFDebugRangesBase *SymbolFileDWARF::GetDebugRanges() {


Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -608,15 +608,7 @@
   if (!comp_unit)
 return nullptr;
 
-  DWARFDebugInfo *info = DebugInfo();
-  if (info) {
-// The compile unit ID is the index of the DWARF unit.
-DWARFUnit *dwarf_cu = info->GetUnitAtIndex(comp_unit->GetID());
-if (dwarf_cu && dwarf_cu->GetUserData() == nullptr)
-  dwarf_cu->SetUserData(comp_unit);
-return dwarf_cu;
-  }
-  return nullptr;
+  return static_cast(comp_unit->GetUserData());
 }
 
 DWARFDebugRangesBase *SymbolFileDWARF::GetDebugRanges() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r362783 - DWARF: Simplify SymbolFileDWARF::GetDWARFCompileUnit

2019-06-07 Thread Pavel Labath via lldb-commits 
Author: labath
Date: Fri Jun  7 02:43:47 2019
New Revision: 362783

URL: http://llvm.org/viewvc/llvm-project?rev=362783&view=rev
Log:
DWARF: Simplify SymbolFileDWARF::GetDWARFCompileUnit

Summary:
The DWARFCompileUnit is set as the "user data" of the lldb compile unit
directly in the constructor (see ParseCompileUnit).

This means that instead of going through unit indexes, we can just fetch
the DWARF unit directly from there.

Reviewers: clayborg, JDevlieghere

Subscribers: aprantl, jdoerfert, lldb-commits

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=362783&r1=362782&r2=362783&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Fri Jun  7 
02:43:47 2019
@@ -608,15 +608,7 @@ SymbolFileDWARF::GetDWARFCompileUnit(lld
   if (!comp_unit)
 return nullptr;
 
-  DWARFDebugInfo *info = DebugInfo();
-  if (info) {
-// The compile unit ID is the index of the DWARF unit.
-DWARFUnit *dwarf_cu = info->GetUnitAtIndex(comp_unit->GetID());
-if (dwarf_cu && dwarf_cu->GetUserData() == nullptr)
-  dwarf_cu->SetUserData(comp_unit);
-return dwarf_cu;
-  }
-  return nullptr;
+  return static_cast(comp_unit->GetUserData());
 }
 
 DWARFDebugRangesBase *SymbolFileDWARF::GetDebugRanges() {


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

[Lldb-commits] [PATCH] D62948: lit/Register: Avoid stdio in register write tests

2019-06-07 Thread Pavel Labath via Phabricator via lldb-commits 
labath planned changes to this revision.
labath added a comment.

It looks like this problem is more widespread than we originally thought (a 
bunch of other tests are affected too). I'll need to think whether we can come 
up with a more general solution.

In D62948#1533088 , @jgorbe wrote:

> About %T not working for "process launch", what about something like `RUN: 
> %lldb -b --one-line-before-file "process launch --stdout 
> %T/x86-zmm-write.out" -s %s %t` and then FileCheck-ing?


There are two issues with that. The first one is that with --one-line, lldb 
stops processing commands after hitting the int3 instruction. I think this is 
related to the fact that lldb aborts the script if the inferior crashes, and it 
considers an unexpected int3 instruction to be a "crash". However, it's not 
clear to me why should the behavior depend on whether the command is in the 
script or on the command line, so this may be a bug actually.

The second one is that putting complex commands on the command line creates a 
bit of a quoting nightmare, as the command is quote-processed both by the shell 
and by lldb. And shells (particularly windows ones) differ in how they handle 
that. If %T doesn't contain any funny characters, then everything is fine (and 
I expect a lot of our tests would fail if it did, so we kind of already assume 
that). However, I am reluctant to recommend that as the best practice for 
handling these kinds of things.


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

https://reviews.llvm.org/D62948



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

[Lldb-commits] [PATCH] D63005: DWARF: Don't create lldb CompileUnits for DWARF type units

2019-06-07 Thread Pavel Labath via Phabricator via lldb-commits 
labath created this revision.
labath added reviewers: clayborg, JDevlieghere, aprantl.
Herald added a subscriber: jdoerfert.
labath added a parent revision: D62894: DWARF: Share line tables of type units.

Type units don't represent actual compilations and a lot of the
operations that we do with lldb compile units (getting their line
tables, variables, etc.) don't make sense for them. There is also a lot
more of them (sometimes over 100x), so making them more lightweight pays
off.

The main change in this patch is that we stop creating lldb CompileUnits
for DWARF type units. The trickiest part here is that the SymbolFile
interface requires that we assign consecutive sequence IDs to the
compile units we create. As DWARF type and compile units can come in any
order (in v5), this means we can no longer use 1-1 mapping between DWARF
and lldb compile units. Instead I build a translation table between the
two indices. To avoid pessimizing the case where there are no type
units, I build the translation table only in case we have at least one
type unit.

Additionaly, I also tried to strenghted type safete by replacing
DWARFUnit with DWARFCompileUnit where applicable. Though that was not
stricly necessary, I found it a good way to ensure that the
transformations I am doing here make sense. In the places where I was
changing the function signatures, and where it was obvious that the
objects being handled were not null, I also replaced pointers with
references.

There shouldn't be any major functional change with this patch. The only
change I observed is that now the types in the type units will not be
parsed when one calls Module::ParseAllDebugSymbols, unless they are
referenced from other compile units. This makes sense, given how
ParseAllDebugSymbols is implemented (it iterates over all compile
units), and it only matters for one hand-writted test where I did not
bother to reference the types from the compile units (which I now do).


https://reviews.llvm.org/D63005

Files:
  lit/SymbolFile/DWARF/debug-types-address-ranges.s
  lit/SymbolFile/DWARF/debug-types-line-tables.s
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
@@ -24,7 +24,7 @@
   Create(lldb::ModuleSP module_sp, const lldb_private::FileSpec &file_spec);
 
   std::unique_ptr
-  GetSymbolFileForDwoId(DWARFUnit *dwarf_cu, uint64_t dwo_id);
+  GetSymbolFileForDwoId(DWARFCompileUnit &dwarf_cu, uint64_t dwo_id);
 
   bool LoadSectionData(uint64_t dwo_id, lldb::SectionType sect_type,
lldb_private::DWARFDataExtractor &data);
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
@@ -81,7 +81,7 @@
 {}
 
 std::unique_ptr
-SymbolFileDWARFDwp::GetSymbolFileForDwoId(DWARFUnit *dwarf_cu,
+SymbolFileDWARFDwp::GetSymbolFileForDwoId(DWARFCompileUnit &dwarf_cu,
   uint64_t dwo_id) {
   return std::unique_ptr(
   new SymbolFileDWARFDwoDwp(this, m_obj_file, dwarf_cu, dwo_id));
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.h
@@ -15,7 +15,7 @@
 class SymbolFileDWARFDwoDwp : public SymbolFileDWARFDwo {
 public:
   SymbolFileDWARFDwoDwp(SymbolFileDWARFDwp *dwp_symfile,
-lldb::ObjectFileSP objfile, DWARFUnit *dwarf_cu,
+lldb::ObjectFileSP objfile, DWARFCompileUnit &dwarf_cu,
 uint64_t dwo_id);
 
 protected:
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.cpp
@@ -21,7 +21,7 @@
 
 SymbolFileDWARFDwoDwp::SymbolFileDWARFDwoDwp(SymbolFileDWARFDwp *dwp_symfile,
  ObjectFileSP objfile,
-

[Lldb-commits] [PATCH] D62395: WIP: Dont vend lldb_private::CompileUnits for DWARFTypeUnits

2019-06-07 Thread Pavel Labath via Phabricator via lldb-commits 
labath abandoned this revision.
labath added a comment.

Obsoleted by https://reviews.llvm.org/D63005.


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

https://reviews.llvm.org/D62395



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

[Lldb-commits] [lldb] r362803 - [CMake] Add special case for processing LLDB_DOTEST_ARGS

2019-06-07 Thread Stefan Granitz via lldb-commits 
Author: stefan.graenitz
Date: Fri Jun  7 07:32:51 2019
New Revision: 362803

URL: http://llvm.org/viewvc/llvm-project?rev=362803&view=rev
Log:
[CMake] Add special case for processing LLDB_DOTEST_ARGS

Summary:
Allow to run the test suite when building LLDB Standalone with Xcode against a 
provided LLVM build-tree that used a single-configuration generator like Ninja.

So far both test drivers, lit-based `check-lldb` as well as `lldb-dotest`, were 
looking for test dependencies (clang/dsymutil/etc.) in a subdirectory with the 
configuration name (Debug/Release/etc.). It was implicitly assuming that both, 
LLDB and the provided LLVM used the same generator. In practice, however, the 
opposite is quite common: build the dependencies with Ninja and LLDB with Xcode 
for development*. With this patch it becomes the default.

(* In fact, it turned out that the Xcode<->Xcode variant didn't even build out 
of the box. It's fixed since D62879)

Once this is sound, I'm planning the following steps:
* add stage to the [lldb-cmake-standalone 
bot](http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-standalone/) to 
build and test it
* update `Building LLDB with Xcode` section in the docs
* bring the same mechanism to swift-lldb
* fade out the manually maintained Xcode project

On macOS build and test like this:
```
$ git clone https://github.com/llvm/llvm-project.git /path/to/llvm-project

$ cd /path/to/lldb-dev-deps
$ cmake -GNinja 
-C/path/to/llvm-project/lldb/cmake/caches/Apple-lldb-macOS.cmake 
-DLLVM_ENABLE_PROJECTS="clang;libcxx;libcxxabi" /path/to/llvm-project/llvm
$ ninja

$ cd /path/to/lldb-dev-xcode
$ cmake -GXcode 
-C/path/to/llvm-project/lldb/cmake/caches/Apple-lldb-macOS.cmake 
-DLLDB_BUILD_FRAMEWORK=Off -DLLVM_DIR=/path/to/lldb-dev-deps/lib/cmake/llvm 
-DClang_DIR=/path/to/lldb-dev-deps/lib/cmake/clang /path/to/llvm-project/lldb
$ xcodebuild -configuration Debug -target check-lldb
$ xcodebuild -configuration Debug -target lldb-dotest
$ ./Debug/bin/lldb-dotest
```

Reviewers: JDevlieghere, jingham, xiaobai, stella.stamenova, labath

Reviewed By: stella.stamenova

Subscribers: labath, mgorny, lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/lit/CMakeLists.txt
lldb/trunk/utils/lldb-dotest/CMakeLists.txt

Modified: lldb/trunk/lit/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/CMakeLists.txt?rev=362803&r1=362802&r2=362803&view=diff
==
--- lldb/trunk/lit/CMakeLists.txt (original)
+++ lldb/trunk/lit/CMakeLists.txt Fri Jun  7 07:32:51 2019
@@ -1,6 +1,7 @@
 # Test runner infrastructure for LLDB. This configures the LLDB test trees
 # for use by Lit, and delegates to LLVM's lit test handlers.
 
+# LLVM_BUILD_MODE is used in lit.site.cfg
 if (CMAKE_CFG_INTDIR STREQUAL ".")
   set(LLVM_BUILD_MODE ".")
 else ()
@@ -12,10 +13,35 @@ if (CMAKE_SIZEOF_VOID_P EQUAL 8)
 endif()
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
+set(dotest_args_replacement ${LLVM_BUILD_MODE})
 
+if(LLDB_BUILT_STANDALONE)
+  # In paths to our build-tree, replace CMAKE_CFG_INTDIR with our 
configuration name placeholder.
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} 
config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
+  string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} 
LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
+
+  # Remaining ones must be paths to the provided LLVM build-tree.
+  if(LLVM_CONFIGURATION_TYPES)
+# LLDB uses single-config; LLVM multi-config; pick one and prefer Release 
types.
+# Otherwise, if both use multi-config the default is fine.
+if(NOT CMAKE_CONFIGURATION_TYPES)
+  if(RelWithDebInfo IN_LIST LLVM_CONFIGURATION_TYPES)
+set(dotest_args_replacement RelWithDebInfo)
+  elseif(Release IN_LIST LLVM_CONFIGURATION_TYPES)
+set(dotest_args_replacement Release)
+  else()
+list(GET LLVM_CONFIGURATION_TYPES 0 dotest_args_replacement)
+  endif()
+endif()
+  else()
+# Common case: LLVM used a single-configuration generator like Ninja.
+set(dotest_args_replacement ".")
+  endif()
+endif()
+
+string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
-string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 
 list(APPEND LLDB_TEST_DEPS
   LLDBUnitTests

Modified: lldb/trunk/utils/lldb-dotest/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/utils/lldb-dotest/CMakeLists.txt?rev=362803&r1=362802&r2=362803&view=diff
==
--- lldb/trunk/utils/lldb-dotest/CMakeLists.txt (original)

[Lldb-commits] [PATCH] D62859: [CMake] Add special case for processing LLDB_DOTEST_ARGS

2019-06-07 Thread Phabricator via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362803: [CMake] Add special case for processing 
LLDB_DOTEST_ARGS (authored by stefan.graenitz, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62859?vs=203121&id=203557#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62859

Files:
  lldb/trunk/lit/CMakeLists.txt
  lldb/trunk/utils/lldb-dotest/CMakeLists.txt


Index: lldb/trunk/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/trunk/utils/lldb-dotest/CMakeLists.txt
+++ lldb/trunk/utils/lldb-dotest/CMakeLists.txt
@@ -5,8 +5,28 @@
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
-# Generate wrapper for each build mode.
-if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+# Generate lldb-dotest Python driver script for each build mode.
+if(LLDB_BUILT_STANDALONE)
+  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+# In paths to our build-tree, replace CMAKE_CFG_INTDIR with our actual 
configuration names.
+string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} 
config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
+string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} 
LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
+
+# Remaining ones must be paths to the provided LLVM build-tree.
+if(${config_type} IN_LIST LLVM_CONFIGURATION_TYPES)
+  # Multi-configuration generator like Xcode (with a matching config).
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+else()
+  # Single-configuration generator like Ninja.
+  string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
+endif()
+
+configure_file(
+  lldb-dotest.in
+  ${config_runtime_output_dir}/lldb-dotest @ONLY
+)
+  endforeach()
+elseif(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
   foreach(LLVM_BUILD_MODE ${CMAKE_CONFIGURATION_TYPES})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
Index: lldb/trunk/lit/CMakeLists.txt
===
--- lldb/trunk/lit/CMakeLists.txt
+++ lldb/trunk/lit/CMakeLists.txt
@@ -1,6 +1,7 @@
 # Test runner infrastructure for LLDB. This configures the LLDB test trees
 # for use by Lit, and delegates to LLVM's lit test handlers.
 
+# LLVM_BUILD_MODE is used in lit.site.cfg
 if (CMAKE_CFG_INTDIR STREQUAL ".")
   set(LLVM_BUILD_MODE ".")
 else ()
@@ -12,10 +13,35 @@
 endif()
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
+set(dotest_args_replacement ${LLVM_BUILD_MODE})
 
+if(LLDB_BUILT_STANDALONE)
+  # In paths to our build-tree, replace CMAKE_CFG_INTDIR with our 
configuration name placeholder.
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} 
config_runtime_output_dir ${LLVM_RUNTIME_OUTPUT_INTDIR})
+  string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} 
LLDB_DOTEST_ARGS "${LLDB_DOTEST_ARGS}")
+
+  # Remaining ones must be paths to the provided LLVM build-tree.
+  if(LLVM_CONFIGURATION_TYPES)
+# LLDB uses single-config; LLVM multi-config; pick one and prefer Release 
types.
+# Otherwise, if both use multi-config the default is fine.
+if(NOT CMAKE_CONFIGURATION_TYPES)
+  if(RelWithDebInfo IN_LIST LLVM_CONFIGURATION_TYPES)
+set(dotest_args_replacement RelWithDebInfo)
+  elseif(Release IN_LIST LLVM_CONFIGURATION_TYPES)
+set(dotest_args_replacement Release)
+  else()
+list(GET LLVM_CONFIGURATION_TYPES 0 dotest_args_replacement)
+  endif()
+endif()
+  else()
+# Common case: LLVM used a single-configuration generator like Ninja.
+set(dotest_args_replacement ".")
+  endif()
+endif()
+
+string(REPLACE ${CMAKE_CFG_INTDIR} ${dotest_args_replacement} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR})
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
-string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS 
"${LLDB_DOTEST_ARGS}")
 
 list(APPEND LLDB_TEST_DEPS
   LLDBUnitTests


Index: lldb/trunk/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/trunk/utils/lldb-dotest/CMakeLists.txt
+++ lldb/trunk/utils/lldb-dotest/CMakeLists.txt
@@ -5,8 +5,28 @@
 
 get_property(LLDB_DOTEST_ARGS GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY)
 
-# Generate wrapper for each build mode.
-if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+# Generate lldb-dotest Python driver script for each build mode.
+if(LLDB_BUILT_STANDA

[Lldb-commits] [PATCH] D62934: [LanguageRuntime] Introdce LLVM-style casts

2019-06-07 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: 
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp:1128
 
-  RenderScriptRuntime *lang_rt =
-  (RenderScriptRuntime *)exe_ctx.GetProcessPtr()->GetLanguageRuntime(
-  eLanguageTypeExtRenderScript);
+  RenderScriptRuntime *lang_rt = llvm::cast_or_null(
+  exe_ctx.GetProcessPtr()->GetLanguageRuntime(

nit: the code (in this whole file) is clearly assuming the pointer is non-null, 
so you don't need the _or_null part. :)


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

https://reviews.llvm.org/D62934



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

[Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

2019-06-07 Thread Guilherme Andrade via Phabricator via lldb-commits 
guiandrade added a comment.

In D62931#1532510 , @clayborg wrote:

> In D62931#1531965 , @labath wrote:
>
> >
>
>
> There are so many various GDB remote servers and it is hard to say what will 
> work for most people. Might be worth testing this with the setting set to 
> true and false on a complex program single step on mac, linux and android and 
> verifying that stepping speeds don't regress at all. Android tends to have 
> hundreds of threads which usually makes it the slowest of all when stepping. 
> If we don't see regressions, then I am fine with no setting. Every scenario I 
> know of benefits from fewer packets with larger content (macOS, watchOS, 
> linux, android) so I am fine just enabling it. Maybe it would be good to 
> check what GDB does by default as it might tell us, via comments, why they 
> don't use it if they did run into issues.


How can we run those tests? Is there an automated way to do that?

> So I am ok with no setting as long as perf doesn't regress on complex steps 
> and as long as GDB uses 'g' packets by default.

I think they first try to use 'g' packets - 
https://github.com/bminor/binutils-gdb/blob/4fa0265edea0940b865678d93749e224547dd36a/gdb/remote.c#L8130
  (I'm not familiar with that code at all, so I may be wrong)




Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:1722
+thread_sp = std::make_shared(
+*this, tid, GetGlobalPluginProperties()->GetPacketTimeout());
 LLDB_LOGV(log, "Making new thread: {0} for thread ID: {1:x}.",

note to self: fix wrong method call



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:1839
+thread_sp = std::make_shared(
+*this, tid, GetGlobalPluginProperties()->GetPacketTimeout());
 m_thread_list_real.AddThread(thread_sp);

note to self: fix wrong method call


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62931



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

[Lldb-commits] [PATCH] D62894: DWARF: Share line tables of type units

2019-06-07 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:309
+  decl.SetFile(
+  die.GetDWARF()->GetSupportFile(*die.GetCU(), form_value.Unsigned()));
   break;

Maybe make a "FileSpec DWARFDie::GetFile(uint32_t idx);" function? 



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:388
+static std::string GetUnitName(const DWARFDIE &die) {
+  std::string result = "the source file";
+  DWARFUnit *unit = die.GetCU();

Maybe:
```
std::string result = "";
```



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:392-395
+  if (FileSpec spec =
+  unit->GetCompilationDirectory().CopyByAppendingPathComponent(
+  unit->GetUnitDIEOnly().GetName()))
+return spec.GetPath();

Maybe create a DWARFUnit function?:
```
const lldb_private::FileSpec &DWARFUnit::GetFileSpec();
```




Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:393
+  if (FileSpec spec =
+  unit->GetCompilationDirectory().CopyByAppendingPathComponent(
+  unit->GetUnitDIEOnly().GetName()))

Does this do the right thing if DW_AT_name is absolute?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2266-2267
 case DW_AT_decl_file:
-  decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
+  decl.SetFile(die.GetDWARF()->GetSupportFile(
+  *die.GetCU(), form_value.Unsigned()));
   break;

Use new DWARFDie::GetFile()



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2424-2425
 decl_up.reset(new Declaration(
-comp_unit.GetSupportFiles().GetFileSpecAtIndex(decl_file),
-decl_line, decl_column));
+die.GetDWARF()->GetSupportFile(*die.GetCU(), decl_file), decl_line,
+decl_column));
 

Use new DWARFDie::GetFile()



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:286-306
   dw_addr_t addr_base = cu_die.GetAttributeValueAsUnsigned(
   this, DW_AT_addr_base, LLDB_INVALID_ADDRESS);
   if (addr_base != LLDB_INVALID_ADDRESS)
 SetAddrBase(addr_base);
 
   dw_addr_t ranges_base = cu_die.GetAttributeValueAsUnsigned(
   this, DW_AT_rnglists_base, LLDB_INVALID_ADDRESS);

Many attributes being individually fetched here. This is slow. We should 
probably iterate over all attributes?



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:812-813
+  if (offset == DW_INVALID_OFFSET ||
+  offset == llvm::DenseMapInfo::getEmptyKey() ||
+  offset == llvm::DenseMapInfo::getTombstoneKey())
+return empty_list;

why are these checks needed? Remove? Maybe left over from previous approach?



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:303
 
+  lldb_private::FileSpec GetSupportFile(DWARFUnit &unit, size_t file_idx);
+

Can probably just be named GetFile


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

https://reviews.llvm.org/D62894



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

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

2019-06-07 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

In D58678#1530031 , @lanza wrote:

> @clayborg Seems like this still steps into the `call` if the call is the last 
> instruction in the range. `ThreadPlanStepRange::SetNextBranchBreakpoint` 
> checks `if (last_index - pc_index > 1)` before setting the breakpoint. So if 
> `last_index == pc_index` and `pc` points to `call` then the thread plan will 
> resort to single stepping and thus go through all the same machinery. 
> Obviously, this isn't a problem as this just leads to using the same 
> functionality that it used prior to this patch, but you miss out on the 
> optimization you're aiming for.


Thanks for the heads up. Will come up with a fix ASAP


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58678



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

[Lldb-commits] [lldb] r362843 - [lldb] Fix msan use-of-uninitialized-value in DWARFDebugLine::FileNameEntry.

2019-06-07 Thread Jorge Gorbe Moya via lldb-commits 
Author: jgorbe
Date: Fri Jun  7 14:09:30 2019
New Revision: 362843

URL: http://llvm.org/viewvc/llvm-project?rev=362843&view=rev
Log:
[lldb] Fix msan use-of-uninitialized-value in DWARFDebugLine::FileNameEntry.

lldb/lit/SymbolFile/DWARF/debug-types-expressions.test fails with msan.
This change fixes the issue by ensuring FileNameEntry::checksum is
always default-initialized.

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h?rev=362843&r1=362842&r2=362843&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h Fri Jun  7 
14:09:30 2019
@@ -29,7 +29,8 @@ class DWARFDebugLine {
 public:
   // FileNameEntry
   struct FileNameEntry {
-FileNameEntry() : name(nullptr), dir_idx(0), mod_time(0), length(0) {}
+FileNameEntry()
+: name(nullptr), dir_idx(0), mod_time(0), length(0), checksum() {}
 
 const char *name;
 dw_sleb128_t dir_idx;


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

[Lldb-commits] [PATCH] D62887: Update the thread list before setting stop reasons with an OS plugin

2019-06-07 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added inline comments.



Comment at: source/Target/Process.cpp:3037-3041
+  // Somebody might have gotten threads before now, but we need to force the
+  // update after we've loaded the OperatingSystem plugin or it won't get a
+  // chance to process the threads.
+  m_thread_list.Clear();
+  UpdateThreadListIfNeeded();

Should this be in the above if statement?

```
if (!m_os_up) {
LoadOperatingSystemPlugin(false);
// Somebody might have gotten threads before now, but we need to force the
// update after we've loaded the OperatingSystem plugin or it won't get a
// chance to process the threads.
m_thread_list.Clear();
UpdateThreadListIfNeeded();
}
```

And if we do this in the if statement, do we need to clear the m_thread_list?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62887



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

[Lldb-commits] [lldb] r362844 - Fix lit tests on Windows related to CR+LF

2019-06-07 Thread Adrian McCarthy via lldb-commits 
Author: amccarth
Date: Fri Jun  7 14:13:30 2019
New Revision: 362844

URL: http://llvm.org/viewvc/llvm-project?rev=362844&view=rev
Log:
Fix lit tests on Windows related to CR+LF

Problem discovered in the breakpoint lit test, but probably exists in others.
lldb-test splits lines on LF.  Input files that are CR+LF separated (as is
common on Windows) then resulted in commands being sent to LLDB that ended
in CR, which confused the command interpreter.

This could be fixed at different levels:

1.  Treat '\r' like a tab or space in the argument splitter.
2.  Fix the line splitters (plural) in lldb-test.
3.  Normalize the test files to LF only.

If we did only 3, I'd expect similar problems to recur, so this patch does
1 and 2.  I may also do 3 in a separate patch later, but that's tricky
because I believe we have some input files that MUST use CR+LF.

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

Modified:
lldb/trunk/source/Utility/Args.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/source/Utility/Args.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Args.cpp?rev=362844&r1=362843&r2=362844&view=diff
==
--- lldb/trunk/source/Utility/Args.cpp (original)
+++ lldb/trunk/source/Utility/Args.cpp Fri Jun  7 14:13:30 2019
@@ -95,7 +95,7 @@ ParseSingleArgument(llvm::StringRef comm
   bool arg_complete = false;
   do {
 // Skip over over regular characters and append them.
-size_t regular = command.find_first_of(" \t\"'`\\");
+size_t regular = command.find_first_of(" \t\r\"'`\\");
 arg += command.substr(0, regular);
 command = command.substr(regular);
 
@@ -123,6 +123,7 @@ ParseSingleArgument(llvm::StringRef comm
 
 case ' ':
 case '\t':
+case '\r':
   // We are not inside any quotes, we just found a space after an argument.
   // We are done.
   arg_complete = true;

Modified: lldb/trunk/tools/lldb-test/lldb-test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-test/lldb-test.cpp?rev=362844&r1=362843&r2=362844&view=diff
==
--- lldb/trunk/tools/lldb-test/lldb-test.cpp (original)
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp Fri Jun  7 14:13:30 2019
@@ -312,7 +312,7 @@ int opts::breakpoint::evaluateBreakpoint
   while (!Rest.empty()) {
 StringRef Line;
 std::tie(Line, Rest) = Rest.split('\n');
-Line = Line.ltrim();
+Line = Line.ltrim().rtrim();
 if (Line.empty() || Line[0] == '#')
   continue;
 
@@ -939,7 +939,7 @@ int opts::irmemorymap::evaluateMemoryMap
   while (!Rest.empty()) {
 StringRef Line;
 std::tie(Line, Rest) = Rest.split('\n');
-Line = Line.ltrim();
+Line = Line.ltrim().rtrim();
 
 if (Line.empty() || Line[0] == '#')
   continue;


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

[Lldb-commits] [lldb] r362845 - NFC: Fix typo in a cmake message

2019-06-07 Thread Adrian McCarthy via lldb-commits 
Author: amccarth
Date: Fri Jun  7 14:14:01 2019
New Revision: 362845

URL: http://llvm.org/viewvc/llvm-project?rev=362845&view=rev
Log:
NFC:  Fix typo in a cmake message

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

Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=362845&r1=362844&r2=362845&view=diff
==
--- lldb/trunk/cmake/modules/LLDBConfig.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake Fri Jun  7 14:14:01 2019
@@ -181,7 +181,7 @@ function(find_python_libs_windows)
   set (PYTHON_DLL ${PYTHON_DLL} PARENT_SCOPE)
   set (PYTHON_INCLUDE_DIR ${PYTHON_INCLUDE_DIR} PARENT_SCOPE)
 
-  message("-- LLDB Found PythonExecutable: ${PYTHON_EXE}}")
+  message("-- LLDB Found PythonExecutable: ${PYTHON_EXE}")
   message("-- LLDB Found PythonLibs: ${PYTHON_LIB}")
   message("-- LLDB Found PythonDLL: ${PYTHON_DLL}")
   message("-- LLDB Found PythonIncludeDirs: ${PYTHON_INCLUDE_DIR}")


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

[Lldb-commits] [PATCH] D62759: Fix lit tests on Windows related to CR

2019-06-07 Thread Adrian McCarthy via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362844: Fix lit tests on Windows related to CR+LF (authored 
by amccarth, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62759?vs=202505&id=203623#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62759

Files:
  lldb/trunk/source/Utility/Args.cpp
  lldb/trunk/tools/lldb-test/lldb-test.cpp


Index: lldb/trunk/tools/lldb-test/lldb-test.cpp
===
--- lldb/trunk/tools/lldb-test/lldb-test.cpp
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp
@@ -312,7 +312,7 @@
   while (!Rest.empty()) {
 StringRef Line;
 std::tie(Line, Rest) = Rest.split('\n');
-Line = Line.ltrim();
+Line = Line.ltrim().rtrim();
 if (Line.empty() || Line[0] == '#')
   continue;
 
@@ -939,7 +939,7 @@
   while (!Rest.empty()) {
 StringRef Line;
 std::tie(Line, Rest) = Rest.split('\n');
-Line = Line.ltrim();
+Line = Line.ltrim().rtrim();
 
 if (Line.empty() || Line[0] == '#')
   continue;
Index: lldb/trunk/source/Utility/Args.cpp
===
--- lldb/trunk/source/Utility/Args.cpp
+++ lldb/trunk/source/Utility/Args.cpp
@@ -95,7 +95,7 @@
   bool arg_complete = false;
   do {
 // Skip over over regular characters and append them.
-size_t regular = command.find_first_of(" \t\"'`\\");
+size_t regular = command.find_first_of(" \t\r\"'`\\");
 arg += command.substr(0, regular);
 command = command.substr(regular);
 
@@ -123,6 +123,7 @@
 
 case ' ':
 case '\t':
+case '\r':
   // We are not inside any quotes, we just found a space after an argument.
   // We are done.
   arg_complete = true;


Index: lldb/trunk/tools/lldb-test/lldb-test.cpp
===
--- lldb/trunk/tools/lldb-test/lldb-test.cpp
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp
@@ -312,7 +312,7 @@
   while (!Rest.empty()) {
 StringRef Line;
 std::tie(Line, Rest) = Rest.split('\n');
-Line = Line.ltrim();
+Line = Line.ltrim().rtrim();
 if (Line.empty() || Line[0] == '#')
   continue;
 
@@ -939,7 +939,7 @@
   while (!Rest.empty()) {
 StringRef Line;
 std::tie(Line, Rest) = Rest.split('\n');
-Line = Line.ltrim();
+Line = Line.ltrim().rtrim();
 
 if (Line.empty() || Line[0] == '#')
   continue;
Index: lldb/trunk/source/Utility/Args.cpp
===
--- lldb/trunk/source/Utility/Args.cpp
+++ lldb/trunk/source/Utility/Args.cpp
@@ -95,7 +95,7 @@
   bool arg_complete = false;
   do {
 // Skip over over regular characters and append them.
-size_t regular = command.find_first_of(" \t\"'`\\");
+size_t regular = command.find_first_of(" \t\r\"'`\\");
 arg += command.substr(0, regular);
 command = command.substr(regular);
 
@@ -123,6 +123,7 @@
 
 case ' ':
 case '\t':
+case '\r':
   // We are not inside any quotes, we just found a space after an argument.
   // We are done.
   arg_complete = true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62894: DWARF: Share line tables of type units

2019-06-07 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:814
+  offset == llvm::DenseMapInfo::getTombstoneKey())
+return empty_list;
+

`return {};` ?


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

https://reviews.llvm.org/D62894



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

[Lldb-commits] [PATCH] D63005: DWARF: Don't create lldb CompileUnits for DWARF type units

2019-06-07 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:726
   DWARFDebugInfo *info = DebugInfo();
   if (info) {
+BuildCuTranslationTable();

```
if (!info) 
  return {};
``` 
?


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

https://reviews.llvm.org/D63005



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

[Lldb-commits] [lldb] r362862 - Revert "DWARF: Simplify SymbolFileDWARF::GetDWARFCompileUnit"

2019-06-07 Thread Alex Langford via lldb-commits 
Author: xiaobai
Date: Fri Jun  7 17:55:03 2019
New Revision: 362862

URL: http://llvm.org/viewvc/llvm-project?rev=362862&view=rev
Log:
Revert "DWARF: Simplify SymbolFileDWARF::GetDWARFCompileUnit"

This reverts commit 58afc1bdebf9fa8b178d6c9d89af94c5cc091760.
This commit caused the test suite on macOS to fail many tests. It
appears that setting breakpoints is the issue. One example that fails
is the lit test Breakpoint/case-sensitive.test.

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=362862&r1=362861&r2=362862&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Fri Jun  7 
17:55:03 2019
@@ -608,7 +608,15 @@ SymbolFileDWARF::GetDWARFCompileUnit(lld
   if (!comp_unit)
 return nullptr;
 
-  return static_cast(comp_unit->GetUserData());
+  DWARFDebugInfo *info = DebugInfo();
+  if (info) {
+// The compile unit ID is the index of the DWARF unit.
+DWARFUnit *dwarf_cu = info->GetUnitAtIndex(comp_unit->GetID());
+if (dwarf_cu && dwarf_cu->GetUserData() == nullptr)
+  dwarf_cu->SetUserData(comp_unit);
+return dwarf_cu;
+  }
+  return nullptr;
 }
 
 DWARFDebugRangesBase *SymbolFileDWARF::GetDebugRanges() {


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

[Lldb-commits] [PATCH] D62943: DWARF: Simplify SymbolFileDWARF::GetDWARFCompileUnit

2019-06-07 Thread Alex Langford via Phabricator via lldb-commits 
xiaobai added a comment.

Just a heads up: I reverted this change (rL362862 
) because it broke the test suite on macOS. 
Breakpoints were not being resolved correctly.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62943



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

[Lldb-commits] [PATCH] D62501: Implement GetSharedLibraryInfoAddress

2019-06-07 Thread António Afonso via Phabricator via lldb-commits 
aadsm updated this revision to Diff 203653.
aadsm marked 2 inline comments as done.
aadsm added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62501

Files:
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/POSIX/CMakeLists.txt
  lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
  lldb/source/Plugins/Process/POSIX/NativeProcessELF.h
  lldb/unittests/Host/NativeProcessProtocolTest.cpp
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/POSIX/CMakeLists.txt
  lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp
  lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
  patches.tgz

Index: lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
===
--- /dev/null
+++ lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
@@ -0,0 +1,150 @@
+//===-- NativeProcessTestUtils.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef lldb_unittests_Host_NativeProcessTestUtils_h_
+#define lldb_unittests_Host_NativeProcessTestUtils_h_
+
+#include "lldb/Host/common/NativeProcessProtocol.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+
+using namespace lldb_private;
+using namespace lldb;
+using namespace testing;
+
+namespace lldb_private {
+
+class MockDelegate : public NativeProcessProtocol::NativeDelegate {
+public:
+  MOCK_METHOD1(InitializeDelegate, void(NativeProcessProtocol *Process));
+  MOCK_METHOD2(ProcessStateChanged,
+   void(NativeProcessProtocol *Process, StateType State));
+  MOCK_METHOD1(DidExec, void(NativeProcessProtocol *Process));
+};
+
+// NB: This class doesn't use the override keyword to avoid
+// -Winconsistent-missing-override warnings from the compiler. The
+// inconsistency comes from the overriding definitions in the MOCK_*** macros.
+template  class MockProcess : public T {
+public:
+  MockProcess(NativeProcessProtocol::NativeDelegate &Delegate,
+  const ArchSpec &Arch, lldb::pid_t Pid = 1)
+  : T(Pid, -1, Delegate), Arch(Arch) {}
+
+  MOCK_METHOD1(Resume, Status(const ResumeActionList &ResumeActions));
+  MOCK_METHOD0(Halt, Status());
+  MOCK_METHOD0(Detach, Status());
+  MOCK_METHOD1(Signal, Status(int Signo));
+  MOCK_METHOD0(Kill, Status());
+  MOCK_METHOD3(AllocateMemory,
+   Status(size_t Size, uint32_t Permissions, addr_t &Addr));
+  MOCK_METHOD1(DeallocateMemory, Status(addr_t Addr));
+  MOCK_METHOD0(GetSharedLibraryInfoAddress, addr_t());
+  MOCK_METHOD0(UpdateThreads, size_t());
+  MOCK_CONST_METHOD0(GetAuxvData,
+ llvm::ErrorOr>());
+  MOCK_METHOD2(GetLoadedModuleFileSpec,
+   Status(const char *ModulePath, FileSpec &Spec));
+  MOCK_METHOD2(GetFileLoadAddress,
+   Status(const llvm::StringRef &FileName, addr_t &Addr));
+
+  const ArchSpec &GetArchitecture() const /*override*/ { return Arch; }
+  Status SetBreakpoint(lldb::addr_t Addr, uint32_t Size,
+   bool Hardware) /*override*/ {
+if (Hardware)
+  return this->SetHardwareBreakpoint(Addr, Size);
+else
+  return this->SetSoftwareBreakpoint(Addr, Size);
+  }
+
+  // Redirect base class Read/Write Memory methods to functions whose signatures
+  // are more mock-friendly.
+  Status ReadMemory(addr_t Addr, void *Buf, size_t Size,
+size_t &BytesRead) /*override*/ {
+auto ExpectedMemory = this->ReadMemory(Addr, Size);
+if (!ExpectedMemory) {
+  BytesRead = 0;
+  return Status(ExpectedMemory.takeError());
+}
+BytesRead = ExpectedMemory->size();
+assert(BytesRead <= Size);
+std::memcpy(Buf, ExpectedMemory->data(), BytesRead);
+return Status();
+  }
+
+  Status WriteMemory(addr_t Addr, const void *Buf, size_t Size,
+ size_t &BytesWritten) /*override*/ {
+auto ExpectedBytes = this->WriteMemory(
+Addr, llvm::makeArrayRef(static_cast(Buf), Size));
+if (!ExpectedBytes) {
+  BytesWritten = 0;
+  return Status(ExpectedBytes.takeError());
+}
+BytesWritten = *ExpectedBytes;
+return Status();
+  }
+
+  MOCK_METHOD2(ReadMemory,
+   llvm::Expected>(addr_t Addr, size_t Size));
+  MOCK_METHOD2(WriteMemory,
+   llvm::Expected(addr_t Addr,
+  llvm::ArrayRef Data));
+
+  using T::GetSoftwareBreakpointTrapOpcode;
+  llvm::Expected> ReadMemoryWithoutTrap(addr_t Addr,
+ size_t Size) {
+std::v

[Lldb-commits] [PATCH] D62501: Implement GetSharedLibraryInfoAddress

2019-06-07 Thread António Afonso via Phabricator via lldb-commits 
aadsm updated this revision to Diff 203654.
aadsm added a comment.

Removed unwanted change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62501

Files:
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/POSIX/CMakeLists.txt
  lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
  lldb/source/Plugins/Process/POSIX/NativeProcessELF.h
  lldb/unittests/Host/NativeProcessProtocolTest.cpp
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/POSIX/CMakeLists.txt
  lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp
  lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h

Index: lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
===
--- /dev/null
+++ lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
@@ -0,0 +1,150 @@
+//===-- NativeProcessTestUtils.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef lldb_unittests_Host_NativeProcessTestUtils_h_
+#define lldb_unittests_Host_NativeProcessTestUtils_h_
+
+#include "lldb/Host/common/NativeProcessProtocol.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+
+using namespace lldb_private;
+using namespace lldb;
+using namespace testing;
+
+namespace lldb_private {
+
+class MockDelegate : public NativeProcessProtocol::NativeDelegate {
+public:
+  MOCK_METHOD1(InitializeDelegate, void(NativeProcessProtocol *Process));
+  MOCK_METHOD2(ProcessStateChanged,
+   void(NativeProcessProtocol *Process, StateType State));
+  MOCK_METHOD1(DidExec, void(NativeProcessProtocol *Process));
+};
+
+// NB: This class doesn't use the override keyword to avoid
+// -Winconsistent-missing-override warnings from the compiler. The
+// inconsistency comes from the overriding definitions in the MOCK_*** macros.
+template  class MockProcess : public T {
+public:
+  MockProcess(NativeProcessProtocol::NativeDelegate &Delegate,
+  const ArchSpec &Arch, lldb::pid_t Pid = 1)
+  : T(Pid, -1, Delegate), Arch(Arch) {}
+
+  MOCK_METHOD1(Resume, Status(const ResumeActionList &ResumeActions));
+  MOCK_METHOD0(Halt, Status());
+  MOCK_METHOD0(Detach, Status());
+  MOCK_METHOD1(Signal, Status(int Signo));
+  MOCK_METHOD0(Kill, Status());
+  MOCK_METHOD3(AllocateMemory,
+   Status(size_t Size, uint32_t Permissions, addr_t &Addr));
+  MOCK_METHOD1(DeallocateMemory, Status(addr_t Addr));
+  MOCK_METHOD0(GetSharedLibraryInfoAddress, addr_t());
+  MOCK_METHOD0(UpdateThreads, size_t());
+  MOCK_CONST_METHOD0(GetAuxvData,
+ llvm::ErrorOr>());
+  MOCK_METHOD2(GetLoadedModuleFileSpec,
+   Status(const char *ModulePath, FileSpec &Spec));
+  MOCK_METHOD2(GetFileLoadAddress,
+   Status(const llvm::StringRef &FileName, addr_t &Addr));
+
+  const ArchSpec &GetArchitecture() const /*override*/ { return Arch; }
+  Status SetBreakpoint(lldb::addr_t Addr, uint32_t Size,
+   bool Hardware) /*override*/ {
+if (Hardware)
+  return this->SetHardwareBreakpoint(Addr, Size);
+else
+  return this->SetSoftwareBreakpoint(Addr, Size);
+  }
+
+  // Redirect base class Read/Write Memory methods to functions whose signatures
+  // are more mock-friendly.
+  Status ReadMemory(addr_t Addr, void *Buf, size_t Size,
+size_t &BytesRead) /*override*/ {
+auto ExpectedMemory = this->ReadMemory(Addr, Size);
+if (!ExpectedMemory) {
+  BytesRead = 0;
+  return Status(ExpectedMemory.takeError());
+}
+BytesRead = ExpectedMemory->size();
+assert(BytesRead <= Size);
+std::memcpy(Buf, ExpectedMemory->data(), BytesRead);
+return Status();
+  }
+
+  Status WriteMemory(addr_t Addr, const void *Buf, size_t Size,
+ size_t &BytesWritten) /*override*/ {
+auto ExpectedBytes = this->WriteMemory(
+Addr, llvm::makeArrayRef(static_cast(Buf), Size));
+if (!ExpectedBytes) {
+  BytesWritten = 0;
+  return Status(ExpectedBytes.takeError());
+}
+BytesWritten = *ExpectedBytes;
+return Status();
+  }
+
+  MOCK_METHOD2(ReadMemory,
+   llvm::Expected>(addr_t Addr, size_t Size));
+  MOCK_METHOD2(WriteMemory,
+   llvm::Expected(addr_t Addr,
+  llvm::ArrayRef Data));
+
+  using T::GetSoftwareBreakpointTrapOpcode;
+  llvm::Expected> ReadMemoryWithoutTrap(addr_t Addr,
+ size_t Size) {
+std::vector Data(Size, 0);
+size_t BytesRead;
+

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-07 Thread António Afonso via Phabricator via lldb-commits 
aadsm updated this revision to Diff 203656.
aadsm added a comment.

Move XMLEncodeAttributeValue to GDBRemoteCommunicationServerLLGS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/main.cpp
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_a.cpp
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_a.mk
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_b_quote.cpp
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/svr4lib_b_quote.mk
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -196,6 +196,8 @@
   llvm::Expected>
   ReadXferObject(llvm::StringRef object, llvm::StringRef annex);
 
+  static std::string XMLEncodeAttributeValue(llvm::StringRef value);
+
 private:
   void HandleInferiorState_Exited(NativeProcessProtocol *process);
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2765,6 +2765,24 @@
 return std::move(*buffer_or_error);
   }
 
+  if (object == "libraries-svr4") {
+auto library_list = m_debugged_process_up->GetLoadedSVR4Libraries();
+if (!library_list)
+  return library_list.takeError();
+
+StreamString response;
+response.Printf("");
+for (auto const &library : *library_list) {
+  response.Printf("", library.ld_addr);
+}
+response.Printf("");
+return MemoryBuffer::getMemBufferCopy(response.GetString(), __FUNCTION__);
+  }
+
   return llvm::make_error(
   "Xfer object not supported");
 }
@@ -3283,3 +3301,28 @@
 
   return GDBRemoteCommunicationServerCommon::FindModuleFile(module_path, arch);
 }
+
+std::string GDBRemoteCommunicationServerLLGS::XMLEncodeAttributeValue(
+llvm::StringRef value) {
+  std::string result;
+  for (const char &c : value) {
+switch (c) {
+case '\'':
+  result += "'";
+  break;
+case '"':
+  result += """;
+  break;
+case '<':
+  result += "<";
+  break;
+case '>':
+  result += ">";
+  break;
+default:
+  result += c;
+  break;
+}
+  }
+  return result;
+}
\ No newline at end of file
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -826,6 +826,9 @@
   response.PutCString(";QPassSignals+");
   response.PutCString(";qXfer:auxv:read+");
 #endif
+#if defined(__linux__)
+  response.PutCString(";qXfer:libraries-svr4:read+");
+#endif
 
   return SendPacketNoLock(response.GetString());
 }
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -76,6 +76,9 @@
 
   Status DeallocateMemory(lldb::addr_t addr) override;
 
+  llvm::Expected>
+  GetLoadedSVR4Libraries() override;
+
   size_t UpdateThreads() override;
 
   const ArchSpec &GetArchitecture() const override { return m_arch; }
@@ -127,6 +130,10 @@
   llvm::Expected>
   GetSoftwareBreakpointTrapOpcode(size_t size_hint) override;
 
+  template 
+  Status ReadSVR4LibraryInfo(lldb::addr_t link_map_addr, SVR4LibraryInfo &info,
+ lldb::addr_t &next);
+
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugin

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-07 Thread António Afonso via Phabricator via lldb-commits 
aadsm added a comment.

Sounds good, I put it there initially because the XML.cpp set of classes 
abstract the libxml2 specific away but I guess it is odd to have 
XMLDocument::XMLEnabled() returning false and still be able to call that new 
function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502



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

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

2019-06-07 Thread António Afonso via Phabricator via lldb-commits 
aadsm updated this revision to Diff 203658.
aadsm added a comment.

Fix a little bug and add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62503

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/unittests/Host/NativeProcessProtocolTest.cpp

Index: lldb/unittests/Host/NativeProcessProtocolTest.cpp
===
--- lldb/unittests/Host/NativeProcessProtocolTest.cpp
+++ lldb/unittests/Host/NativeProcessProtocolTest.cpp
@@ -96,3 +96,39 @@
   EXPECT_THAT_EXPECTED(Process.ReadMemoryWithoutTrap(4, 2),
llvm::HasValue(std::vector{4, 5}));
 }
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory) {
+  NiceMock DummyDelegate;
+  MockProcess Process(DummyDelegate,
+ ArchSpec("aarch64-pc-linux"));
+  FakeMemory M{{'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'}};
+  EXPECT_CALL(Process, ReadMemory(_, _))
+  .WillRepeatedly(Invoke(&M, &FakeMemory::Read));
+
+  char string[1024];
+  size_t bytes_read;
+  EXPECT_THAT_ERROR(
+  Process.ReadCStringFromMemory(0x0, &string[0], sizeof(string), bytes_read)
+  .ToError(),
+  llvm::Succeeded());
+  EXPECT_STREQ(string, "hello");
+  EXPECT_EQ(bytes_read, 6UL);
+}
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory_MaxSize) {
+  NiceMock DummyDelegate;
+  MockProcess Process(DummyDelegate,
+ ArchSpec("aarch64-pc-linux"));
+  FakeMemory M{{'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'}};
+  EXPECT_CALL(Process, ReadMemory(_, _))
+  .WillRepeatedly(Invoke(&M, &FakeMemory::Read));
+
+  char string[4];
+  size_t bytes_read;
+  EXPECT_THAT_ERROR(
+  Process.ReadCStringFromMemory(0x0, &string[0], sizeof(string), bytes_read)
+  .ToError(),
+  llvm::Succeeded());
+  EXPECT_STREQ(string, "hel");
+  EXPECT_EQ(bytes_read, 3UL);
+}
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -2090,11 +2090,12 @@
 return error;
 
   char name_buffer[PATH_MAX];
-  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
- bytes_read);
+  error = ReadCStringFromMemory(link_map.l_name,
+reinterpret_cast(&name_buffer),
+sizeof(name_buffer), bytes_read);
   if (!error.Success())
 return error;
-  name_buffer[PATH_MAX - 1] = '\0';
+
   info.name = std::string(name_buffer);
   info.link_map = link_map_addr;
   info.base_addr = link_map.l_addr;
Index: lldb/source/Host/common/NativeProcessProtocol.cpp
===
--- lldb/source/Host/common/NativeProcessProtocol.cpp
+++ lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -16,6 +16,8 @@
 #include "lldb/Utility/State.h"
 #include "lldb/lldb-enumerations.h"
 
+#include "llvm/Support/Process.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -659,6 +661,52 @@
   return Status();
 }
 
+Status NativeProcessProtocol::ReadCStringFromMemory(lldb::addr_t addr,
+char *buffer,
+size_t max_size,
+size_t &total_bytes_read) {
+  const size_t cache_line_size = llvm::sys::Process::getPageSizeEstimate();
+  size_t bytes_read = 0;
+  size_t bytes_left = max_size;
+  addr_t curr_addr = addr;
+  char *curr_buffer = buffer;
+  total_bytes_read = 0;
+  Status error;
+
+  while (bytes_left > 0 && error.Success()) {
+addr_t cache_line_bytes_left =
+cache_line_size - (curr_addr % cache_line_size);
+addr_t bytes_to_read = std::min(bytes_left, cache_line_bytes_left);
+error = ReadMemory(curr_addr, reinterpret_cast(curr_buffer),
+   bytes_to_read, bytes_read);
+
+if (bytes_read == 0)
+  break;
+
+auto str_end = std::memchr(curr_buffer, '\0', bytes_read);
+if (str_end != NULL) {
+  total_bytes_read =
+  (size_t)(reinterpret_cast(str_end) - buffer + 1);
+  error.Clear();
+  break;
+}
+
+total_bytes_read += bytes_read;
+curr_buffer += bytes_read;
+curr_addr = reinterpret_cast(reinterpret_cast(curr_addr) +
+ bytes_read);
+bytes_left -= bytes_read;
+  }
+
+  // Make sure we return a null terminated string.
+  if (bytes_left == 0 && buffer[max_size - 1] != '\0') {
+buffer[max_size - 1] = '\0';
+total_bytes_read--;
+  }
+
+  return error;
+}
+
 lldb::StateType NativeProcessProtocol::GetState() const {
   std::lock_gua