date:20190613

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

2019-06-13 Thread Pavel Labath via Phabricator via lldb-commits

labath added a reviewer: aprantl.
labath added a comment.

+ Adrian, as I believe we touched this code recently...




Comment at: include/lldb/Target/LanguageRuntime.h:160-162
+  /// Identify whether a name is a runtime value that should not be hidden by
+  /// from the user interface.
+  virtual bool IsWhitelistedRuntimeValue(ConstString name) { return false; }

Could we make this a non-public extension point, and make it so that it is 
automatically invoked from `IsRuntimeSupportValue` ?

That would simplify the code on the caller side..



Comment at: source/Target/CPPLanguageRuntime.cpp:47-59
-  // All runtime support values have to be marked as artificial by the
-  // compiler. But not all artificial variables should be hidden from
-  // the user.
-  if (!valobj.GetVariable())
-return false;
-  if (!valobj.GetVariable()->IsArtificial())
-return false;

I don't think you're preserving the semantics here. Previously If 
`GetObjectRuntimeLanguage()` returned "C++", we would still end up consulting 
the objc white list. However, now you don't do that..

The new code actually makes more sense to me, but I'm guessing there was a 
reason things were implemented this way. However, I'm not that familiar with 
this code, so I'm not sure what to do about that, or whether it even matters...


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

https://reviews.llvm.org/D63240



___
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-13 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D62503#1540959 , @aadsm wrote:

> Yeah, that's a good question and I did think about it at the time I wrote 
> D62715 . Here's my rationale:
>  Regarding the chunk reading in the ReadMemory I was not sure because I don't 
> know how common that is and it is faster to read the entire size without 
> chunking (the code is also less complex this way). I'd imagine that in the 
> common case we usually know what we're reading? (e.g.: reading the entire 
> function assembly code)
>  That's why I thought about the string "extraction" as a special case for 
> memory reading. However, even if we had a ReadMemory that did chunking I'd 
> still want to do chunking in the ReadCStringFromMemory. Otherwise we'd be 
> reading much more than needed for a string because there is no way for the 
> ReadMemory to know it could stop (after finding a '\0') so it might end up 
> reading say 2KB word by word with ptrace. (I did think for 5s to pass an 
> optional stop character to ReadMemory but that would be shoehorning an edge 
> case into a generic and simple function, it would be better to put the chunk 
> reading logic into a separate thing that both ReadMemory and 
> ReadCStringFromMemory could reuse).


Ok, I think I understand what you mean. It does make sense to do chunked reads 
here, as most strings will be relatively short in most cases, so it does not 
make sense to read full 4k bytes only to find the null terminator on byte 2. 
However, in this case one would expect to have some fixed chunk size (128, or 
whatever would be the "common" string length), and read the data like that. So 
seeing page size being used as input here is surprising (and it means you can 
still end up reading 4k bytes slowly, if you happen to start reading near the 
beginning of a page). However, I also get why you're implementing it this way 
-- you're betting on the fact that most read will end up in the "fast" memory, 
and just want to avoid wandering into the "slow" part needlessly. That sounds 
like a reasonable thing to assume. So overall, I think you've convinced me 
about this, and we can keep this implementation, until evidence saying 
otherwise shows up.




Comment at: lldb/unittests/Host/NativeProcessProtocolTest.cpp:100
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory) {
+  NiceMock DummyDelegate;

It would be nice to have one more test where reads cross the page boundary, to 
make sure chunks are concatenated properly.



Comment at: lldb/unittests/Host/NativeProcessProtocolTest.cpp:128-133
+  EXPECT_THAT_ERROR(
+  Process.ReadCStringFromMemory(0x0, &string[0], sizeof(string), 
bytes_read)
+  .ToError(),
+  llvm::Succeeded());
+  EXPECT_STREQ(string, "hel");
+  EXPECT_EQ(bytes_read, 3UL);

aadsm wrote:
> labath wrote:
> > I'm wondering how will the caller know that the read has been truncated. 
> > Given that ReadMemory already returns an error in case of partial reads, 
> > maybe we could do the same here ? (return an error, but still set 
> > bytes_read and the string as they are now). What do you think ?
> I'm a bit hesitant on this one, because it's different semantics.
> ReadMemory returns an error when it was not able to read everything that it 
> was told to.
> 
> In this test case we were able to read everything but didn't find a null 
> terminator. I decided to set a null terminator in this case and say that we 
> read 1 less byte in order to 1) actually have a C string (I don't want people 
> to strlen it and fail) and 2) allow people to read in chunks if they want 
> (however, I don't know how useful this will be if at all) or allow them to 
> partially read a string or even to realize that the string is larger than 
> originally expected.
> 
> There is a way to know if it didn't read a full string with `strlen(string) 
> == bytes_read` but that'd be O(n). You can also do `bytes_read == sizeof-1 && 
> string[size-2] != '\0'` but it kind of sucks.
> I have no good solution here :D unless we want to return an enum with the 3 
> different options.
> In this test case we were able to read everything but ...
Is that really true? We were told to read a c string. We did not find a c 
string, so instead we chose to mangle the data that we have read into a c 
string. That seems a bit error-ish.

I also find it confusing that in this (truncated) case the returned value 
corresponds with the length of the string, while in the "normal" case in 
includes the nul terminator. I think it would be more natural to do it the 
other way around, as that would be similar to what snprintf does.

How about we avoid this issue entirely, and have the function return 
`Expected` (the StringRef being backed by the buffer you pass in as 
the argument) instead? This way you don't have to do any null termination, as 
the StringRef carries the length implicitly. A

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

2019-06-13 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, with the following comment taken into account.

The ProcessFreeBSD parts are definitely wrong, as ProcessFreeBSD does not use 
lldb-server yet (it uses local only path). Just revert that part.

As for NetBSD, doing that might be fine, but I'd also be fine with leaving it 
up to the netbsd maintainers to enable this feature (by inheriting from 
NativeProcessELF). In other words, my request was only to make it *possible* 
for free/netbsd to use this code (by putting it into NativeProcessELF). I 
didn't mean they have to start using it *right now*.


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] r363250 - DWARF: Don't create lldb CompileUnits for DWARF type units

2019-06-13 Thread Pavel Labath via lldb-commits

Author: labath
Date: Thu Jun 13 04:22:47 2019
New Revision: 363250

URL: http://llvm.org/viewvc/llvm-project?rev=363250&view=rev
Log:
DWARF: Don't create lldb CompileUnits for DWARF type units

Summary:
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).

Reviewers: clayborg, JDevlieghere, aprantl

Subscribers: jdoerfert, lldb-commits

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

Added:
lldb/trunk/lit/SymbolFile/DWARF/debug-types-dwarf5.s
Modified:
lldb/trunk/lit/SymbolFile/DWARF/debug-types-line-tables.s
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h

Added: lldb/trunk/lit/SymbolFile/DWARF/debug-types-dwarf5.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/debug-types-dwarf5.s?rev=363250&view=auto
==
--- lldb/trunk/lit/SymbolFile/DWARF/debug-types-dwarf5.s (added)
+++ lldb/trunk/lit/SymbolFile/DWARF/debug-types-dwarf5.s Thu Jun 13 04:22:47 
2019
@@ -0,0 +1,94 @@
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
+# RUN: %lldb %t -o "image lookup -v -s f1" -o exit | FileCheck %s
+
+# CHECK:  Function: id = {0x7fff003c}, name = "f1", range = 
[0x-0x0001)
+# CHECK:Blocks: id = {0x7fff003c}, range = [0x-0x0001)
+
+
+.text
+.globl  f1
+.type   f1,@function
+f1:
+nop
+.Lfunc_end0:
+.size   f1, .Lfunc_end0-f1
+# -- End function
+.section.debug_str,"MS",@progbits,1
+.Lproducer:
+.asciz  "Hand-written DWARF"
+.Lf1:
+.asciz  "f1"
+.Le1:
+.asciz  "e1"
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   1   # DW_CHILDREN_yes
+.byte   37  # DW_AT_producer
+.byte   14  # DW_FORM_strp
+.byte   17  # DW_AT_low_pc
+.byte   1   # DW_FORM_addr
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   2   # Abbreviation Code
+.byte   46  # DW_TAG_subprogram
+.byte   0   # DW_CHILDREN_no
+.byte   17  # DW_AT_low_pc
+

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

2019-06-13 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363250: DWARF: Don't create lldb CompileUnits for DWARF 
type units (authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63005?vs=203820&id=204483#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63005

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

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -13,11 +13,11 @@
 
 class SymbolFileDWARFDwo : public SymbolFileDWARF {
 public:
-  SymbolFileDWARFDwo(lldb::ObjectFileSP objfile, DWARFUnit *dwarf_cu);
+  SymbolFileDWARFDwo(lldb::ObjectFileSP objfile, DWARFCompileUnit &dwarf_cu);
 
   ~SymbolFileDWARFDwo() override = default;
 
-  lldb::CompUnitSP ParseCompileUnit(DWARFUnit *dwarf_cu) override;
+  lldb::CompUnitSP ParseCompileUnit(DWARFCompileUnit &dwarf_cu) override;
 
   DWARFUnit *GetCompileUnit();
 
@@ -42,7 +42,7 @@
 return nullptr;
   }
 
-  DWARFUnit *GetBaseCompileUnit() override;
+  DWARFCompileUnit *GetBaseCompileUnit() override { return &m_base_dwarf_cu; }
 
 protected:
   void LoadSectionData(lldb::SectionType sect_type,
@@ -68,7 +68,7 @@
   SymbolFileDWARF *GetBaseSymbolFile();
 
   lldb::ObjectFileSP m_obj_file_sp;
-  DWARFUnit *m_base_dwarf_cu;
+  DWARFCompileUnit &m_base_dwarf_cu;
 };
 
 #endif // SymbolFileDWARFDwo_SymbolFileDWARFDwo_h_
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
+++ lldb/trunk/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: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -13,18 +13,19 @@
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
 
-#include "DWARFUnit.h"
+#include "DWARFCompileUnit.h"
 #include "DWARFDebugInfo.h"
+#include "DWARFUnit.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 SymbolFileDWARFDwo::SymbolFileDWARFDwo(ObjectFileSP objfile,
-   DWARFUnit *dwarf_cu)
+   DWARFCompileUnit &dwarf_cu)
 : SymbolFileDWARF(objfile.get(), objfile->GetSectionList(
  /*update_module_section_list*/ false)),
   m_obj_file_sp(objfile), m_base_dwarf_cu(dwarf_cu) {
-  SetID(((lldb::user_id_t)dwarf_cu->GetID()) << 32);
+  SetID(((lldb::user_id_t)dwarf_cu.GetID()) << 32);
 }
 
 void SymbolFileDWARFDwo::LoadSectionData(lldb::SectionType sect_type,
@@ -45,10 +46,11 @@
   SymbolFileDWARF::LoadSectionData(sect_type, data);
 }
 
-lldb::CompUnitSP SymbolFileDWARFDwo::ParseCompileUnit(DWARFUnit *dwarf_cu) {
-  assert(GetCompileUnit() == dwarf_cu && "SymbolFileDWARFDwo::ParseCompileUnit "
- "called with incompatible compile "
- "unit");
+lldb::CompUnitSP
+SymbolFileDWARFDwo::ParseCompileUnit(DWARFCompileUnit &dwarf_cu) {
+  assert(GetCompileUnit() == &dwarf_cu &&
+ "SymbolFileDWARFDwo::ParseCompileUnit called with incompatible "
+ "compile unit");
   return GetBaseSymbolFile()->ParseCompileUnit(m_base_dwarf_cu);
 }
 
@@ -106,12 +108,8 @@
   die, type_name, must_be_implementa

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

2019-06-13 Thread Kamil Rytarowski via Phabricator via lldb-commits

krytarowski added a comment.

I would leave the NetBSD version as it is in this patch and let us to fix/test 
it later.


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] D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

2019-06-13 Thread Pavel Labath via Phabricator via lldb-commits

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

The motivation for this was me wanting to make the validity of dwarf
DIERefs explicit (via llvm::Optional). This meant that the class
would no longer have a default constructor. As the DIERef was being
stored in a UniqueCStringMap, this meant that this container (like all
standard containers) needed to work with non-default-constructible types
too.

This part is achieved by removing die default constructors for the map
entry types, and providing appropriate comparison overloads so that we
can search for map entries without constructing a dummy entry. While
doing that, I took the opportunity to modernize the code, and add some
tests. Functions that were completely unused are deleted.

This required also some changes in the Symtab code, as it was default
constructing map entries, which was not impossible even though its
value type was default-constructible. Technically, these changes could
be avoided with some SFINAE on the entry type, but I felt that the code
is cleaner this way anyway.


https://reviews.llvm.org/D63268

Files:
  include/lldb/Core/UniqueCStringMap.h
  include/lldb/Symbol/Symtab.h
  source/Symbol/Symtab.cpp
  unittests/Core/CMakeLists.txt
  unittests/Core/UniqueCStringMapTest.cpp

Index: unittests/Core/UniqueCStringMapTest.cpp
===
--- /dev/null
+++ unittests/Core/UniqueCStringMapTest.cpp
@@ -0,0 +1,53 @@
+//===-- UniqueCStringMapTest.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
+//
+//===--===//
+
+#include "lldb/Core/UniqueCStringMap.h"
+#include "gmock/gmock.h"
+
+using namespace lldb_private;
+
+namespace {
+struct NoDefault {
+  int x;
+
+  NoDefault(int x) : x(x) {}
+  NoDefault() = delete;
+
+  friend bool operator==(NoDefault lhs, NoDefault rhs) {
+return lhs.x == rhs.x;
+  }
+
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+   NoDefault x) {
+return OS << "NoDefault{" << x.x << "}";
+  }
+};
+} // namespace
+
+TEST(UniqueCStringMap, NoDefaultConstructor) {
+  using MapT = UniqueCStringMap;
+  using EntryT = MapT::Entry;
+
+  MapT Map;
+  ConstString Foo("foo"), Bar("bar");
+
+  Map.Append(Foo, NoDefault(42));
+  EXPECT_THAT(Map.Find(Foo, NoDefault(47)), NoDefault(42));
+  EXPECT_THAT(Map.Find(Bar, NoDefault(47)), NoDefault(47));
+  EXPECT_THAT(Map.FindFirstValueForName(Foo),
+  testing::Pointee(testing::Field(&EntryT::value, NoDefault(42;
+  EXPECT_THAT(Map.FindFirstValueForName(Bar), nullptr);
+
+  std::vector Values;
+  EXPECT_THAT(Map.GetValues(Foo, Values), 1);
+  EXPECT_THAT(Values, testing::ElementsAre(NoDefault(42)));
+
+  Values.clear();
+  EXPECT_THAT(Map.GetValues(Bar, Values), 0);
+  EXPECT_THAT(Values, testing::IsEmpty());
+}
Index: unittests/Core/CMakeLists.txt
===
--- unittests/Core/CMakeLists.txt
+++ unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
   MangledTest.cpp
   RichManglingContextTest.cpp
   StreamCallbackTest.cpp
+  UniqueCStringMapTest.cpp
 
   LINK_LIBS
 lldbCore
Index: source/Symbol/Symtab.cpp
===
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -288,10 +288,8 @@
 // Instantiation of the demangler is expensive, so better use a single one
 // for all entries during batch processing.
 RichManglingContext rmc;
-NameToIndexMap::Entry entry;
-
-for (entry.value = 0; entry.value < num_symbols; ++entry.value) {
-  Symbol *symbol = &m_symbols[entry.value];
+for (uint32_t value = 0; value < num_symbols; ++value) {
+  Symbol *symbol = &m_symbols[value];
 
   // Don't let trampolines get into the lookup by name map If we ever need
   // the trampoline symbols to be searchable by name we can remove this and
@@ -303,52 +301,46 @@
   // If the symbol's name string matched a Mangled::ManglingScheme, it is
   // stored in the mangled field.
   Mangled &mangled = symbol->GetMangled();
-  entry.cstring = mangled.GetMangledName();
-  if (entry.cstring) {
-m_name_to_index.Append(entry);
+  if (ConstString name = mangled.GetMangledName()) {
+m_name_to_index.Append(name, value);
 
 if (symbol->ContainsLinkerAnnotations()) {
   // If the symbol has linker annotations, also add the version without
   // the annotations.
-  entry.cstring = ConstString(m_objfile->StripLinkerSymbolAnnotations(
-entry.cstring.GetStringRef()));
-  m_name_to_index.Append(

[Lldb-commits] [lldb] r363271 - [CMake] Add fallbacks for copying clang-resource-headers to LLDB.framework in standalone builds

2019-06-13 Thread Stefan Granitz via lldb-commits

Author: stefan.graenitz
Date: Thu Jun 13 08:07:56 2019
New Revision: 363271

URL: http://llvm.org/viewvc/llvm-project?rev=363271&view=rev
Log:
[CMake] Add fallbacks for copying clang-resource-headers to LLDB.framework in 
standalone builds

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

Modified: lldb/trunk/cmake/modules/LLDBFramework.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBFramework.cmake?rev=363271&r1=363270&r2=363271&view=diff
==
--- lldb/trunk/cmake/modules/LLDBFramework.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBFramework.cmake Thu Jun 13 08:07:56 2019
@@ -84,11 +84,33 @@ add_custom_command(TARGET lldb-framework
 )
 
 # Copy vendor-specific headers from clang (without staging).
-if(NOT IOS AND NOT LLDB_BUILT_STANDALONE)
-  add_dependencies(lldb-framework clang-resource-headers)
+if(NOT IOS)
+  if (TARGET clang-resource-headers)
+add_dependencies(lldb-framework clang-resource-headers)
+set(clang_resource_headers_dir 
$)
+  else()
+# In standalone builds try the best possible guess
+if(Clang_DIR)
+  set(clang_lib_dir ${Clang_DIR}/../..)
+elseif(LLVM_DIR)
+  set(clang_lib_dir ${LLVM_DIR}/../..)
+elseif(LLVM_LIBRARY_DIRS)
+  set(clang_lib_dir ${LLVM_LIBRARY_DIRS})
+elseif(LLVM_BUILD_LIBRARY_DIR)
+  set(clang_lib_dir ${LLVM_BUILD_LIBRARY_DIR})
+elseif(LLVM_BINARY_DIR)
+  set(clang_lib_dir ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
+endif()
+set(clang_version 
${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH})
+set(clang_resource_headers_dir 
${clang_lib_dir}/clang/${clang_version}/include)
+if(NOT EXISTS ${clang_resource_headers_dir})
+  message(WARNING "Expected directory for clang-resource headers not 
found: ${clang_resource_headers_dir}")
+endif()
+  endif()
+
   add_custom_command(TARGET lldb-framework POST_BUILD
 COMMAND ${CMAKE_COMMAND} -E copy_directory
-$
+${clang_resource_headers_dir}
 $/Resources/Clang/include
 COMMENT "LLDB.framework: copy clang vendor-specific headers"
   )


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

[Lldb-commits] [PATCH] D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

2019-06-13 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: include/lldb/Core/UniqueCStringMap.h:87
   T Find(ConstString unique_cstr, T fail_value) const {
-Entry search_entry(unique_cstr);
-const_iterator end = m_map.end();
-const_iterator pos = std::lower_bound(m_map.begin(), end, search_entry);
-if (pos != end) {
-  if (pos->cstring == unique_cstr)
-return pos->value;
-}
+auto pos = llvm::lower_bound(m_map, unique_cstr, Compare());
+if (pos != m_map.end() && pos->cstring == unique_cstr)

What benefits does llvm::lower_bound offer here? With std::lower_bound and 
std::upper_bound you can write a comparison static functions that take a "const 
T &" on one side and a ConstString on the other. Lower bound will use one 
flavor ("static bool operator <(const T&, ConstString);") and upper_bound will 
use the other ("static bool operator <(ConstString, const T&);"). No need to 
specify a Compare() object. So the code would be:
```
T Find(ConstString unique_cstr, T fail_value) const {
  const_iterator end = m_map.end();
  const_iterator pos = std::lower_bound(m_map.begin(), end, unique_cstr);
  if (pos != end && pos->cstring == unique_cstr)
return pos->value;
  return fail_value;
}
```


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

https://reviews.llvm.org/D63268



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

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

2019-06-13 Thread Greg Clayton via Phabricator via lldb-commits

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: source/Core/ValueObject.cpp:1702-1724
+  return runtime->IsRuntimeSupportValue(*this) &&
+ !runtime->IsWhitelistedRuntimeValue(GetName());
+
+// It's possible we have more than one language involved here. For example,
+// in ObjC `_cmd` is a whitelisted runtime variable name, but
+// GetObjectRuntimeLanguage will say it's a C variable since it's just a
+// cstring.

Seems like this should still just make one call. The two calls to 
IsRuntimeSupportValue followed by IsWhitelistedRuntimeValue seems like a bunch 
of hard to read code. We should make just one call to each runtime and do all 
the work needed from within there. So I vote to revert this change and do the 
work in each IsRuntimeSupportValue method. See additional comments in 
CPPLanguageRuntime::IsRuntimeSupportValue() that was removed. 



Comment at: source/Target/CPPLanguageRuntime.cpp:59
-return false;
-  return !ObjCLanguageRuntime::IsWhitelistedRuntimeValue(name);
 }

Seems like we should restore this function and just get the ObjC language 
runtime from the process, and if it exists, call IsWhitelistedRuntimeValue on 
it?

```
auto objc_runtime = ObjCLanguageRuntime::Get(m_process);
if (objc_runtime)
  return objc_runtime->IsWhitelistedRuntimeValue(name);
return false;
```


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

https://reviews.llvm.org/D63240



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

[Lldb-commits] [PATCH] D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

2019-06-13 Thread Pavel Labath via Phabricator via lldb-commits

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



Comment at: include/lldb/Core/UniqueCStringMap.h:87
   T Find(ConstString unique_cstr, T fail_value) const {
-Entry search_entry(unique_cstr);
-const_iterator end = m_map.end();
-const_iterator pos = std::lower_bound(m_map.begin(), end, search_entry);
-if (pos != end) {
-  if (pos->cstring == unique_cstr)
-return pos->value;
-}
+auto pos = llvm::lower_bound(m_map, unique_cstr, Compare());
+if (pos != m_map.end() && pos->cstring == unique_cstr)

clayborg wrote:
> What benefits does llvm::lower_bound offer here? With std::lower_bound and 
> std::upper_bound you can write a comparison static functions that take a 
> "const T &" on one side and a ConstString on the other. Lower bound will use 
> one flavor ("static bool operator <(const T&, ConstString);") and upper_bound 
> will use the other ("static bool operator <(ConstString, const T&);"). No 
> need to specify a Compare() object. So the code would be:
> ```
> T Find(ConstString unique_cstr, T fail_value) const {
>   const_iterator end = m_map.end();
>   const_iterator pos = std::lower_bound(m_map.begin(), end, unique_cstr);
>   if (pos != end && pos->cstring == unique_cstr)
> return pos->value;
>   return fail_value;
> }
> ```
The only benefit of using llvm lower/upper bound is that operates on ranges -- 
you don't need to pass the begin/end iterator pairs, you can just pass the 
container. Under the hood, it just delegates to the `std` version.

The issue of using the compare object is completely orthogonal to that. I could 
define appropriate `operator<`s on the Entry class directly, and thereby avoid 
the need for comparison objects. The reason I did not do that is because the 
Entry class is public (and other parts of the code use and manipulate it), and 
having a (ConstString, Entry) operator< on the Entry class would mean exposing 
it to the outside world.. It's not a strong reason, it seemed to me like this 
is the kind of thing that users outside of this class should not be able to do 
implicitly/accidentally, so I tried to hide it a bit.


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

https://reviews.llvm.org/D63268



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

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

2019-06-13 Thread Greg Clayton via Phabricator via lldb-commits

clayborg updated this revision to Diff 204553.
clayborg added a comment.

Let compiler hoist "cu_language == eLanguageTypeObjC || cu_language == 
eLanguageTypeObjC_plus_plus" by inlining it into if statement so it is more 
readable.


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

https://reviews.llvm.org/D63171

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


Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -248,26 +248,30 @@
 case DW_TAG_subprogram:
   if (has_address) {
 if (name) {
-  ObjCLanguage::MethodName objc_method(name, true);
-  if (objc_method.IsValid(true)) {
-ConstString objc_class_name_with_category(
-objc_method.GetClassNameWithCategory());
-ConstString objc_selector_name(objc_method.GetSelector());
-ConstString objc_fullname_no_category_name(
-objc_method.GetFullNameWithoutCategory(true));
-ConstString 
objc_class_name_no_category(objc_method.GetClassName());
-set.function_fullnames.Insert(ConstString(name), ref);
-if (objc_class_name_with_category)
-  set.objc_class_selectors.Insert(objc_class_name_with_category,
+  bool is_objc_method = false;
+  if (cu_language == eLanguageTypeObjC ||
+  cu_language == eLanguageTypeObjC_plus_plus) {
+ObjCLanguage::MethodName objc_method(name, true);
+if (objc_method.IsValid(true)) {
+  is_objc_method = true;
+  ConstString class_name_with_category(
+  objc_method.GetClassNameWithCategory());
+  ConstString objc_selector_name(objc_method.GetSelector());
+  ConstString objc_fullname_no_category_name(
+  objc_method.GetFullNameWithoutCategory(true));
+  ConstString class_name_no_category(objc_method.GetClassName());
+  set.function_fullnames.Insert(ConstString(name), ref);
+  if (class_name_with_category)
+set.objc_class_selectors.Insert(class_name_with_category, ref);
+  if (class_name_no_category &&
+  class_name_no_category != class_name_with_category)
+set.objc_class_selectors.Insert(class_name_no_category, ref);
+  if (objc_selector_name)
+set.function_selectors.Insert(objc_selector_name, ref);
+  if (objc_fullname_no_category_name)
+set.function_fullnames.Insert(objc_fullname_no_category_name,
   ref);
-if (objc_class_name_no_category &&
-objc_class_name_no_category != objc_class_name_with_category)
-  set.objc_class_selectors.Insert(objc_class_name_no_category, 
ref);
-if (objc_selector_name)
-  set.function_selectors.Insert(objc_selector_name, ref);
-if (objc_fullname_no_category_name)
-  set.function_fullnames.Insert(objc_fullname_no_category_name,
-ref);
+}
   }
   // If we have a mangled name, then the DW_AT_name attribute is
   // usually the method name without the class or any parameters
@@ -278,7 +282,7 @@
   else
 set.function_basenames.Insert(ConstString(name), ref);
 
-  if (!is_method && !mangled_cstr && !objc_method.IsValid(true))
+  if (!is_method && !mangled_cstr && !is_objc_method)
 set.function_fullnames.Insert(ConstString(name), ref);
 }
 if (mangled_cstr) {


Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -248,26 +248,30 @@
 case DW_TAG_subprogram:
   if (has_address) {
 if (name) {
-  ObjCLanguage::MethodName objc_method(name, true);
-  if (objc_method.IsValid(true)) {
-ConstString objc_class_name_with_category(
-objc_method.GetClassNameWithCategory());
-ConstString objc_selector_name(objc_method.GetSelector());
-ConstString objc_fullname_no_category_name(
-objc_method.GetFullNameWithoutCategory(true));
-ConstString objc_class_name_no_category(objc_method.GetClassName());
-set.function_fullnames.Insert(ConstString(name), ref);
-if (objc_class_name_with_category)
-  set.objc_class_selectors.Insert(objc_class_name_with_category,
+  bool is_objc_method = false;
+  if (cu_language == eLanguageTypeObjC ||
+  cu_language == eLanguageTypeObjC_plus_

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

2019-06-13 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D63165#1540606 , @Hui wrote:

> > In D63165#1539118 , @amccarth 
> > wrote:
> > 
> >> Sorry for the stupid question, but ...
> >>
> >> What exactly is meant here by "Native"?  How is a NativeProcessWindows 
> >> different from ProcessWindows?
> > 
> > 
> > The Native*** classes are meant to be used from lldb-server. They look 
> > somewhat similar to their non-native counterpart because they still do 
> > debugging, but they're a lot dumber, because they only deal with basic 
> > process control, and none of the fancy symbolic stuff that you'd need debug 
> > info for.
>
> They differ in APIs but most of them have common implementations. The APIs 
> from native process classes are more easy to apply process/thread control.
>  Hope the native and non-native ones can be merged. The similar thing to the 
> RegisterContext and NativeRegisterContext classes.
>
> The other thing is that using "native" classes can avoid linking a lot of 
> unnecessary lldb libs (LLDB plugins or whatever comes with the plugins) to 
> lldb-server.
>  The nativeprocesswindows could just be a pass-through to processwindows 
> plugin, but the usage is a sort of strange since the
>  lldb-server needs to initialize the plugin, create a target, and create a 
> instance just like what lldb does. This means literally
>  there will be two lldb debuggers, one on host and the other one on remote. 
> It is  doable, but not that applicable.

So the idea is ProcessWindows will be deleted once lldb-server supports windows 
debugging. A bit of history here: when we first started we started making 
Process for each platform (mac, linux, windows). Then we thought about 
remote debugging and decided to have everyone just make a lldb-server for their 
platform and even when we are locally debugging, we launch a lldb-server. This 
is how linux, macOS, the BSDs and other targets do it. Why? Because if you do 
it the other way you have more code to support: one native plug-in and one 
remote plug-in. This means the remote debugging usually has more issues since 
it is the less tested debugging scenario. It also means that if you had a 
native process implementation only, like ProcessWindows is currently, you can't 
remotely debug to a windows machine.

So yes there is duplicated code for now, but ProcessWindows.cpp and 
ThreadWindows.cpp should go away in the near future once lldb-server takes over 
so the temporary code duplication is just to keep things working until 
lldb-server takes over.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63165

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

[Lldb-commits] [lldb] r363279 - [CMake] Fix lldb-dotest for single-config generators in standalone builds

2019-06-13 Thread Stefan Granitz via lldb-commits

Author: stefan.graenitz
Date: Thu Jun 13 10:35:43 2019
New Revision: 363279

URL: http://llvm.org/viewvc/llvm-project?rev=363279&view=rev
Log:
[CMake] Fix lldb-dotest for single-config generators in standalone builds

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

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

Modified: lldb/trunk/utils/lldb-dotest/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/utils/lldb-dotest/CMakeLists.txt?rev=363279&r1=363278&r2=363279&view=diff
==
--- lldb/trunk/utils/lldb-dotest/CMakeLists.txt (original)
+++ lldb/trunk/utils/lldb-dotest/CMakeLists.txt Thu Jun 13 10:35:43 2019
@@ -7,7 +7,11 @@ get_property(LLDB_DOTEST_ARGS GLOBAL PRO
 
 # Generate lldb-dotest Python driver script for each build mode.
 if(LLDB_BUILT_STANDALONE)
-  foreach(config_type ${CMAKE_CONFIGURATION_TYPES})
+  set(config_types ".")
+  if(CMAKE_CONFIGURATION_TYPES)
+set(config_types ${CMAKE_CONFIGURATION_TYPES})
+  endif()
+  foreach(config_type ${config_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}")


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

[Lldb-commits] [lldb] r363280 - [CMake] Fix generated Xcode-project ignoring output directory setting for LLDB.framework

2019-06-13 Thread Stefan Granitz via lldb-commits

Author: stefan.graenitz
Date: Thu Jun 13 10:35:50 2019
New Revision: 363280

URL: http://llvm.org/viewvc/llvm-project?rev=363280&view=rev
Log:
[CMake] Fix generated Xcode-project ignoring output directory setting for 
LLDB.framework

Other generators honor the `LIBRARY_OUTPUT_DIRECTORY` target property, but 
apparently Xcode doesn't. So we call `set_output_directory()` as 
`llvm_add_library()` would do and this works.
Note that `LIBRARY_OUTPUT_DIRECTORY` is still necessary, because it's used to 
store and read the target's absolute build directory (while 
`LLDB_FRAMEWORK_BUILD_DIR` is relative!).

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

Modified: lldb/trunk/cmake/modules/LLDBFramework.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBFramework.cmake?rev=363280&r1=363279&r2=363280&view=diff
==
--- lldb/trunk/cmake/modules/LLDBFramework.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBFramework.cmake Thu Jun 13 10:35:50 2019
@@ -26,6 +26,12 @@ set_target_properties(liblldb PROPERTIES
   MACOSX_FRAMEWORK_INFO_PLIST ${LLDB_SOURCE_DIR}/resources/LLDB-Info.plist.in
 )
 
+# Defined in AddLLVM.cmake; handles edge cases for multi-config generators
+set_output_directory(liblldb
+  BINARY_DIR ${framework_target_dir}
+  LIBRARY_DIR ${framework_target_dir}
+)
+
 # Affects the layout of the framework bundle (default is macOS layout).
 if(IOS)
   set_target_properties(liblldb PROPERTIES


___
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-13 Thread António Afonso via Phabricator via lldb-commits

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

I see what you mean, this is a good point. Yes, like you say I'm assuming the 
string I want to read is in a page that I can read using the fast method. And 
in this situation I prefer to minimize the number of calls to ReadMemory than 
the amount of data I'll read (given that I'll be reading from within the same 
page that is already cached, and also assuming the destination is also cached 
(probably true if in the stack)). I also wanted to side step the question of 
what is a good string average size because I don't have good data to back it up.
So yeah, optimizing for the case I know it exists, and we can tweak this better 
once we have other cases to analyse.




Comment at: lldb/unittests/Host/NativeProcessProtocolTest.cpp:128-133
+  EXPECT_THAT_ERROR(
+  Process.ReadCStringFromMemory(0x0, &string[0], sizeof(string), 
bytes_read)
+  .ToError(),
+  llvm::Succeeded());
+  EXPECT_STREQ(string, "hel");
+  EXPECT_EQ(bytes_read, 3UL);

labath wrote:
> aadsm wrote:
> > labath wrote:
> > > I'm wondering how will the caller know that the read has been truncated. 
> > > Given that ReadMemory already returns an error in case of partial reads, 
> > > maybe we could do the same here ? (return an error, but still set 
> > > bytes_read and the string as they are now). What do you think ?
> > I'm a bit hesitant on this one, because it's different semantics.
> > ReadMemory returns an error when it was not able to read everything that it 
> > was told to.
> > 
> > In this test case we were able to read everything but didn't find a null 
> > terminator. I decided to set a null terminator in this case and say that we 
> > read 1 less byte in order to 1) actually have a C string (I don't want 
> > people to strlen it and fail) and 2) allow people to read in chunks if they 
> > want (however, I don't know how useful this will be if at all) or allow 
> > them to partially read a string or even to realize that the string is 
> > larger than originally expected.
> > 
> > There is a way to know if it didn't read a full string with `strlen(string) 
> > == bytes_read` but that'd be O(n). You can also do `bytes_read == sizeof-1 
> > && string[size-2] != '\0'` but it kind of sucks.
> > I have no good solution here :D unless we want to return an enum with the 3 
> > different options.
> > In this test case we were able to read everything but ...
> Is that really true? We were told to read a c string. We did not find a c 
> string, so instead we chose to mangle the data that we have read into a c 
> string. That seems a bit error-ish.
> 
> I also find it confusing that in this (truncated) case the returned value 
> corresponds with the length of the string, while in the "normal" case in 
> includes the nul terminator. I think it would be more natural to do it the 
> other way around, as that would be similar to what snprintf does.
> 
> How about we avoid this issue entirely, and have the function return 
> `Expected` (the StringRef being backed by the buffer you pass in 
> as the argument) instead? This way you don't have to do any null termination, 
> as the StringRef carries the length implicitly. And one can tell that the 
> read may be incomplete by comparing the  stringref size with the input buffer 
> size.
> Is that really true? We were told to read a c string. We did not find a c 
> string, so instead we chose to mangle the data that we have read into a c 
> string. That seems a bit error-ish.

That is fair.

> How about we avoid this issue entirely, and have the function return 
> Expected (the StringRef being backed by the buffer you pass in as 
> the argument) instead? This way you don't have to do any null termination, as 
> the StringRef carries the length implicitly. And one can tell that the read 
> may be incomplete by comparing the stringref size with the input buffer size.

I think my only concern with this is that people might think the passed in 
buffer would be null terminated after the call (this is true for snprintf). I'm 
not sure how to set the expectation that it wouldn't...



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

2019-06-13 Thread Alex Langford via Phabricator via lldb-commits

xiaobai marked 3 inline comments as done.
xiaobai added inline comments.



Comment at: include/lldb/Target/LanguageRuntime.h:160-162
+  /// Identify whether a name is a runtime value that should not be hidden by
+  /// from the user interface.
+  virtual bool IsWhitelistedRuntimeValue(ConstString name) { return false; }

labath wrote:
> Could we make this a non-public extension point, and make it so that it is 
> automatically invoked from `IsRuntimeSupportValue` ?
> 
> That would simplify the code on the caller side..
See my comment on ValueObject::IsRuntimeSupportValue to see why I think this 
isn't going to be enough.



Comment at: source/Core/ValueObject.cpp:1702-1724
+  return runtime->IsRuntimeSupportValue(*this) &&
+ !runtime->IsWhitelistedRuntimeValue(GetName());
+
+// It's possible we have more than one language involved here. For example,
+// in ObjC `_cmd` is a whitelisted runtime variable name, but
+// GetObjectRuntimeLanguage will say it's a C variable since it's just a
+// cstring.

clayborg wrote:
> Seems like this should still just make one call. The two calls to 
> IsRuntimeSupportValue followed by IsWhitelistedRuntimeValue seems like a 
> bunch of hard to read code. We should make just one call to each runtime and 
> do all the work needed from within there. So I vote to revert this change and 
> do the work in each IsRuntimeSupportValue method. See additional comments in 
> CPPLanguageRuntime::IsRuntimeSupportValue() that was removed. 
I'm not convinced that doing all the work in 
`LanguageRuntime::IsRuntimeSupportValue` is the right thing to do. The 
interface doesn't make a distinction between "Isn't a runtime support value" 
and "Is a whitelisted runtime support value".
There's not a good way to know. The implementation I have here keeps track of 
whether or not any language runtime considers this a runtime support value. If 
a runtime considers it a runtime support value and it's a whitelisted one, you 
immediately return false. If you have just `IsRuntimeSupportValue` and one 
runtime says "this is a runtime support value" but the other says "this isn't a 
runtime support value", how do you know if the second runtime is trying to say 
it's a whitelisted value?

However, getting rid of the initial GetLanguageRuntime+check in favor of just 
having one big loop seems like it'll be better so I'll change that.



Comment at: source/Target/CPPLanguageRuntime.cpp:59
-return false;
-  return !ObjCLanguageRuntime::IsWhitelistedRuntimeValue(name);
 }

clayborg wrote:
> Seems like we should restore this function and just get the ObjC language 
> runtime from the process, and if it exists, call IsWhitelistedRuntimeValue on 
> it?
> 
> ```
> auto objc_runtime = ObjCLanguageRuntime::Get(m_process);
> if (objc_runtime)
>   return objc_runtime->IsWhitelistedRuntimeValue(name);
> return false;
> ```
Indeed, I did not preserve the semantics here because they are technically 
wrong. It means that, regardless of whether or not you have an ObjC runtime 
loaded, you still make sure it's being handled.

Greg's answer is the correct answer while trying to preserve the intention. I 
think that this is unnecessary though, because 
`ValueObject::IsRuntimeSupportValue` after this patch should check every 
available runtime. If you care about ObjC++, the ObjC language runtime should 
handle the ObjC whitelisted runtime values.


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

https://reviews.llvm.org/D63240



___
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-13 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/unittests/Host/NativeProcessProtocolTest.cpp:128-133
+  EXPECT_THAT_ERROR(
+  Process.ReadCStringFromMemory(0x0, &string[0], sizeof(string), 
bytes_read)
+  .ToError(),
+  llvm::Succeeded());
+  EXPECT_STREQ(string, "hel");
+  EXPECT_EQ(bytes_read, 3UL);

aadsm wrote:
> labath wrote:
> > aadsm wrote:
> > > labath wrote:
> > > > I'm wondering how will the caller know that the read has been 
> > > > truncated. Given that ReadMemory already returns an error in case of 
> > > > partial reads, maybe we could do the same here ? (return an error, but 
> > > > still set bytes_read and the string as they are now). What do you think 
> > > > ?
> > > I'm a bit hesitant on this one, because it's different semantics.
> > > ReadMemory returns an error when it was not able to read everything that 
> > > it was told to.
> > > 
> > > In this test case we were able to read everything but didn't find a null 
> > > terminator. I decided to set a null terminator in this case and say that 
> > > we read 1 less byte in order to 1) actually have a C string (I don't want 
> > > people to strlen it and fail) and 2) allow people to read in chunks if 
> > > they want (however, I don't know how useful this will be if at all) or 
> > > allow them to partially read a string or even to realize that the string 
> > > is larger than originally expected.
> > > 
> > > There is a way to know if it didn't read a full string with 
> > > `strlen(string) == bytes_read` but that'd be O(n). You can also do 
> > > `bytes_read == sizeof-1 && string[size-2] != '\0'` but it kind of sucks.
> > > I have no good solution here :D unless we want to return an enum with the 
> > > 3 different options.
> > > In this test case we were able to read everything but ...
> > Is that really true? We were told to read a c string. We did not find a c 
> > string, so instead we chose to mangle the data that we have read into a c 
> > string. That seems a bit error-ish.
> > 
> > I also find it confusing that in this (truncated) case the returned value 
> > corresponds with the length of the string, while in the "normal" case in 
> > includes the nul terminator. I think it would be more natural to do it the 
> > other way around, as that would be similar to what snprintf does.
> > 
> > How about we avoid this issue entirely, and have the function return 
> > `Expected` (the StringRef being backed by the buffer you pass in 
> > as the argument) instead? This way you don't have to do any null 
> > termination, as the StringRef carries the length implicitly. And one can 
> > tell that the read may be incomplete by comparing the  stringref size with 
> > the input buffer size.
> > Is that really true? We were told to read a c string. We did not find a c 
> > string, so instead we chose to mangle the data that we have read into a c 
> > string. That seems a bit error-ish.
> 
> That is fair.
> 
> > How about we avoid this issue entirely, and have the function return 
> > Expected (the StringRef being backed by the buffer you pass in 
> > as the argument) instead? This way you don't have to do any null 
> > termination, as the StringRef carries the length implicitly. And one can 
> > tell that the read may be incomplete by comparing the stringref size with 
> > the input buffer size.
> 
> I think my only concern with this is that people might think the passed in 
> buffer would be null terminated after the call (this is true for snprintf). 
> I'm not sure how to set the expectation that it wouldn't...
> 
That's true, but on the other hand, snprintf does not use StringRefs, only c 
strings. If the users only work with the returned StringRef (like most of our 
code should), then they shouldn't care about whether it's nul terminated or not.

I'd also be fine with nul-terminating the buffer, but still returning a 
StringRef to the part which does not include the nul terminator. That would 
make this function similar to how llvm::Twine::toNullTerminatedStringRef works 
(though the fact that they explicitly put "NullTerminated" in the name shows 
that one normally does not expect StringRefs to be null terminated).

This way, we'd lose the ability to tell whether the read was truncated or not, 
but that's probably not that important. We'd still have the ability to work 
with the string we've read in an llvm-like manner and avoid any additional 
length computations..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [PATCH] D63241: Fixed typos in Log.h

2019-06-13 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Looks like the fallout of reflowing the comments. LGTM, Thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63241



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

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

2019-06-13 Thread Adrian Prantl via Phabricator via lldb-commits

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

Thanks!


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

https://reviews.llvm.org/D63171



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

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

2019-06-13 Thread Alex Langford via Phabricator via lldb-commits

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

Simplify ValueObject::IsRuntimeSupportValue


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

https://reviews.llvm.org/D63240

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  include/lldb/Target/LanguageRuntime.h
  include/lldb/Target/ObjCLanguageRuntime.h
  source/Core/ValueObject.cpp
  source/Target/CPPLanguageRuntime.cpp
  source/Target/ObjCLanguageRuntime.cpp

Index: source/Target/ObjCLanguageRuntime.cpp
===
--- source/Target/ObjCLanguageRuntime.cpp
+++ source/Target/ObjCLanguageRuntime.cpp
@@ -46,19 +46,6 @@
   return name == g_self || name == g_cmd;
 }
 
-bool ObjCLanguageRuntime::IsRuntimeSupportValue(ValueObject &valobj) {
-  // All runtime support values have to be marked as artificial by the
-  // compiler. But not all artificial variables should be hidden from
-  // the user.
-  if (!valobj.GetVariable())
-return false;
-  if (!valobj.GetVariable()->IsArtificial())
-return false;
-
-  // Whitelist "self" and "_cmd".
-  return !IsWhitelistedRuntimeValue(valobj.GetName());
-}
-
 bool ObjCLanguageRuntime::AddClass(ObjCISA isa,
const ClassDescriptorSP &descriptor_sp,
const char *class_name) {
Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -43,20 +43,8 @@
 CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 : LanguageRuntime(process) {}
 
-bool CPPLanguageRuntime::IsRuntimeSupportValue(ValueObject &valobj) {
-  // All runtime support values have to be marked as artificial by the
-  // compiler. But not all artificial variables should be hidden from
-  // the user.
-  if (!valobj.GetVariable())
-return false;
-  if (!valobj.GetVariable()->IsArtificial())
-return false;
-
-  // Whitelist "this" and since there is no ObjC++ runtime, any ObjC names.
-  ConstString name = valobj.GetName();
-  if (name == g_this)
-return false;
-  return !ObjCLanguageRuntime::IsWhitelistedRuntimeValue(name);
+bool CPPLanguageRuntime::IsWhitelistedRuntimeValue(ConstString name) {
+  return name == g_this;
 }
 
 bool CPPLanguageRuntime::GetObjectDescription(Stream &str,
Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -1696,14 +1696,26 @@
 bool ValueObject::IsRuntimeSupportValue() {
   Process *process(GetProcessSP().get());
   if (process) {
-LanguageRuntime *runtime =
-process->GetLanguageRuntime(GetObjectRuntimeLanguage());
-if (!runtime)
-  runtime = ObjCLanguageRuntime::Get(*process);
-if (runtime)
-  return runtime->IsRuntimeSupportValue(*this);
-// If there is no language runtime, trust the compiler to mark all
-// runtime support variables as artificial.
+// It's possible we have more than one language involved here. For example,
+// in ObjC `_cmd` is a whitelisted runtime variable name, but
+// GetObjectRuntimeLanguage will say it's a C variable since it's just a
+// cstring.
+bool marked_as_runtime_support_val = false;
+for (auto *runtime : process->GetLanguageRuntimes()) {
+  bool is_runtime_support_val = runtime->IsRuntimeSupportValue(*this);
+  if (is_runtime_support_val && runtime->IsWhitelistedRuntimeValue(GetName()))
+return false;
+
+  marked_as_runtime_support_val |= is_runtime_support_val;
+}
+
+// If any language runtimes marked it as a runtime support value, we say so.
+if (marked_as_runtime_support_val)
+  return true;
+
+// If no language runtime claims that this value is a runtime support value,
+// then we trust that the compiler marked runtime support variables as
+// artificial.
 return GetVariable() && GetVariable()->IsArtificial();
   }
   return false;
Index: include/lldb/Target/ObjCLanguageRuntime.h
===
--- include/lldb/Target/ObjCLanguageRuntime.h
+++ include/lldb/Target/ObjCLanguageRuntime.h
@@ -299,8 +299,7 @@
 
   /// Check whether the name is "self" or "_cmd" and should show up in
   /// "frame variable".
-  static bool IsWhitelistedRuntimeValue(ConstString name);
-  bool IsRuntimeSupportValue(ValueObject &valobj) override;
+  bool IsWhitelistedRuntimeValue(ConstString name) override;
 
 protected:
   // Classes that inherit from ObjCLanguageRuntime can see and modify these
Index: include/lldb/Target/LanguageRuntime.h
===
--- include/lldb/Target/LanguageRuntime.h
+++ include/lldb/Target/LanguageRuntime.h
@@ -16,6 +16,7 @@
 #include "lldb/Core/Value.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Expression/LLVMUserExpression.h

[Lldb-commits] [PATCH] D63241: Fixed typos in Log.h

2019-06-13 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Let me know if you want me to commit this for you.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63241



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

[Lldb-commits] [PATCH] D63181: [Target] Decouple ObjCLanguageRuntime from LanguageRuntime

2019-06-13 Thread Alex Langford via Phabricator via lldb-commits

xiaobai updated this revision to Diff 204648.
xiaobai added a comment.
Herald added a subscriber: mgorny.

Implement suggestion
Address feedback


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

https://reviews.llvm.org/D63181

Files:
  include/lldb/Breakpoint/Breakpoint.h
  include/lldb/Breakpoint/BreakpointPrecondition.h
  include/lldb/Core/PluginManager.h
  include/lldb/Target/LanguageRuntime.h
  include/lldb/Target/ObjCLanguageRuntime.h
  include/lldb/lldb-forward.h
  include/lldb/lldb-private-interfaces.h
  source/Breakpoint/Breakpoint.cpp
  source/Breakpoint/BreakpointPrecondition.cpp
  source/Breakpoint/CMakeLists.txt
  source/Core/PluginManager.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  source/Target/LanguageRuntime.cpp
  source/Target/ObjCLanguageRuntime.cpp
  source/Target/Target.cpp

Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -11,6 +11,7 @@
 #include "Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h"
 #include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
 #include "lldb/Breakpoint/BreakpointIDList.h"
+#include "lldb/Breakpoint/BreakpointPrecondition.h"
 #include "lldb/Breakpoint/BreakpointResolver.h"
 #include "lldb/Breakpoint/BreakpointResolverAddress.h"
 #include "lldb/Breakpoint/BreakpointResolverFileLine.h"
@@ -573,8 +574,7 @@
   BreakpointSP exc_bkpt_sp = LanguageRuntime::CreateExceptionBreakpoint(
   *this, language, catch_bp, throw_bp, internal);
   if (exc_bkpt_sp && additional_args) {
-Breakpoint::BreakpointPreconditionSP precondition_sp =
-exc_bkpt_sp->GetPrecondition();
+BreakpointPreconditionSP precondition_sp = exc_bkpt_sp->GetPrecondition();
 if (precondition_sp && additional_args) {
   if (error)
 *error = precondition_sp->ConfigurePrecondition(*additional_args);
Index: source/Target/ObjCLanguageRuntime.cpp
===
--- source/Target/ObjCLanguageRuntime.cpp
+++ source/Target/ObjCLanguageRuntime.cpp
@@ -375,6 +375,18 @@
   return found;
 }
 
+lldb::BreakpointPreconditionSP
+ObjCLanguageRuntime::GetBreakpointExceptionPrecondition(LanguageType language,
+bool throw_bp) {
+  if (language != eLanguageTypeObjC)
+return lldb::BreakpointPreconditionSP();
+  if (!throw_bp)
+return lldb::BreakpointPreconditionSP();
+  BreakpointPreconditionSP precondition_sp(
+  new ObjCLanguageRuntime::ObjCExceptionPrecondition());
+  return precondition_sp;
+}
+
 // Exception breakpoint Precondition class for ObjC:
 void ObjCLanguageRuntime::ObjCExceptionPrecondition::AddClassName(
 const char *class_name) {
Index: source/Target/LanguageRuntime.cpp
===
--- source/Target/LanguageRuntime.cpp
+++ source/Target/LanguageRuntime.cpp
@@ -11,7 +11,6 @@
 #include "lldb/Core/SearchFilter.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Target/Language.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Target.h"
 
 using namespace lldb;
@@ -224,19 +223,24 @@
 
 LanguageRuntime::~LanguageRuntime() = default;
 
-Breakpoint::BreakpointPreconditionSP
-LanguageRuntime::CreateExceptionPrecondition(lldb::LanguageType language,
- bool catch_bp, bool throw_bp) {
-  switch (language) {
-  case eLanguageTypeObjC:
-if (throw_bp)
-  return Breakpoint::BreakpointPreconditionSP(
-  new ObjCLanguageRuntime::ObjCExceptionPrecondition());
-break;
-  default:
-break;
+BreakpointPreconditionSP
+LanguageRuntime::GetExceptionPrecondition(LanguageType language,
+  bool throw_bp) {
+  LanguageRuntimeCreateInstance create_callback;
+  for (uint32_t idx = 0;
+   (create_callback =
+PluginManager::GetLanguageRuntimeCreateCallbackAtIndex(idx)) !=
+   nullptr;
+   idx++) {
+if (auto precondition_callback =
+PluginManager::GetLanguageRuntimeGetExceptionPreconditionAtIndex(
+idx)) {
+  if (BreakpointPreconditionSP precond =
+  precondition_callback(language, throw_bp))
+return precond;
+}
   }
-  return Breakpoint::BreakpointPreconditionSP();
+  return BreakpointPreconditionSP();
 }
 
 BreakpointSP LanguageRuntime::CreateExceptionBreakpoint(
@@ -252,10 +256,8 @@
   target.CreateBreakpoint(filter_sp, resolver_sp, is_internal, hardware,
   resolve_indirect_functions));
   if (exc_breakpt_sp) {
-Breakpoint::BreakpointPreconditionSP precondition_sp =
-CreateExceptionPrecondition(language, catch_bp, throw_bp);
-if (precondition_sp)
-  exc_breakpt_sp->SetPrecondition(precond

[Lldb-commits] [PATCH] D63166: Move common functionality from processwindows into processdebugger

2019-06-13 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth added a comment.

It's been a while for me, so I'm not super-familiar with the code being 
changed, but I'm okay with factoring out common code.  I agree with labath's 
open points and will try to look at it in more detail tomorrow.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63166



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

[Lldb-commits] [lldb] r363338 - [NFC] Replace a plugin header with a non-plugin header

2019-06-13 Thread Alex Langford via lldb-commits

Author: xiaobai
Date: Thu Jun 13 16:40:34 2019
New Revision: 363338

URL: http://llvm.org/viewvc/llvm-project?rev=363338&view=rev
Log:
[NFC] Replace a plugin header with a non-plugin header

Modified:
lldb/trunk/source/API/SBFrame.cpp

Modified: lldb/trunk/source/API/SBFrame.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBFrame.cpp?rev=363338&r1=363337&r2=363338&view=diff
==
--- lldb/trunk/source/API/SBFrame.cpp (original)
+++ lldb/trunk/source/API/SBFrame.cpp Thu Jun 13 16:40:34 2019
@@ -14,13 +14,13 @@
 
 #include "lldb/lldb-types.h"
 
-#include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
 #include "SBReproducerPrivate.h"
 #include "Utils.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/ValueObjectRegister.h"
 #include "lldb/Core/ValueObjectVariable.h"
+#include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Symbol/Block.h"


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

[Lldb-commits] [PATCH] D63241: Fixed typos in Log.h

2019-06-13 Thread Yuya Fujita via Phabricator via lldb-commits

ziita added a comment.

I have no access to commit.  Could you please commit this patch?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63241



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

[Lldb-commits] [PATCH] D63310: Make crashlog.py less noisy

2019-06-13 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl created this revision.
aprantl added reviewers: davide, jasonmolenda.
Herald added a project: LLDB.

For end-users there is no point in printing dSYM load errors for system 
frameworks, since they will all fail and there's nothing they can do about it. 
This patch hides them by default and shows them when --verbose is present.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63310

Files:
  lldb/examples/python/crashlog.py

Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -246,7 +246,8 @@
 identifier,
 version,
 uuid,
-path):
+path,
+verbose):
 symbolication.Image.__init__(self, path, uuid)
 self.add_section(
 symbolication.Section(
@@ -255,6 +256,17 @@
 "__TEXT"))
 self.identifier = identifier
 self.version = version
+self.verbose = verbose
+
+def show_symbol_progress(self):
+"""
+Hide progress output and errors from system frameworks as they are plentiful.
+"""
+if self.verbose:
+return True
+return not (self.path.startswith("/System/Library/") or
+self.path.startswith("/usr/lib/"))
+
 
 def find_matching_slice(self):
 dwarfdump_cmd_output = subprocess.check_output(
@@ -271,8 +283,9 @@
 return True
 if not self.resolved_path:
 self.unavailable = True
-print(("error\nerror: unable to locate '%s' with UUID %s"
-  % (self.path, self.get_normalized_uuid_string(
+if self.show_symbol_progress():
+print(("error\nerror: unable to locate '%s' with UUID %s"
+   % (self.path, self.get_normalized_uuid_string(
 return False
 
 def locate_module_and_debug_symbols(self):
@@ -282,7 +295,8 @@
 # Mark this as resolved so we don't keep trying
 self.resolved = True
 uuid_str = self.get_normalized_uuid_string()
-print('Getting symbols for %s %s...' % (uuid_str, self.path), end=' ')
+if self.show_symbol_progress():
+print('Getting symbols for %s %s...' % (uuid_str, self.path), end=' ')
 if os.path.exists(self.dsymForUUIDBinary):
 dsym_for_uuid_command = '%s %s' % (
 self.dsymForUUIDBinary, uuid_str)
@@ -332,7 +346,7 @@
 self.unavailable = True
 return False
 
-def __init__(self, path):
+def __init__(self, path, verbose):
 """CrashLog constructor that take a path to a darwin crash log file"""
 symbolication.Symbolicator.__init__(self)
 self.path = os.path.expanduser(path)
@@ -345,6 +359,7 @@
 self.version = -1
 self.error = None
 self.target = None
+self.verbose = verbose
 # With possible initial component of ~ or ~user replaced by that user's
 # home directory.
 try:
@@ -491,7 +506,8 @@
  img_name.strip(),
  img_version.strip()
  if img_version else "",
- uuid.UUID(img_uuid), img_path)
+ uuid.UUID(img_uuid), img_path,
+ self.verbose)
 self.images.append(image)
 else:
 print("error: image regex failed for: %s" % line)
@@ -557,7 +573,9 @@
 if self.target:
 return self.target  # success
 print('crashlog.create_target()...4')
-print('error: unable to locate any executables from the crash log')
+print('error: Unable to locate any executables from the crash log.')
+print('   Try loading the executable into lldb before running crashlog')
+print('   and/or make sure the .dSYM bundles can be found by Spotlight.')
 return self.target
 
 def get_target(self):
@@ -683,7 +701,7 @@
 crash_logs = list()
 for crash_log_file in crash_log_files:
 # print 'crash_log_file = "%s"' % crash_log_file
-crash_log = CrashLog(crash_log_file)
+crash_log = CrashLog(crash_log_file, options.verbose)
 if crash_log.error:
 print(crash_log.error)
 continue
@@ -1022,7 +1040,7 @@
 interactive_crashlogs(options, args)
 else:
 for crash_log_file in args:
-crash_log = CrashLog(crash_log_file)
+cr

[Lldb-commits] [PATCH] D63310: Make crashlog.py less noisy

2019-06-13 Thread Jason Molenda via Phabricator via lldb-commits

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

Nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63310



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

[Lldb-commits] [PATCH] D63311: Python 3: decode string as utf-8 to avoid type mismatch.

2019-06-13 Thread Adrian Prantl via Phabricator via lldb-commits

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

Side question: do we need to do this on every subprocess.check_output() in this 
file?

rdar://problem/51464644


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63311

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -326,7 +326,7 @@
 try:
 dsym = subprocess.check_output(
 ["/usr/bin/mdfind",
- "com_apple_xcode_dsym_uuids == %s"%uuid_str])[:-1]
+ "com_apple_xcode_dsym_uuids == 
%s"%uuid_str]).decode("utf-8")[:-1]
 if dsym and os.path.exists(dsym):
 print(('falling back to binary inside "%s"'%dsym))
 self.symfile = dsym


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -326,7 +326,7 @@
 try:
 dsym = subprocess.check_output(
 ["/usr/bin/mdfind",
- "com_apple_xcode_dsym_uuids == %s"%uuid_str])[:-1]
+ "com_apple_xcode_dsym_uuids == %s"%uuid_str]).decode("utf-8")[:-1]
 if dsym and os.path.exists(dsym):
 print(('falling back to binary inside "%s"'%dsym))
 self.symfile = dsym
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63311: Python 3: decode string as utf-8 to avoid type mismatch.

2019-06-13 Thread Davide Italiano via Phabricator via lldb-commits

davide added a comment.

yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63311



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

[Lldb-commits] [PATCH] D62213: [ABI] Implement Windows ABI for x86_64

2019-06-13 Thread Wanyi Ye via Phabricator via lldb-commits

kusmour updated this revision to Diff 204695.
kusmour added a comment.

Added support for XMM registers.
Now the step out on a function returns floating point number should have the 
right value.
To play with it, define a function

  float getFloat(float value) {
  return value;
  }

set a break point and step out 
LLDB will print out the right value if debugging on Windows-x86_64


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62213

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ABI/CMakeLists.txt
  lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp
  lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h
  lldb/source/Plugins/ABI/Windows-x86_64/CMakeLists.txt
  lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h
  lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp

Index: lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
+++ lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
@@ -24,6 +24,12 @@
 
 #define DEFINE_GPR(reg, alt) #reg, alt, 8, 0, eEncodingUint, eFormatHexUppercase
 #define DEFINE_GPR_BIN(reg, alt) #reg, alt, 8, 0, eEncodingUint, eFormatBinary
+#define DEFINE_FPU_XMM(reg)\
+  #reg, NULL, 16,   \
+  0, eEncodingUint, eFormatVectorOfUInt64, \
+  {dwarf_##reg##_x86_64, dwarf_##reg##_x86_64, LLDB_INVALID_REGNUM,\
+   LLDB_INVALID_REGNUM, lldb_##reg##_x86_64},\
+  nullptr, nullptr, nullptr, 0
 
 namespace {
 
@@ -51,7 +57,24 @@
   eRegisterIndexR14,
   eRegisterIndexR15,
   eRegisterIndexRip,
-  eRegisterIndexRflags
+  eRegisterIndexRflags,
+
+  eRegisterIndexXmm0,
+  eRegisterIndexXmm1,
+  eRegisterIndexXmm2,
+  eRegisterIndexXmm3,
+  eRegisterIndexXmm4,
+  eRegisterIndexXmm5,
+  eRegisterIndexXmm6,
+  eRegisterIndexXmm7,
+  eRegisterIndexXmm8,
+  eRegisterIndexXmm9,
+  eRegisterIndexXmm10,
+  eRegisterIndexXmm11,
+  eRegisterIndexXmm12,
+  eRegisterIndexXmm13,
+  eRegisterIndexXmm14,
+  eRegisterIndexXmm15
 };
 
 // Array of all register information supported by Windows x86
@@ -133,14 +156,14 @@
  nullptr,
  0},
 {DEFINE_GPR(r10, nullptr),
- {dwarf_r10_x86_64, dwarf_r10_x86_64, LLDB_REGNUM_GENERIC_ARG5,
+ {dwarf_r10_x86_64, dwarf_r10_x86_64, LLDB_INVALID_REGNUM,
   LLDB_INVALID_REGNUM, lldb_r10_x86_64},
  nullptr,
  nullptr,
  nullptr,
  0},
 {DEFINE_GPR(r11, nullptr),
- {dwarf_r11_x86_64, dwarf_r11_x86_64, LLDB_REGNUM_GENERIC_ARG6,
+ {dwarf_r11_x86_64, dwarf_r11_x86_64, LLDB_INVALID_REGNUM,
   LLDB_INVALID_REGNUM, lldb_r11_x86_64},
  nullptr,
  nullptr,
@@ -188,6 +211,22 @@
  nullptr,
  nullptr,
  0},
+{DEFINE_FPU_XMM(xmm0)},
+{DEFINE_FPU_XMM(xmm1)},
+{DEFINE_FPU_XMM(xmm2)},
+{DEFINE_FPU_XMM(xmm3)},
+{DEFINE_FPU_XMM(xmm4)},
+{DEFINE_FPU_XMM(xmm5)},
+{DEFINE_FPU_XMM(xmm6)},
+{DEFINE_FPU_XMM(xmm7)},
+{DEFINE_FPU_XMM(xmm8)},
+{DEFINE_FPU_XMM(xmm9)},
+{DEFINE_FPU_XMM(xmm10)},
+{DEFINE_FPU_XMM(xmm11)},
+{DEFINE_FPU_XMM(xmm12)},
+{DEFINE_FPU_XMM(xmm13)},
+{DEFINE_FPU_XMM(xmm14)},
+{DEFINE_FPU_XMM(xmm15)}
 };
 
 static size_t k_num_register_infos = llvm::array_lengthof(g_register_infos);
@@ -202,10 +241,20 @@
 eRegisterIndexR12, eRegisterIndexR13, eRegisterIndexR14,
 eRegisterIndexR15, eRegisterIndexRip, eRegisterIndexRflags};
 
+uint32_t g_fpu_reg_indices[] = {
+eRegisterIndexXmm0,  eRegisterIndexXmm1,  eRegisterIndexXmm2,
+eRegisterIndexXmm3,  eRegisterIndexXmm4,  eRegisterIndexXmm5,
+eRegisterIndexXmm6,  eRegisterIndexXmm7,  eRegisterIndexXmm8,
+eRegisterIndexXmm9,  eRegisterIndexXmm10, eRegisterIndexXmm11,
+eRegisterIndexXmm12, eRegisterIndexXmm13, eRegisterIndexXmm14,
+eRegisterIndexXmm15
+};
+
 RegisterSet g_register_sets[] = {
 {"General Purpose Registers", "gpr",
  llvm::array_lengthof(g_gpr_reg_indices), g_gpr_reg_indices},
-};
+{"Floating Point Registers", "fpu",
+ llvm::array_lengthof(g_fpu_reg_indices), g_fpu_reg_indices}};
 }
 
 // Constructors and Destructors
@@ -242,7 +291,9 @@
   if (reg_info == nullptr)
 return false;
 
-  switch (reg_info->kinds[eRegisterKindLLDB]) {
+  const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
+
+  switch (reg) {
   case lldb_rax_x86_64:
 reg_value.SetUInt64(m_context.Rax);
 break;
@@ -297,6 +348,70 @@
   case lldb_rflags_x86_64:
 reg_value.SetUInt64(m_context.EFlags);
 break;
+  case lldb_xmm0_x86_64:
+reg_value.S

[Lldb-commits] [lldb] r363356 - Fixed typos in Log.h

2019-06-13 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Thu Jun 13 23:06:47 2019
New Revision: 363356

URL: http://llvm.org/viewvc/llvm-project?rev=363356&view=rev
Log:
Fixed typos in Log.h

Patch by: Yuya Fujita

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

Modified:
lldb/trunk/include/lldb/Utility/Log.h

Modified: lldb/trunk/include/lldb/Utility/Log.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Log.h?rev=363356&r1=363355&r2=363356&view=diff
==
--- lldb/trunk/include/lldb/Utility/Log.h (original)
+++ lldb/trunk/include/lldb/Utility/Log.h Thu Jun 13 23:06:47 2019
@@ -69,7 +69,7 @@ public:
 : log_ptr(nullptr), categories(categories),
   default_flags(default_flags) {}
 
-// This function is safe to call at any time If the channel is disabled
+// This function is safe to call at any time. If the channel is disabled
 // after (or concurrently with) this function returning a non-null Log
 // pointer, it is still safe to attempt to write to the Log object -- the
 // output will be discarded.
@@ -80,7 +80,7 @@ public:
   return nullptr;
 }
 
-// This function is safe to call at any time If the channel is disabled
+// This function is safe to call at any time. If the channel is disabled
 // after (or concurrently with) this function returning a non-null Log
 // pointer, it is still safe to attempt to write to the Log object -- the
 // output will be discarded.


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

[Lldb-commits] [PATCH] D63241: Fixed typos in Log.h

2019-06-13 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363356: Fixed typos in Log.h (authored by JDevlieghere, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63241?vs=204404&id=204706#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63241

Files:
  lldb/trunk/include/lldb/Utility/Log.h


Index: lldb/trunk/include/lldb/Utility/Log.h
===
--- lldb/trunk/include/lldb/Utility/Log.h
+++ lldb/trunk/include/lldb/Utility/Log.h
@@ -69,7 +69,7 @@
 : log_ptr(nullptr), categories(categories),
   default_flags(default_flags) {}
 
-// This function is safe to call at any time If the channel is disabled
+// This function is safe to call at any time. If the channel is disabled
 // after (or concurrently with) this function returning a non-null Log
 // pointer, it is still safe to attempt to write to the Log object -- the
 // output will be discarded.
@@ -80,7 +80,7 @@
   return nullptr;
 }
 
-// This function is safe to call at any time If the channel is disabled
+// This function is safe to call at any time. If the channel is disabled
 // after (or concurrently with) this function returning a non-null Log
 // pointer, it is still safe to attempt to write to the Log object -- the
 // output will be discarded.


Index: lldb/trunk/include/lldb/Utility/Log.h
===
--- lldb/trunk/include/lldb/Utility/Log.h
+++ lldb/trunk/include/lldb/Utility/Log.h
@@ -69,7 +69,7 @@
 : log_ptr(nullptr), categories(categories),
   default_flags(default_flags) {}
 
-// This function is safe to call at any time If the channel is disabled
+// This function is safe to call at any time. If the channel is disabled
 // after (or concurrently with) this function returning a non-null Log
 // pointer, it is still safe to attempt to write to the Log object -- the
 // output will be discarded.
@@ -80,7 +80,7 @@
   return nullptr;
 }
 
-// This function is safe to call at any time If the channel is disabled
+// This function is safe to call at any time. If the channel is disabled
 // after (or concurrently with) this function returning a non-null Log
 // pointer, it is still safe to attempt to write to the Log object -- the
 // output will be discarded.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63311: Python 3: decode string as utf-8 to avoid type mismatch.

2019-06-13 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Yep, we should decode all the check_output results, as they may contain unicode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63311



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

[Lldb-commits] [lldb] r363357 - Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

2019-06-13 Thread Pavel Labath via lldb-commits

Author: labath
Date: Thu Jun 13 23:33:31 2019
New Revision: 363357

URL: http://llvm.org/viewvc/llvm-project?rev=363357&view=rev
Log:
Make UniqueCStringMap work with non-default-constructible types and other 
improvements/cleanups

Summary:
The motivation for this was me wanting to make the validity of dwarf
DIERefs explicit (via llvm::Optional). This meant that the class
would no longer have a default constructor. As the DIERef was being
stored in a UniqueCStringMap, this meant that this container (like all
standard containers) needed to work with non-default-constructible types
too.

This part is achieved by removing the default constructors for the map
entry types, and providing appropriate comparison overloads so that we
can search for map entries without constructing a dummy entry. While
doing that, I took the opportunity to modernize the code, and add some
tests. Functions that were completely unused are deleted.

This required also some changes in the Symtab code, as it was default
constructing map entries, which was not impossible even though its
value type was default-constructible. Technically, these changes could
be avoided with some SFINAE on the entry type, but I felt that the code
is cleaner this way anyway.

Reviewers: JDevlieghere, sgraenitz

Subscribers: mgorny, aprantl, lldb-commits

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

Added:
lldb/trunk/unittests/Core/UniqueCStringMapTest.cpp
Modified:
lldb/trunk/include/lldb/Core/UniqueCStringMap.h
lldb/trunk/include/lldb/Symbol/Symtab.h
lldb/trunk/source/Symbol/Symtab.cpp
lldb/trunk/unittests/Core/CMakeLists.txt

Modified: lldb/trunk/include/lldb/Core/UniqueCStringMap.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/UniqueCStringMap.h?rev=363357&r1=363356&r2=363357&view=diff
==
--- lldb/trunk/include/lldb/Core/UniqueCStringMap.h (original)
+++ lldb/trunk/include/lldb/Core/UniqueCStringMap.h Thu Jun 13 23:33:31 2019
@@ -26,16 +26,10 @@ namespace lldb_private {
 template  class UniqueCStringMap {
 public:
   struct Entry {
-Entry() {}
-
-Entry(ConstString cstr) : cstring(cstr), value() {}
-
 Entry(ConstString cstr, const T &v) : cstring(cstr), value(v) {}
 
-// This is only for uniqueness, not lexicographical ordering, so we can
-// just compare pointers.
-bool operator<(const Entry &rhs) const {
-  return cstring.GetCString() < rhs.cstring.GetCString();
+friend bool operator<(const Entry &lhs, const Entry &rhs) {
+  return Compare()(lhs, rhs);
 }
 
 ConstString cstring;
@@ -53,17 +47,6 @@ public:
 
   void Clear() { m_map.clear(); }
 
-  // Call this function to always keep the map sorted when putting entries into
-  // the map.
-  void Insert(ConstString unique_cstr, const T &value) {
-typename UniqueCStringMap::Entry e(unique_cstr, value);
-m_map.insert(std::upper_bound(m_map.begin(), m_map.end(), e), e);
-  }
-
-  void Insert(const Entry &e) {
-m_map.insert(std::upper_bound(m_map.begin(), m_map.end(), e), e);
-  }
-
   // Get an entries by index in a variety of forms.
   //
   // The caller is responsible for ensuring that the collection does not change
@@ -101,13 +84,9 @@ public:
   // T values and only if there is a sensible failure value that can
   // be returned and that won't match any existing values.
   T Find(ConstString unique_cstr, T fail_value) const {
-Entry search_entry(unique_cstr);
-const_iterator end = m_map.end();
-const_iterator pos = std::lower_bound(m_map.begin(), end, search_entry);
-if (pos != end) {
-  if (pos->cstring == unique_cstr)
-return pos->value;
-}
+auto pos = llvm::lower_bound(m_map, unique_cstr, Compare());
+if (pos != m_map.end() && pos->cstring == unique_cstr)
+  return pos->value;
 return fail_value;
   }
 
@@ -117,10 +96,8 @@ public:
   // The caller is responsible for ensuring that the collection does not change
   // during while using the returned pointer.
   const Entry *FindFirstValueForName(ConstString unique_cstr) const {
-Entry search_entry(unique_cstr);
-const_iterator end = m_map.end();
-const_iterator pos = std::lower_bound(m_map.begin(), end, search_entry);
-if (pos != end && pos->cstring == unique_cstr)
+auto pos = llvm::lower_bound(m_map, unique_cstr, Compare());
+if (pos != m_map.end() && pos->cstring == unique_cstr)
   return &(*pos);
 return nullptr;
   }
@@ -147,15 +124,9 @@ public:
   size_t GetValues(ConstString unique_cstr, std::vector &values) const {
 const size_t start_size = values.size();
 
-Entry search_entry(unique_cstr);
-const_iterator pos, end = m_map.end();
-for (pos = std::lower_bound(m_map.begin(), end, search_entry); pos != end;
- ++pos) {
-  if (pos->cstring == unique_cstr)
-values.push_back(pos->value);
-  else
-break;
-}
+for (const Entry &entry : ll

[Lldb-commits] [PATCH] D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

2019-06-13 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363357: Make UniqueCStringMap work with 
non-default-constructible types and other… (authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63268?vs=204537&id=204707#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63268

Files:
  lldb/trunk/include/lldb/Core/UniqueCStringMap.h
  lldb/trunk/include/lldb/Symbol/Symtab.h
  lldb/trunk/source/Symbol/Symtab.cpp
  lldb/trunk/unittests/Core/CMakeLists.txt
  lldb/trunk/unittests/Core/UniqueCStringMapTest.cpp

Index: lldb/trunk/unittests/Core/UniqueCStringMapTest.cpp
===
--- lldb/trunk/unittests/Core/UniqueCStringMapTest.cpp
+++ lldb/trunk/unittests/Core/UniqueCStringMapTest.cpp
@@ -0,0 +1,53 @@
+//===-- UniqueCStringMapTest.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
+//
+//===--===//
+
+#include "lldb/Core/UniqueCStringMap.h"
+#include "gmock/gmock.h"
+
+using namespace lldb_private;
+
+namespace {
+struct NoDefault {
+  int x;
+
+  NoDefault(int x) : x(x) {}
+  NoDefault() = delete;
+
+  friend bool operator==(NoDefault lhs, NoDefault rhs) {
+return lhs.x == rhs.x;
+  }
+
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+   NoDefault x) {
+return OS << "NoDefault{" << x.x << "}";
+  }
+};
+} // namespace
+
+TEST(UniqueCStringMap, NoDefaultConstructor) {
+  using MapT = UniqueCStringMap;
+  using EntryT = MapT::Entry;
+
+  MapT Map;
+  ConstString Foo("foo"), Bar("bar");
+
+  Map.Append(Foo, NoDefault(42));
+  EXPECT_THAT(Map.Find(Foo, NoDefault(47)), NoDefault(42));
+  EXPECT_THAT(Map.Find(Bar, NoDefault(47)), NoDefault(47));
+  EXPECT_THAT(Map.FindFirstValueForName(Foo),
+  testing::Pointee(testing::Field(&EntryT::value, NoDefault(42;
+  EXPECT_THAT(Map.FindFirstValueForName(Bar), nullptr);
+
+  std::vector Values;
+  EXPECT_THAT(Map.GetValues(Foo, Values), 1);
+  EXPECT_THAT(Values, testing::ElementsAre(NoDefault(42)));
+
+  Values.clear();
+  EXPECT_THAT(Map.GetValues(Bar, Values), 0);
+  EXPECT_THAT(Values, testing::IsEmpty());
+}
Index: lldb/trunk/unittests/Core/CMakeLists.txt
===
--- lldb/trunk/unittests/Core/CMakeLists.txt
+++ lldb/trunk/unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
   MangledTest.cpp
   RichManglingContextTest.cpp
   StreamCallbackTest.cpp
+  UniqueCStringMapTest.cpp
 
   LINK_LIBS
 lldbCore
Index: lldb/trunk/source/Symbol/Symtab.cpp
===
--- lldb/trunk/source/Symbol/Symtab.cpp
+++ lldb/trunk/source/Symbol/Symtab.cpp
@@ -288,10 +288,8 @@
 // Instantiation of the demangler is expensive, so better use a single one
 // for all entries during batch processing.
 RichManglingContext rmc;
-NameToIndexMap::Entry entry;
-
-for (entry.value = 0; entry.value < num_symbols; ++entry.value) {
-  Symbol *symbol = &m_symbols[entry.value];
+for (uint32_t value = 0; value < num_symbols; ++value) {
+  Symbol *symbol = &m_symbols[value];
 
   // Don't let trampolines get into the lookup by name map If we ever need
   // the trampoline symbols to be searchable by name we can remove this and
@@ -303,52 +301,46 @@
   // If the symbol's name string matched a Mangled::ManglingScheme, it is
   // stored in the mangled field.
   Mangled &mangled = symbol->GetMangled();
-  entry.cstring = mangled.GetMangledName();
-  if (entry.cstring) {
-m_name_to_index.Append(entry);
+  if (ConstString name = mangled.GetMangledName()) {
+m_name_to_index.Append(name, value);
 
 if (symbol->ContainsLinkerAnnotations()) {
   // If the symbol has linker annotations, also add the version without
   // the annotations.
-  entry.cstring = ConstString(m_objfile->StripLinkerSymbolAnnotations(
-entry.cstring.GetStringRef()));
-  m_name_to_index.Append(entry);
+  ConstString stripped = ConstString(
+  m_objfile->StripLinkerSymbolAnnotations(name.GetStringRef()));
+  m_name_to_index.Append(stripped, value);
 }
 
 const SymbolType type = symbol->GetType();
 if (type == eSymbolTypeCode || type == eSymbolTypeResolver) {
   if (mangled.DemangleWithRichManglingInfo(rmc, lldb_skip_name))
-RegisterMangledNameEntry(entry, class_contexts, backlog, rmc);
+

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

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

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

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

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

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

[Lldb-commits] [PATCH] D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

[Lldb-commits] [lldb] r363271 - [CMake] Add fallbacks for copying clang-resource-headers to LLDB.framework in standalone builds

[Lldb-commits] [PATCH] D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

[Lldb-commits] [PATCH] D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

[Lldb-commits] [lldb] r363279 - [CMake] Fix lldb-dotest for single-config generators in standalone builds

[Lldb-commits] [lldb] r363280 - [CMake] Fix generated Xcode-project ignoring output directory setting for LLDB.framework

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

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

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

[Lldb-commits] [PATCH] D63241: Fixed typos in Log.h

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

[Lldb-commits] [PATCH] D63241: Fixed typos in Log.h

[Lldb-commits] [PATCH] D63181: [Target] Decouple ObjCLanguageRuntime from LanguageRuntime

[Lldb-commits] [PATCH] D63166: Move common functionality from processwindows into processdebugger

[Lldb-commits] [lldb] r363338 - [NFC] Replace a plugin header with a non-plugin header

[Lldb-commits] [PATCH] D63241: Fixed typos in Log.h

[Lldb-commits] [PATCH] D63310: Make crashlog.py less noisy

[Lldb-commits] [PATCH] D63310: Make crashlog.py less noisy

[Lldb-commits] [PATCH] D63311: Python 3: decode string as utf-8 to avoid type mismatch.

[Lldb-commits] [PATCH] D63311: Python 3: decode string as utf-8 to avoid type mismatch.

[Lldb-commits] [PATCH] D62213: [ABI] Implement Windows ABI for x86_64

[Lldb-commits] [lldb] r363356 - Fixed typos in Log.h

[Lldb-commits] [PATCH] D63241: Fixed typos in Log.h

[Lldb-commits] [PATCH] D63311: Python 3: decode string as utf-8 to avoid type mismatch.

[Lldb-commits] [lldb] r363357 - Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

[Lldb-commits] [PATCH] D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

36 matches

Site Navigation

Mail list logo

Footer information