[Lldb-commits] [lldb] r356262 - Delete type_sp member from TypePair

[Pavel Labath via lldb-commits]

Author: labath
Date: Fri Mar 15 07:02:35 2019
New Revision: 356262

URL: http://llvm.org/viewvc/llvm-project?rev=356262&view=rev
Log:
Delete type_sp member from TypePair

Summary:
As discussed in the review of D59217, this member is unnecessary since
always the first thing we do is convert it to a CompilerType.

This opens up possibilities for further cleanups (e.g. the whole
TypePair class now loses purpose, since we can just pass around
CompilerType everywhere), but I did not want to do that yet, because I
am not sure if this will not introduce breakages in some of the
platforms/configurations that I am not testing on.

Reviewers: clayborg, zturner, jingham

Subscribers: jdoerfert, lldb-commits

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

Modified:
lldb/trunk/include/lldb/DataFormatters/FormatClasses.h
lldb/trunk/include/lldb/Symbol/Type.h

lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
lldb/trunk/source/Symbol/Type.cpp

Modified: lldb/trunk/include/lldb/DataFormatters/FormatClasses.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/DataFormatters/FormatClasses.h?rev=356262&r1=356261&r2=356262&view=diff
==
--- lldb/trunk/include/lldb/DataFormatters/FormatClasses.h (original)
+++ lldb/trunk/include/lldb/DataFormatters/FormatClasses.h Fri Mar 15 07:02:35 
2019
@@ -139,12 +139,6 @@ public:
 return nullptr;
   }
 
-  lldb::TypeSP GetTypeSP() {
-if (m_type.m_type_pair.IsValid())
-  return m_type.m_type_pair.GetTypeSP();
-return lldb::TypeSP();
-  }
-
   CompilerType GetCompilerType() {
 if (m_type.m_type_pair.IsValid())
   return m_type.m_type_pair.GetCompilerType();

Modified: lldb/trunk/include/lldb/Symbol/Type.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/Type.h?rev=356262&r1=356261&r2=356262&view=diff
==
--- lldb/trunk/include/lldb/Symbol/Type.h (original)
+++ lldb/trunk/include/lldb/Symbol/Type.h Fri Mar 15 07:02:35 2019
@@ -239,19 +239,16 @@ protected:
 
 // these classes are used to back the SBType* objects
 
+// TODO: This class is just a wrapper around CompilerType. Delete it.
 class TypePair {
 public:
-  TypePair() : compiler_type(), type_sp() {}
+  TypePair() : compiler_type() {}
 
-  TypePair(CompilerType type) : compiler_type(type), type_sp() {}
+  TypePair(CompilerType type) : compiler_type(type) {}
 
-  TypePair(lldb::TypeSP type) : compiler_type(), type_sp(type) {
-compiler_type = type_sp->GetForwardCompilerType();
-  }
+  TypePair(lldb::TypeSP type) : compiler_type(type->GetForwardCompilerType()) 
{}
 
-  bool IsValid() const {
-return compiler_type.IsValid() || (type_sp.get() != nullptr);
-  }
+  bool IsValid() const { return compiler_type.IsValid(); }
 
   explicit operator bool() const { return IsValid(); }
 
@@ -261,101 +258,58 @@ public:
 
   bool operator!=(const TypePair &rhs) const { return !(*this == rhs); }
 
-  void Clear() {
-compiler_type.Clear();
-type_sp.reset();
-  }
+  void Clear() { compiler_type.Clear(); }
 
   ConstString GetName() const {
-if (type_sp)
-  return type_sp->GetName();
 if (compiler_type)
   return compiler_type.GetTypeName();
 return ConstString();
   }
 
   ConstString GetDisplayTypeName() const {
-if (type_sp)
-  return type_sp->GetForwardCompilerType().GetDisplayTypeName();
 if (compiler_type)
   return compiler_type.GetDisplayTypeName();
 return ConstString();
   }
 
   void SetType(CompilerType type) {
-type_sp.reset();
 compiler_type = type;
   }
 
   void SetType(lldb::TypeSP type) {
-type_sp = type;
-if (type_sp)
-  compiler_type = type_sp->GetForwardCompilerType();
-else
-  compiler_type.Clear();
+compiler_type = type->GetForwardCompilerType();
   }
 
-  lldb::TypeSP GetTypeSP() const { return type_sp; }
-
   CompilerType GetCompilerType() const { return compiler_type; }
 
-  CompilerType GetPointerType() const {
-if (type_sp)
-  return type_sp->GetForwardCompilerType().GetPointerType();
-return compiler_type.GetPointerType();
-  }
+  CompilerType GetPointerType() const { return compiler_type.GetPointerType(); 
}
 
-  CompilerType GetPointeeType() const {
-if (type_sp)
-  return type_sp->GetForwardCompilerType().GetPointeeType();
-return compiler_type.GetPointeeType();
-  }
+  CompilerType GetPointeeType() const { return compiler_type.GetPointeeType(); 
}
 
   CompilerType GetReferenceType() const {
-if (type_sp)
-  return type_sp->GetForwardCompilerType().GetLValueReferenceType();
-else
-  return compiler_type.GetLValueReferenceType();
+return compiler_type.GetLValueReferenceType();
   }
 
   CompilerType GetTypedefedType() const {
-if (type_sp)
-  return type_sp->GetForwardCompilerType().GetTypedefedType();
-else
-

[Lldb-commits] [PATCH] D59297: Delete type_sp member from TypePair

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356262: Delete type_sp member from TypePair (authored by 
labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59297

Files:
  lldb/trunk/include/lldb/DataFormatters/FormatClasses.h
  lldb/trunk/include/lldb/Symbol/Type.h
  
lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  lldb/trunk/source/Symbol/Type.cpp

Index: lldb/trunk/source/Symbol/Type.cpp
===
--- lldb/trunk/source/Symbol/Type.cpp
+++ lldb/trunk/source/Symbol/Type.cpp
@@ -765,10 +765,6 @@
 
 bool TypeAndOrName::HasName() const { return (bool)m_type_name; }
 
-bool TypeAndOrName::HasTypeSP() const {
-  return m_type_pair.GetTypeSP().get() != nullptr;
-}
-
 bool TypeAndOrName::HasCompilerType() const {
   return m_type_pair.GetCompilerType().IsValid();
 }
@@ -832,7 +828,7 @@
 }
 
 void TypeImpl::SetType(const TypePair &pair, const CompilerType &dynamic) {
-  m_module_wp = pair.GetModule();
+  m_module_wp.reset();
   m_static_type = pair;
   m_dynamic_type = dynamic;
 }
Index: lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
===
--- lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
+++ lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
@@ -235,16 +235,15 @@
   if (!class_type_or_name)
 return false;
 
-  TypeSP type_sp = class_type_or_name.GetTypeSP();
+  CompilerType type = class_type_or_name.GetCompilerType();
   // There can only be one type with a given name, so we've just found
   // duplicate definitions, and this one will do as well as any other. We
   // don't consider something to have a dynamic type if it is the same as
   // the static type.  So compare against the value we were handed.
-  if (!type_sp)
+  if (!type)
 return true;
 
-  if (ClangASTContext::AreTypesSame(in_value.GetCompilerType(),
-type_sp->GetForwardCompilerType())) {
+  if (ClangASTContext::AreTypesSame(in_value.GetCompilerType(), type)) {
 // The dynamic type we found was the same type, so we don't have a
 // dynamic type here...
 return false;
Index: lldb/trunk/include/lldb/DataFormatters/FormatClasses.h
===
--- lldb/trunk/include/lldb/DataFormatters/FormatClasses.h
+++ lldb/trunk/include/lldb/DataFormatters/FormatClasses.h
@@ -139,12 +139,6 @@
 return nullptr;
   }
 
-  lldb::TypeSP GetTypeSP() {
-if (m_type.m_type_pair.IsValid())
-  return m_type.m_type_pair.GetTypeSP();
-return lldb::TypeSP();
-  }
-
   CompilerType GetCompilerType() {
 if (m_type.m_type_pair.IsValid())
   return m_type.m_type_pair.GetCompilerType();
Index: lldb/trunk/include/lldb/Symbol/Type.h
===
--- lldb/trunk/include/lldb/Symbol/Type.h
+++ lldb/trunk/include/lldb/Symbol/Type.h
@@ -239,19 +239,16 @@
 
 // these classes are used to back the SBType* objects
 
+// TODO: This class is just a wrapper around CompilerType. Delete it.
 class TypePair {
 public:
-  TypePair() : compiler_type(), type_sp() {}
+  TypePair() : compiler_type() {}
 
-  TypePair(CompilerType type) : compiler_type(type), type_sp() {}
+  TypePair(CompilerType type) : compiler_type(type) {}
 
-  TypePair(lldb::TypeSP type) : compiler_type(), type_sp(type) {
-compiler_type = type_sp->GetForwardCompilerType();
-  }
+  TypePair(lldb::TypeSP type) : compiler_type(type->GetForwardCompilerType()) {}
 
-  bool IsValid() const {
-return compiler_type.IsValid() || (type_sp.get() != nullptr);
-  }
+  bool IsValid() const { return compiler_type.IsValid(); }
 
   explicit operator bool() const { return IsValid(); }
 
@@ -261,101 +258,58 @@
 
   bool operator!=(const TypePair &rhs) const { return !(*this == rhs); }
 
-  void Clear() {
-compiler_type.Clear();
-type_sp.reset();
-  }
+  void Clear() { compiler_type.Clear(); }
 
   ConstString GetName() const {
-if (type_sp)
-  return type_sp->GetName();
 if (compiler_type)
   return compiler_type.GetTypeName();
 return ConstString();
   }
 
   ConstString GetDisplayTypeName() const {
-if (type_sp)
-  return type_sp->GetForwardCompilerType().GetDisplayTypeName();
 if (compiler_type)
   return compiler_type.GetDisplayTypeName();
 return ConstString();
   }
 
   void SetType(CompilerType type) {
-type_sp.reset();
 compiler_type = type;
   }
 
   void SetType(lldb::TypeSP type) {
-type_sp = type;
-if (type_sp)
-  compiler_type = type_sp->GetForwardCompilerType();
-else
-  c

[Lldb-commits] [lldb] r356263 - Fix a typo in FindLibEdit.cmake

[Pavel Labath via lldb-commits]

Author: labath
Date: Fri Mar 15 07:03:52 2019
New Revision: 356263

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

The package name is LibEdit, so we should use that name in the call to
find_package_handle_standard_args. Failing to do so results in the
standard_args (such as the one telling us whether REQUIRED was used in
the find_package invocation) not being handled.

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

Modified: lldb/trunk/cmake/modules/FindLibEdit.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/FindLibEdit.cmake?rev=356263&r1=356262&r2=356263&view=diff
==
--- lldb/trunk/cmake/modules/FindLibEdit.cmake (original)
+++ lldb/trunk/cmake/modules/FindLibEdit.cmake Fri Mar 15 07:03:52 2019
@@ -51,7 +51,7 @@ else()
   endif()
 
   include(FindPackageHandleStandardArgs)
-  find_package_handle_standard_args(libedit
+  find_package_handle_standard_args(LibEdit
 REQUIRED_VARS
   libedit_INCLUDE_DIRS
   libedit_LIBRARIES


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

Re: [Lldb-commits] [lldb] r355103 - [cmake] Move LLDB_DISABLE_LIBEDIT handling code into a central place

[Pavel Labath via lldb-commits]

On 14/03/2019 21:45, Nico Weber wrote:

I tried LLVM_ENABLE_PROJECTS=all today, and it fails with

CMake Error: The following variables are used in this project, but they 
are set to NOTFOUND.
Please set them or make sure they are set and tested correctly in the 
CMake files:

libedit_INCLUDE_DIRS
    used as include directory in directory 
/usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp
    used as include directory in directory 
/usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp
    used as include directory in directory 
/usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp

libedit_LIBRARIES
     linked by target "cmTC_43ea9" in directory 
/usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp


CMake Error at /usr/share/cmake-3.10/Modules/CheckTypeSize.cmake:114 
(try_compile):

   Failed to configure test project build system.
Call Stack (most recent call first):
   /usr/share/cmake-3.10/Modules/CheckTypeSize.cmake:239 
(__check_type_size_impl)
   
/usr/local/google/home/thakis/src/llvm-project/lldb/cmake/modules/LLDBConfig.cmake:106 
(check_type_size)
   /usr/local/google/home/thakis/src/llvm-project/lldb/CMakeLists.txt:15 
(include)




Is that expected?



Whether it is "expected" to fail the build if you don't have libedit is 
a bit of a complicated question and the answer depends on which side of 
the "should dependencies auto-disable themselves when not found" debate. 
However, it's certainly not expected that the build fails in this way, 
as there is a find_package(LibEdit REQUIRED) call two lines above this 
check.


I've done some digging around, and it seems this is due to a bug in our 
our libedit-finding code. r356263 will fix that, so now the build will 
fail slightly earlier and with a slightly better error message.


If we want to change the libedit dependency to auto-disabling, then we 
should start a separate thread about that. I believe that's not 
something that this commit has caused. What it may have done is that it 
changed the failure from a build-time thing (we would fail when trying 
to compile code using libedit) to a configure-time failure. That I would 
consider to be an improvement. :)

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

[Lldb-commits] [PATCH] D59414: Remove the TypePair class

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, jingham, zturner.
Herald added subscribers: jdoerfert, dexonsmith, mehdi_amini.

After D59297 , the TypePair class kind of lost 
its purpose as it was no
longer a "pair". This finished the job started in that patch and deletes
the class altogether. All usages have been updated to use CompilerType
class directly.


https://reviews.llvm.org/D59414

Files:
  include/lldb/DataFormatters/FormatClasses.h
  include/lldb/Symbol/Type.h
  include/lldb/lldb-forward.h
  source/Symbol/Type.cpp

Index: source/Symbol/Type.cpp
===
--- source/Symbol/Type.cpp
+++ source/Symbol/Type.cpp
@@ -690,32 +690,21 @@
   return ModuleSP();
 }
 
-TypeAndOrName::TypeAndOrName() : m_type_pair(), m_type_name() {}
-
-TypeAndOrName::TypeAndOrName(TypeSP &in_type_sp) : m_type_pair(in_type_sp) {
-  if (in_type_sp)
+TypeAndOrName::TypeAndOrName(TypeSP &in_type_sp) {
+  if (in_type_sp) {
+m_compiler_type = in_type_sp->GetForwardCompilerType();
 m_type_name = in_type_sp->GetName();
+  }
 }
 
 TypeAndOrName::TypeAndOrName(const char *in_type_str)
 : m_type_name(in_type_str) {}
 
-TypeAndOrName::TypeAndOrName(const TypeAndOrName &rhs)
-: m_type_pair(rhs.m_type_pair), m_type_name(rhs.m_type_name) {}
-
 TypeAndOrName::TypeAndOrName(ConstString &in_type_const_string)
 : m_type_name(in_type_const_string) {}
 
-TypeAndOrName &TypeAndOrName::operator=(const TypeAndOrName &rhs) {
-  if (this != &rhs) {
-m_type_name = rhs.m_type_name;
-m_type_pair = rhs.m_type_pair;
-  }
-  return *this;
-}
-
 bool TypeAndOrName::operator==(const TypeAndOrName &other) const {
-  if (m_type_pair != other.m_type_pair)
+  if (m_compiler_type != other.m_compiler_type)
 return false;
   if (m_type_name != other.m_type_name)
 return false;
@@ -729,8 +718,8 @@
 ConstString TypeAndOrName::GetName() const {
   if (m_type_name)
 return m_type_name;
-  if (m_type_pair)
-return m_type_pair.GetName();
+  if (m_compiler_type)
+return m_compiler_type.GetTypeName();
   return ConstString("");
 }
 
@@ -743,30 +732,32 @@
 }
 
 void TypeAndOrName::SetTypeSP(lldb::TypeSP type_sp) {
-  m_type_pair.SetType(type_sp);
-  if (m_type_pair)
-m_type_name = m_type_pair.GetName();
+  if (type_sp) {
+m_compiler_type = type_sp->GetForwardCompilerType();
+m_type_name = type_sp->GetName();
+  } else
+Clear();
 }
 
 void TypeAndOrName::SetCompilerType(CompilerType compiler_type) {
-  m_type_pair.SetType(compiler_type);
-  if (m_type_pair)
-m_type_name = m_type_pair.GetName();
+  m_compiler_type = compiler_type;
+  if (m_compiler_type)
+m_type_name = m_compiler_type.GetTypeName();
 }
 
 bool TypeAndOrName::IsEmpty() const {
-  return !((bool)m_type_name || (bool)m_type_pair);
+  return !((bool)m_type_name || (bool)m_compiler_type);
 }
 
 void TypeAndOrName::Clear() {
   m_type_name.Clear();
-  m_type_pair.Clear();
+  m_compiler_type.Clear();
 }
 
 bool TypeAndOrName::HasName() const { return (bool)m_type_name; }
 
 bool TypeAndOrName::HasCompilerType() const {
-  return m_type_pair.GetCompilerType().IsValid();
+  return m_compiler_type.IsValid();
 }
 
 TypeImpl::TypeImpl() : m_module_wp(), m_static_type(), m_dynamic_type() {}
@@ -786,7 +777,7 @@
 }
 
 TypeImpl::TypeImpl(const lldb::TypeSP &type_sp, const CompilerType &dynamic)
-: m_module_wp(), m_static_type(type_sp), m_dynamic_type(dynamic) {
+: m_module_wp(), m_static_type(), m_dynamic_type(dynamic) {
   SetType(type_sp, dynamic);
 }
 
@@ -796,22 +787,19 @@
   SetType(static_type, dynamic_type);
 }
 
-TypeImpl::TypeImpl(const TypePair &pair, const CompilerType &dynamic)
-: m_module_wp(), m_static_type(), m_dynamic_type() {
-  SetType(pair, dynamic);
-}
-
 void TypeImpl::SetType(const lldb::TypeSP &type_sp) {
-  m_static_type.SetType(type_sp);
-  if (type_sp)
+  if (type_sp) {
+m_static_type = type_sp->GetForwardCompilerType();
 m_module_wp = type_sp->GetModule();
-  else
+  } else {
+m_static_type.Clear();
 m_module_wp = lldb::ModuleWP();
+  }
 }
 
 void TypeImpl::SetType(const CompilerType &compiler_type) {
   m_module_wp = lldb::ModuleWP();
-  m_static_type.SetType(compiler_type);
+  m_static_type = compiler_type;
 }
 
 void TypeImpl::SetType(const lldb::TypeSP &type_sp,
@@ -823,13 +811,7 @@
 void TypeImpl::SetType(const CompilerType &compiler_type,
const CompilerType &dynamic) {
   m_module_wp = lldb::ModuleWP();
-  m_static_type.SetType(compiler_type);
-  m_dynamic_type = dynamic;
-}
-
-void TypeImpl::SetType(const TypePair &pair, const CompilerType &dynamic) {
-  m_module_wp.reset();
-  m_static_type = pair;
+  m_static_type = compiler_type;
   m_dynamic_type = dynamic;
 }
 
@@ -900,7 +882,7 @@
   if (CheckModule(module_sp)) {
 if (m_dynamic_type)
   return m_dynamic_type.GetTypeName();
-return m_static_type.GetName();
+return m_static_type.GetTypeName();

[Lldb-commits] [PATCH] D59291: [Object] Add basic minidump support

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: include/llvm/Object/Minidump.h:77
+   ArrayRef Streams,
+   std::unordered_map StreamMap)
+  : Binary(ID_Minidump, Source), Header(Header), Streams(Streams),

zturner wrote:
> jhenderson wrote:
> > Are you deliberately making a copy of StreamMap? I would normally expect 
> > this to be passed by some form of reference.
> It's actually not making a copy.  This gives the caller the ability to write 
> `std::move(StreamMap)`, which will actually *prevent* a copy.  If you change 
> this to a reference, then in order to store it in the class by value a copy 
> must be made.  By passing it by value in the constructor, you can prevent 
> *any* copies from being made, because the caller can move it in at 
> construction time (and indeed, this is what happens later on).
> 
> 
> However, I would also ask why we're using `std::unordered_map<>` instead of 
> `llvm::DenseMap<>`, which is generally more efficient.
I think Zachary answered the "copy" part very well.

As for unordered_map, my reason for doing that was two-fold:
- laziness to define DenseMapInfo for StreamType
- the fact that setting aside some values for the empty and tombstone keys this 
would render some minidumps invalid (although only a couple of dozen values of 
the entire 32-bit range is used, we must be prepared to handle any input)

However, neither of these two reasons is very strong, so I now define the 
DenseMapInfo traits, and also hard-reject any minidumps that contain one of the 
two stream types (to avoid asserting when trying to insert them into the map). 
We can always switch the special key values, if we find someone uses these for 
something else.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59291



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

[Lldb-commits] [PATCH] D59291: [Object] Add basic minidump support

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 190832.
labath marked 8 inline comments as done.
labath edited the summary of this revision.
labath added a comment.

- fix comments and error messages
- use DenseMap


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59291

Files:
  include/llvm/BinaryFormat/Magic.h
  include/llvm/BinaryFormat/Minidump.h
  include/llvm/BinaryFormat/MinidumpConstants.def
  include/llvm/Object/Binary.h
  include/llvm/Object/Minidump.h
  lib/BinaryFormat/CMakeLists.txt
  lib/BinaryFormat/Magic.cpp
  lib/BinaryFormat/Minidump.cpp
  lib/Object/Binary.cpp
  lib/Object/CMakeLists.txt
  lib/Object/Minidump.cpp
  lib/Object/ObjectFile.cpp
  lib/Object/SymbolicFile.cpp
  unittests/Object/CMakeLists.txt
  unittests/Object/MinidumpTest.cpp

Index: unittests/Object/MinidumpTest.cpp
===
--- /dev/null
+++ unittests/Object/MinidumpTest.cpp
@@ -0,0 +1,256 @@
+//===- MinidumpTest.cpp - Tests for Minidump.cpp --===//
+//
+// 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 "llvm/Object/Minidump.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::object;
+using namespace minidump;
+
+static Expected> create(ArrayRef Data) {
+  return MinidumpFile::create(
+  MemoryBufferRef(toStringRef(Data), "Test buffer"));
+}
+
+TEST(MinidumpFile, BasicInterface) {
+  // A very simple minidump file which contains just a single stream.
+  auto ExpectedFile =
+  create({  // Header
+  'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+  1, 0, 0, 0,   // NumberOfStreams,
+  0x20, 0, 0, 0,// StreamDirectoryRVA
+  0, 1, 2, 3, 4, 5, 6, 7,   // Checksum, TimeDateStamp
+  8, 9, 0, 1, 2, 3, 4, 5,   // Flags
+// Stream Directory
+  3, 0, 0x67, 0x47, 7, 0, 0, 0, // Type, DataSize,
+  0x2c, 0, 0, 0,// RVA
+// Stream
+  'C', 'P', 'U', 'I', 'N', 'F', 'O'});
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  const MinidumpFile &File = **ExpectedFile;
+  const Header &H = File.header();
+  EXPECT_EQ(Header::MagicSignature, H.Signature);
+  EXPECT_EQ(Header::MagicVersion, H.Version);
+  EXPECT_EQ(1u, H.NumberOfStreams);
+  EXPECT_EQ(0x20u, H.StreamDirectoryRVA);
+  EXPECT_EQ(0x03020100u, H.Checksum);
+  EXPECT_EQ(0x07060504u, H.TimeDateStamp);
+  EXPECT_EQ(uint64_t(0x0504030201000908), H.Flags);
+
+  ASSERT_EQ(1u, File.streams().size());
+  const Directory &Stream0 = File.streams()[0];
+  EXPECT_EQ(StreamType::LinuxCPUInfo, Stream0.Type);
+  EXPECT_EQ(7u, Stream0.Location.DataSize);
+  EXPECT_EQ(0x2cu, Stream0.Location.RVA);
+
+  EXPECT_EQ("CPUINFO", toStringRef(File.getRawStream(Stream0)));
+  EXPECT_EQ("CPUINFO",
+toStringRef(*File.getRawStream(StreamType::LinuxCPUInfo)));
+
+  EXPECT_THAT_EXPECTED(File.getSystemInfo(), Failed());
+}
+
+// Use the input from the previous test, but corrupt it in various ways
+TEST(MinidumpFile, create_ErrorCases) {
+  // File too short
+  EXPECT_THAT_EXPECTED(create({'M', 'D', 'M', 'P'}), Failed());
+
+  // Wrong Signature
+  EXPECT_THAT_EXPECTED(
+  create({  // Header
+  '!', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+  1, 0, 0, 0,   // NumberOfStreams,
+  0x20, 0, 0, 0,// StreamDirectoryRVA
+  0, 1, 2, 3, 4, 5, 6, 7,   // Checksum, TimeDateStamp
+  8, 9, 0, 1, 2, 3, 4, 5,   // Flags
+// Stream Directory
+  3, 0, 0x67, 0x47, 7, 0, 0, 0, // Type, DataSize,
+  0x2c, 0, 0, 0,// RVA
+// Stream
+  'C', 'P', 'U', 'I', 'N', 'F', 'O'}),
+  Failed());
+
+  // Wrong Version
+  EXPECT_THAT_EXPECTED(
+  create({  // Header
+  'M', 'D', 'M', 'P', 0x39, 0xa7, 0, 0, // Signature, Version
+  1, 0, 0, 0,   // NumberOfStreams,
+  0x20, 0, 0, 0,// StreamDirectoryRVA
+  0, 1, 2, 3, 4, 5, 6, 7,   // Checksum, TimeDateStamp
+  8, 9, 0, 1, 2, 3,

Re: [Lldb-commits] [lldb] r355103 - [cmake] Move LLDB_DISABLE_LIBEDIT handling code into a central place

[Nico Weber via lldb-commits]

On Fri, Mar 15, 2019 at 10:04 AM Pavel Labath  wrote:

> On 14/03/2019 21:45, Nico Weber wrote:
> > I tried LLVM_ENABLE_PROJECTS=all today, and it fails with
> >
> > CMake Error: The following variables are used in this project, but they
> > are set to NOTFOUND.
> > Please set them or make sure they are set and tested correctly in the
> > CMake files:
> > libedit_INCLUDE_DIRS
> > used as include directory in directory
> > /usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp
> > used as include directory in directory
> > /usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp
> > used as include directory in directory
> > /usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp
> > libedit_LIBRARIES
> >  linked by target "cmTC_43ea9" in directory
> > /usr/local/google/home/thakis/src/llvm-build-project/CMakeFiles/CMakeTmp
> >
> > CMake Error at /usr/share/cmake-3.10/Modules/CheckTypeSize.cmake:114
> > (try_compile):
> >Failed to configure test project build system.
> > Call Stack (most recent call first):
> >/usr/share/cmake-3.10/Modules/CheckTypeSize.cmake:239
> > (__check_type_size_impl)
> >
> >
> /usr/local/google/home/thakis/src/llvm-project/lldb/cmake/modules/LLDBConfig.cmake:106
>
> > (check_type_size)
> >/usr/local/google/home/thakis/src/llvm-project/lldb/CMakeLists.txt:15
> > (include)
> >
> >
> >
> > Is that expected?
> >
>
> Whether it is "expected" to fail the build if you don't have libedit is
> a bit of a complicated question and the answer depends on which side of
> the "should dependencies auto-disable themselves when not found" debate.
>

I am in the "document build prereqs and require them" camp but nothing else
in LLVM's CMake build is, as far as I can tell. In fact, if that was
project philosophy we wouldn't need all these cmake config checks, and yet
we have them. So I think this should be consistent with the rest of LLVM.


> However, it's certainly not expected that the build fails in this way,
> as there is a find_package(LibEdit REQUIRED) call two lines above this
> check.
>
> I've done some digging around, and it seems this is due to a bug in our
> our libedit-finding code. r356263 will fix that, so now the build will
> fail slightly earlier and with a slightly better error message.
>
> If we want to change the libedit dependency to auto-disabling, then we
> should start a separate thread about that.


I think that's the status quo already.


> I believe that's not
> something that this commit has caused. What it may have done is that it
> changed the failure from a build-time thing (we would fail when trying
> to compile code using libedit) to a configure-time failure. That I would
> consider to be an improvement. :)
>

I'm not sure I agree with that; if it fails at build time I can at least
still build everything that's not lldb.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r356271 - [DataFormatters] Remove LLDB_DISABLE_PYTHON from TypeCategory.

[Davide Italiano via lldb-commits]

Author: davide
Date: Fri Mar 15 09:55:51 2019
New Revision: 356271

URL: http://llvm.org/viewvc/llvm-project?rev=356271&view=rev
Log:
[DataFormatters] Remove LLDB_DISABLE_PYTHON from TypeCategory.

Modified:
lldb/trunk/include/lldb/DataFormatters/TypeCategoryMap.h
lldb/trunk/source/DataFormatters/TypeCategory.cpp
lldb/trunk/source/DataFormatters/TypeCategoryMap.cpp

Modified: lldb/trunk/include/lldb/DataFormatters/TypeCategoryMap.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/DataFormatters/TypeCategoryMap.h?rev=356271&r1=356270&r2=356271&view=diff
==
--- lldb/trunk/include/lldb/DataFormatters/TypeCategoryMap.h (original)
+++ lldb/trunk/include/lldb/DataFormatters/TypeCategoryMap.h Fri Mar 15 
09:55:51 2019
@@ -81,10 +81,8 @@ public:
 
   lldb::TypeSummaryImplSP GetSummaryFormat(FormattersMatchData &match_data);
 
-#ifndef LLDB_DISABLE_PYTHON
   lldb::SyntheticChildrenSP
   GetSyntheticChildren(FormattersMatchData &match_data);
-#endif
 
   lldb::TypeValidatorImplSP GetValidator(FormattersMatchData &match_data);
 

Modified: lldb/trunk/source/DataFormatters/TypeCategory.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/TypeCategory.cpp?rev=356271&r1=356270&r2=356271&view=diff
==
--- lldb/trunk/source/DataFormatters/TypeCategory.cpp (original)
+++ lldb/trunk/source/DataFormatters/TypeCategory.cpp Fri Mar 15 09:55:51 2019
@@ -136,7 +136,6 @@ bool TypeCategoryImpl::Get(ValueObject &
 regex_filter = GetRegexTypeFiltersContainer()->Get(candidates, filter_sp,
&reason_filter);
 
-#ifndef LLDB_DISABLE_PYTHON
   bool regex_synth = false;
   uint32_t reason_synth = 0;
   bool pick_synth = false;
@@ -167,14 +166,6 @@ bool TypeCategoryImpl::Get(ValueObject &
 entry = filter_sp;
 return true;
   }
-
-#else
-  if (filter_sp) {
-entry = filter_sp;
-return true;
-  }
-#endif
-
   return false;
 }
 
@@ -210,12 +201,10 @@ void TypeCategoryImpl::Clear(FormatCateg
   eFormatCategoryItemRegexFilter)
 GetRegexTypeFiltersContainer()->Clear();
 
-#ifndef LLDB_DISABLE_PYTHON
   if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth)
 GetTypeSyntheticsContainer()->Clear();
   if ((items & eFormatCategoryItemRegexSynth) == eFormatCategoryItemRegexSynth)
 GetRegexTypeSyntheticsContainer()->Clear();
-#endif
 
   if ((items & eFormatCategoryItemValidator) == eFormatCategoryItemValidator)
 GetTypeValidatorsContainer()->Clear();
@@ -244,12 +233,10 @@ bool TypeCategoryImpl::Delete(ConstStrin
   eFormatCategoryItemRegexFilter)
 success = GetRegexTypeFiltersContainer()->Delete(name) || success;
 
-#ifndef LLDB_DISABLE_PYTHON
   if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth)
 success = GetTypeSyntheticsContainer()->Delete(name) || success;
   if ((items & eFormatCategoryItemRegexSynth) == eFormatCategoryItemRegexSynth)
 success = GetRegexTypeSyntheticsContainer()->Delete(name) || success;
-#endif
 
   if ((items & eFormatCategoryItemValidator) == eFormatCategoryItemValidator)
 success = GetTypeValidatorsContainer()->Delete(name) || success;
@@ -280,12 +267,10 @@ uint32_t TypeCategoryImpl::GetCount(Form
   eFormatCategoryItemRegexFilter)
 count += GetRegexTypeFiltersContainer()->GetCount();
 
-#ifndef LLDB_DISABLE_PYTHON
   if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth)
 count += GetTypeSyntheticsContainer()->GetCount();
   if ((items & eFormatCategoryItemRegexSynth) == eFormatCategoryItemRegexSynth)
 count += GetRegexTypeSyntheticsContainer()->GetCount();
-#endif
 
   if ((items & eFormatCategoryItemValidator) == eFormatCategoryItemValidator)
 count += GetTypeValidatorsContainer()->GetCount();
@@ -306,9 +291,7 @@ bool TypeCategoryImpl::AnyMatches(ConstS
   lldb::TypeFormatImplSP format_sp;
   lldb::TypeSummaryImplSP summary_sp;
   TypeFilterImpl::SharedPointer filter_sp;
-#ifndef LLDB_DISABLE_PYTHON
   ScriptedSyntheticChildren::SharedPointer synth_sp;
-#endif
   TypeValidatorImpl::SharedPointer validator_sp;
 
   if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue) {
@@ -371,7 +354,6 @@ bool TypeCategoryImpl::AnyMatches(ConstS
 }
   }
 
-#ifndef LLDB_DISABLE_PYTHON
   if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth) {
 if (GetTypeSyntheticsContainer()->Get(type_name, synth_sp)) {
   if (matching_category)
@@ -391,7 +373,6 @@ bool TypeCategoryImpl::AnyMatches(ConstS
   return true;
 }
   }
-#endif
 
   if ((items & eFormatCategoryItemValidator) == eFormatCategoryItemValidator) {
 if (GetTypeValidatorsContainer()->Get(type_name, validator_sp)) {
@@ -464,7 +445,6 @@ TypeCategoryImpl::GetFilterForType(lldb:
   return retval;
 }
 
-#ifndef LLDB_DISABLE_PYTHON
 TypeCategoryImpl::SynthContainer

[Lldb-commits] [lldb] r356275 - Implement a better way of not passing the sanitizer environment on to tests.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Fri Mar 15 10:22:00 2019
New Revision: 356275

URL: http://llvm.org/viewvc/llvm-project?rev=356275&view=rev
Log:
Implement a better way of not passing the sanitizer environment on to tests.

rdar://problem/48889580

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py?rev=356275&r1=356274&r2=356275&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py Fri Mar 15 10:22:00 
2019
@@ -1875,9 +1875,8 @@ class TestBase(Base):
 self.runCmd('settings set symbols.enable-external-lookup false')
 
 # Make sure that a sanitizer LLDB's environment doesn't get passed on.
-if (('DYLD_INSERT_LIBRARIES' in os.environ) and
-'libclang_rt.asan' in os.environ['DYLD_INSERT_LIBRARIES']):
-self.runCmd('settings set target.env-vars DYLD_INSERT_LIBRARIES=')
+if 'DYLD_LIBRARY_PATH' in os.environ:
+self.runCmd('settings set target.env-vars DYLD_LIBRARY_PATH=')
 
 if "LLDB_MAX_LAUNCH_COUNT" in os.environ:
 self.maxLaunchCount = int(os.environ["LLDB_MAX_LAUNCH_COUNT"])


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

[Lldb-commits] [lldb] r356278 - Return Error and Expected from more DWARF interfaces.

[Zachary Turner via lldb-commits]

Author: zturner
Date: Fri Mar 15 10:32:05 2019
New Revision: 356278

URL: http://llvm.org/viewvc/llvm-project?rev=356278&view=rev
Log:
Return Error and Expected from more DWARF interfaces.

This continues the work of introducing Error and Expected into
the DWARF parsing interfaces, this time for the DWARFCompileUnit
and DWARFDebugAranges classes.

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp?rev=356278&r1=356277&r2=356278&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp Fri Mar 15 
10:32:05 2019
@@ -10,6 +10,7 @@
 
 #include "SymbolFileDWARF.h"
 #include "lldb/Utility/Stream.h"
+#include "llvm/Object/Error.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -17,50 +18,64 @@ using namespace lldb_private;
 DWARFCompileUnit::DWARFCompileUnit(SymbolFileDWARF *dwarf2Data)
 : DWARFUnit(dwarf2Data) {}
 
-DWARFUnitSP DWARFCompileUnit::Extract(SymbolFileDWARF *dwarf2Data,
-  const DWARFDataExtractor &debug_info,
-  lldb::offset_t *offset_ptr) {
+llvm::Expected
+DWARFCompileUnit::extract(SymbolFileDWARF *dwarf2Data,
+  const DWARFDataExtractor &debug_info,
+  lldb::offset_t *offset_ptr) {
+  assert(debug_info.ValidOffset(*offset_ptr));
+
   // std::make_shared would require the ctor to be public.
   std::shared_ptr cu_sp(new DWARFCompileUnit(dwarf2Data));
 
   cu_sp->m_offset = *offset_ptr;
 
-  if (debug_info.ValidOffset(*offset_ptr)) {
-dw_offset_t abbr_offset;
-const DWARFDebugAbbrev *abbr = dwarf2Data->DebugAbbrev();
-cu_sp->m_length = debug_info.GetDWARFInitialLength(offset_ptr);
-cu_sp->m_version = debug_info.GetU16(offset_ptr);
-
-if (cu_sp->m_version == 5) {
-  cu_sp->m_unit_type = debug_info.GetU8(offset_ptr);
-  cu_sp->m_addr_size = debug_info.GetU8(offset_ptr);
-  abbr_offset = debug_info.GetDWARFOffset(offset_ptr);
-
-  if (cu_sp->m_unit_type == llvm::dwarf::DW_UT_skeleton)
-cu_sp->m_dwo_id = debug_info.GetU64(offset_ptr);
-} else {
-  abbr_offset = debug_info.GetDWARFOffset(offset_ptr);
-  cu_sp->m_addr_size = debug_info.GetU8(offset_ptr);
-}
-
-bool length_OK =
-debug_info.ValidOffset(cu_sp->GetNextCompileUnitOffset() - 1);
-bool version_OK = SymbolFileDWARF::SupportedVersion(cu_sp->m_version);
-bool abbr_offset_OK =
-dwarf2Data->get_debug_abbrev_data().ValidOffset(abbr_offset);
-bool addr_size_OK = (cu_sp->m_addr_size == 4) || (cu_sp->m_addr_size == 8);
-
-if (length_OK && version_OK && addr_size_OK && abbr_offset_OK &&
-abbr != NULL) {
-  cu_sp->m_abbrevs = abbr->GetAbbreviationDeclarationSet(abbr_offset);
-  return cu_sp;
-}
-
-// reset the offset to where we tried to parse from if anything went wrong
-*offset_ptr = cu_sp->m_offset;
+  dw_offset_t abbr_offset;
+  const DWARFDebugAbbrev *abbr = dwarf2Data->DebugAbbrev();
+  if (!abbr)
+return llvm::make_error(
+"No debug_abbrev data");
+
+  cu_sp->m_length = debug_info.GetDWARFInitialLength(offset_ptr);
+  cu_sp->m_version = debug_info.GetU16(offset_ptr);
+
+  if (cu_sp->m_version == 5) {
+cu_sp->m_unit_type = debug_info.GetU8(offset_ptr);
+cu_sp->m_addr_size = debug_info.GetU8(offset_ptr);
+abbr_offset = debug_info.GetDWARFOffset(offset_ptr);
+
+if (cu_sp->m_unit_type == llvm::dwarf::DW_UT_skeleton)
+  cu_sp->m_dwo_id = debug_info.GetU64(offset_ptr);
+  } else {
+abbr_offset = debug_info.GetDWARFOffset(offset_ptr);
+cu_sp->m_addr_size = debug_info.GetU8(offset_ptr);
   }
 
-  return nullptr;
+  bool length_OK =
+  debug_info.ValidOffset(cu_sp->GetNextCompileUnitOffset() - 1);
+  bool version_OK = SymbolFileDWARF::SupportedVersion(cu_sp->m_version);
+  bool abbr_offset_OK =
+  dwarf2Data->get_debug_abbrev_data().ValidOffset(abbr_offset);
+  bool addr_size_OK = (cu_sp->m_addr_size == 4) || (cu_sp->m_addr_size == 8);
+
+  if (!length_OK)
+return llvm::make_error(
+   

[Lldb-commits] [PATCH] D59381: Change CompileUnit and ARanges interfaces to propagate errors

[Zachary Turner via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Revision".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL356278: Return Error and Expected from more DWARF 
interfaces. (authored by zturner, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59381?vs=190700&id=190851#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59381

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
@@ -10,12 +10,14 @@
 #define SymbolFileDWARF_DWARFCompileUnit_h_
 
 #include "DWARFUnit.h"
+#include "llvm/Support/Error.h"
 
 class DWARFCompileUnit : public DWARFUnit {
 public:
-  static DWARFUnitSP Extract(SymbolFileDWARF *dwarf2Data,
- const lldb_private::DWARFDataExtractor &debug_info,
- lldb::offset_t *offset_ptr);
+  static llvm::Expected
+  extract(SymbolFileDWARF *dwarf2Data,
+  const lldb_private::DWARFDataExtractor &debug_info,
+  lldb::offset_t *offset_ptr);
   void Dump(lldb_private::Stream *s) const override;
 
   //--
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
@@ -10,6 +10,7 @@
 
 #include "SymbolFileDWARF.h"
 #include "lldb/Utility/Stream.h"
+#include "llvm/Object/Error.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -17,50 +18,64 @@
 DWARFCompileUnit::DWARFCompileUnit(SymbolFileDWARF *dwarf2Data)
 : DWARFUnit(dwarf2Data) {}
 
-DWARFUnitSP DWARFCompileUnit::Extract(SymbolFileDWARF *dwarf2Data,
-  const DWARFDataExtractor &debug_info,
-  lldb::offset_t *offset_ptr) {
+llvm::Expected
+DWARFCompileUnit::extract(SymbolFileDWARF *dwarf2Data,
+  const DWARFDataExtractor &debug_info,
+  lldb::offset_t *offset_ptr) {
+  assert(debug_info.ValidOffset(*offset_ptr));
+
   // std::make_shared would require the ctor to be public.
   std::shared_ptr cu_sp(new DWARFCompileUnit(dwarf2Data));
 
   cu_sp->m_offset = *offset_ptr;
 
-  if (debug_info.ValidOffset(*offset_ptr)) {
-dw_offset_t abbr_offset;
-const DWARFDebugAbbrev *abbr = dwarf2Data->DebugAbbrev();
-cu_sp->m_length = debug_info.GetDWARFInitialLength(offset_ptr);
-cu_sp->m_version = debug_info.GetU16(offset_ptr);
-
-if (cu_sp->m_version == 5) {
-  cu_sp->m_unit_type = debug_info.GetU8(offset_ptr);
-  cu_sp->m_addr_size = debug_info.GetU8(offset_ptr);
-  abbr_offset = debug_info.GetDWARFOffset(offset_ptr);
-
-  if (cu_sp->m_unit_type == llvm::dwarf::DW_UT_skeleton)
-cu_sp->m_dwo_id = debug_info.GetU64(offset_ptr);
-} else {
-  abbr_offset = debug_info.GetDWARFOffset(offset_ptr);
-  cu_sp->m_addr_size = debug_info.GetU8(offset_ptr);
-}
-
-bool length_OK =
-debug_info.ValidOffset(cu_sp->GetNextCompileUnitOffset() - 1);
-bool version_OK = SymbolFileDWARF::SupportedVersion(cu_sp->m_version);
-bool abbr_offset_OK =
-dwarf2Data->get_debug_abbrev_data().ValidOffset(abbr_offset);
-bool addr_size_OK = (cu_sp->m_addr_size == 4) || (cu_sp->m_addr_size == 8);
-
-if (length_OK && version_OK && addr_size_OK && abbr_offset_OK &&
-abbr != NULL) {
-  cu_sp->m_abbrevs = abbr->GetAbbreviationDeclarationSet(abbr_offset);
-  return cu_sp;
-}
-
-// reset the offset to where we tried to parse from if anything went wrong
-*offset_ptr = cu_sp->m_offset;
+  dw_offset_t abbr_offset;
+  const DWARFDebugAbbrev *abbr = dwarf2Data->DebugAbbrev();
+  if (!abbr)
+return llvm::make_error(
+"No debug_abbrev data");
+
+  cu_sp->m_length = debug_info.GetDWARFInitialLength(offset_ptr);
+  cu_sp->m_version = debug_info.GetU16(offset_ptr);
+
+  if (cu_sp->m_versi

[Lldb-commits] [lldb] r356284 - Abbreviation declarations are required to have non-null tags.

[Zachary Turner via lldb-commits]

Author: zturner
Date: Fri Mar 15 11:00:43 2019
New Revision: 356284

URL: http://llvm.org/viewvc/llvm-project?rev=356284&view=rev
Log:
Abbreviation declarations are required to have non-null tags.

Treat a null tag as an error.

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

Modified: 
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp?rev=356284&r1=356283&r2=356284&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp 
Fri Mar 15 11:00:43 2019
@@ -34,11 +34,9 @@ DWARFAbbreviationDeclaration::extract(co
 
   m_attributes.clear();
   m_tag = data.GetULEB128(offset_ptr);
-  if (m_tag == DW_TAG_null) {
-// FIXME: According to the DWARF spec this may actually be malformed.
-// Should this return an error instead?
-return DWARFEnumState::Complete;
-  }
+  if (m_tag == DW_TAG_null)
+return llvm::make_error(
+"abbrev decl requires non-null tag.");
 
   m_has_children = data.GetU8(offset_ptr);
 


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

[Lldb-commits] [PATCH] D59430: Update DwarfDebugInfoEntry to use llvm::Error and llvm::Expected

[Zachary Turner via Phabricator via lldb-commits]

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

This hits the next batch of classes and adds better error handling and recovery 
to them.  After this, the only remaining case is the line table stuff, at which 
point we should be able to get all logging out of the low level DWARF parsing 
entirely.


https://reviews.llvm.org/D59430

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -723,18 +723,25 @@
 }
 
 DWARFDebugRangesBase *SymbolFileDWARF::DebugRanges() {
-  if (m_ranges == NULL) {
-static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
-Timer scoped_timer(func_cat, "%s this = %p", LLVM_PRETTY_FUNCTION,
-   static_cast(this));
+  if (m_ranges)
+return m_ranges.get();
+
+  static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+  Timer scoped_timer(func_cat, "%s this = %p", LLVM_PRETTY_FUNCTION,
+ static_cast(this));
 
-if (get_debug_ranges_data().GetByteSize() > 0)
-  m_ranges.reset(new DWARFDebugRanges());
-else if (get_debug_rnglists_data().GetByteSize() > 0)
-  m_ranges.reset(new DWARFDebugRngLists());
+  if (get_debug_ranges_data().GetByteSize() > 0)
+m_ranges = llvm::make_unique();
+  else if (get_debug_rnglists_data().GetByteSize() > 0)
+m_ranges = llvm::make_unique();
+  else
+return nullptr;
 
-if (m_ranges)
-  m_ranges->Extract(this);
+  auto error = m_ranges->extract(this);
+  if (error) {
+Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
+LLDB_LOG_ERROR(log, std::move(error),
+   "Unable to get debug range information: {0}");
   }
   return m_ranges.get();
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
@@ -12,13 +12,15 @@
 #include "DWARFDIE.h"
 #include "SymbolFileDWARF.h"
 
+#include "llvm/Support/Error.h"
+
 #include 
 
 class DWARFDebugRangesBase {
 public:
   virtual ~DWARFDebugRangesBase(){};
 
-  virtual void Extract(SymbolFileDWARF *dwarf2Data) = 0;
+  virtual llvm::Error extract(SymbolFileDWARF *dwarf2Data) = 0;
   virtual bool FindRanges(const DWARFUnit *cu, dw_offset_t debug_ranges_offset,
   DWARFRangeList &range_list) const = 0;
   virtual uint64_t GetOffset(size_t Index) const = 0;
@@ -28,7 +30,7 @@
 public:
   DWARFDebugRanges();
 
-  void Extract(SymbolFileDWARF *dwarf2Data) override;
+  llvm::Error extract(SymbolFileDWARF *dwarf2Data) override;
   bool FindRanges(const DWARFUnit *cu, dw_offset_t debug_ranges_offset,
   DWARFRangeList &range_list) const override;
   uint64_t GetOffset(size_t Index) const override;
@@ -38,8 +40,9 @@
lldb::offset_t *offset_ptr, dw_addr_t cu_base_addr);
 
 protected:
-  bool Extract(SymbolFileDWARF *dwarf2Data, lldb::offset_t *offset_ptr,
-   DWARFRangeList &range_list);
+  llvm::Error extractOne(SymbolFileDWARF *dwarf2Data,
+ lldb::offset_t *offset_ptr,
+ DWARFRangeList &range_list);
 
   typedef std::map range_map;
   typedef range_map::iterator range_map_iterator;
@@ -56,7 +59,7 @@
   };
 
 public:
-  void Extract(SymbolFileDWARF *dwarf2Data) override;
+  llvm::Error extract(SymbolFileDWARF *dwarf2Data) override;
   bool FindRanges(const DWARFUnit *cu, dw_offset_t debug_ranges_offset,
   DWARFRangeList &range_list) const override;
   uint64_t GetOffset(size_t Index) const override;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
@@ -10,6 +10,7 @@
 #include "DWARFUnit.h"
 #include "SymbolFileDWARF.h"
 #include "lldb/Utility/Stream.h"
+#include "llvm/Object/Error.h"
 #include 
 
 using namespace lldb_private;
@@ -29,23 +30,33 @@
 
 DWARFDebugRanges::DWARFDebugRanges() : m_range_map() {}
 
-void DWARFDebugRanges::Extract(SymbolFileDWARF *dwarf2Data) {
+llvm::Error DWARFDebugRanges::extract(SymbolFileDWARF *dwarf2Data) {
   DWARFRangeList range_list;
   lldb::offset_t offset = 0;
   dw_offset_t debug_ranges_offset = offset;
-  while (Extract(dwarf2Data, &offset, range_list)) {
+
+  const DWARFDataExtrac

[Lldb-commits] [PATCH] D59427: [lldb] [API] Split SBRegistry into smaller files

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

It looks good to me, but maybe @JDevlieghere has a better idea how to 
optimize it.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59427



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

[Lldb-commits] [PATCH] D59430: Update DwarfDebugInfoEntry to use llvm::Error and llvm::Expected

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:218
+  assert(offset < cu->GetNextCompileUnitOffset() &&
+ debug_info_data.ValidOffset(offset));
 

Should this be a lldb_assert() followed by `return make_error` instead?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1170
   const dw_offset_t die_offset, Stream &s) {
-  if (dwarf2Data == NULL) {
-s.PutCString("NULL");
-return false;
-  }
+  assert(dwarf2Data);
 

Instead of passing a pointer and asserting, wouldn't it be better to pass a 
`SymbolFileDWARF &`?


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

https://reviews.llvm.org/D59430



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

[Lldb-commits] [PATCH] D59430: Update DwarfDebugInfoEntry to use llvm::Error and llvm::Expected

[Zachary Turner via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:218
+  assert(offset < cu->GetNextCompileUnitOffset() &&
+ debug_info_data.ValidOffset(offset));
 

aprantl wrote:
> Should this be a lldb_assert() followed by `return make_error` instead?
I think it's reasonable to treat this as an internal consistency check, where 
the pre-condition of this function is "offset must be a valid offset".  Similar 
to indexing an array out of bounds, your `operator[]` implementation would 
assert that the index you passed is within range.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1170
   const dw_offset_t die_offset, Stream &s) {
-  if (dwarf2Data == NULL) {
-s.PutCString("NULL");
-return false;
-  }
+  assert(dwarf2Data);
 

aprantl wrote:
> Instead of passing a pointer and asserting, wouldn't it be better to pass a 
> `SymbolFileDWARF &`?
Yes, but I was trying to keep the change set minimal, and doing so would 
propagate that change many levels up the callstack until we reach 
`SymbolFileDwarf`, at which point we would change all calls to pass `*this` 
instead of `this`.  That's a reasonable change, but should probably be done 
separately to keep logically separate changes separate.

Note that, long term, we just won't even pass a `SymbolFileDWARF` to this 
function at all, because if the point is to decouple the high and low level 
interfaces, then the low-level interface can't know about the high level 
interface.


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

https://reviews.llvm.org/D59430



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

[Lldb-commits] [PATCH] D59427: [lldb] [API] Split SBRegistry into smaller files

[Jonas Devlieghere via Phabricator via lldb-commits]

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

In D59427#1431383 , @krytarowski wrote:

> It looks good to me, but maybe @JDevlieghere   has a better idea how to 
> optimize it.


How about moving the register macros in the corresponding files? They're 
already grouped by file, so it should be easy enough. Something like:

  void RegisterSBProcess();
  void RegisterSBTarget();
  ...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59427



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

[Lldb-commits] [PATCH] D59427: [lldb] [API] Split SBRegistry into smaller files

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

Or better yet, make it a static method on each SB class.  E.g. 
`SBTarget::InitializeReproducerRegistry();` etc, one for each class.

Random thought, but this all seems like a very high maintenance strategy, 
having to manually update this registry when methods are added or removed.  Did 
you ever consider auto-generating them by parsing the SWIG?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59427



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

Re: [Lldb-commits] [PATCH] D59427: [lldb] [API] Split SBRegistry into smaller files

[Jonas Devlieghere via lldb-commits]

They actually are auto generated, using the lldb-inst tool. Currently the
record macros are inserted in place, while the register macros are dumped
to stdout. I didn’t want to add too much complexity. However, it would be
really easy to just dump the register macros in the file as well.

On Fri, Mar 15, 2019 at 13:44 Zachary Turner via Phabricator <
revi...@reviews.llvm.org> wrote:

> zturner added a comment.
>
> Or better yet, make it a static method on each SB class.  E.g.
> `SBTarget::InitializeReproducerRegistry();` etc, one for each class.
>
> Random thought, but this all seems like a very high maintenance strategy,
> having to manually update this registry when methods are added or removed.
> Did you ever consider auto-generating them by parsing the SWIG?
>
>
> Repository:
>   rLLDB LLDB
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D59427/new/
>
> https://reviews.llvm.org/D59427
>
>
>
> --
Sent from my iPhone
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D59427: [lldb] [API] Split SBRegistry into smaller files

[Jonas Devlieghere via lldb-commits]

That being said, the tool won't generate register macros when the
corresponding record macro is already in place, so the initial move would
have to be done by hand.

On Fri, Mar 15, 2019 at 1:47 PM Jonas Devlieghere 
wrote:

> They actually are auto generated, using the lldb-inst tool. Currently the
> record macros are inserted in place, while the register macros are dumped
> to stdout. I didn’t want to add too much complexity. However, it would be
> really easy to just dump the register macros in the file as well.
>
> On Fri, Mar 15, 2019 at 13:44 Zachary Turner via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> zturner added a comment.
>>
>> Or better yet, make it a static method on each SB class.  E.g.
>> `SBTarget::InitializeReproducerRegistry();` etc, one for each class.
>>
>> Random thought, but this all seems like a very high maintenance strategy,
>> having to manually update this registry when methods are added or removed.
>> Did you ever consider auto-generating them by parsing the SWIG?
>>
>>
>> Repository:
>>   rLLDB LLDB
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D59427/new/
>>
>> https://reviews.llvm.org/D59427
>>
>>
>>
>> --
> Sent from my iPhone
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: zturner, labath.
Herald added a subscriber: javed.absar.
Herald added a reviewer: serge-sans-paille.

This patch fixes:

- UUIDs now don't include the age field from a PDB70 when the age is zero. 
Prior to this they would incorrectly contain the zero age which stopped us from 
being able to match up the UUID with real files.
- UUIDs for Apple targets get the first 32 bit value and next two 16 bit values 
swapped. Breakpad incorrectly swaps these values when it creates darwin 
minidump files, so this must be undone so we can match up symbol files with the 
minidump modules.
- UUIDs that are all zeroes are treated as invalid UUIDs. Breakpad will always 
save out a UUID, even if one wasn't available. This caused all files that have 
UUID values of zero to be uniqued to the first module that had a zero UUID. We 
now don't fill in the UUID if it is all zeroes.

Added tests for PDB70 and ELF build ID based CvRecords.


https://reviews.llvm.org/D59433

Files:
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-16.dmp
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-20.dmp
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-no-age.dmp
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-with-age.dmp
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-zero-uuids.dmp
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/macos-arm-uuids-no-age.dmp
  source/Plugins/Process/minidump/MinidumpParser.cpp

Index: source/Plugins/Process/minidump/MinidumpParser.cpp
===
--- source/Plugins/Process/minidump/MinidumpParser.cpp
+++ source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -165,21 +165,44 @@
   static_cast(static_cast(*signature));
 
   if (cv_signature == CvSignature::Pdb70) {
-// PDB70 record
 const CvRecordPdb70 *pdb70_uuid = nullptr;
 Status error = consumeObject(cv_record, pdb70_uuid);
-if (!error.Fail()) {
-  auto arch = GetArchitecture();
-  // For Apple targets we only need a 16 byte UUID so that we can match
-  // the UUID in the Module to actual UUIDs from the built binaries. The
-  // "Age" field is zero in breakpad minidump files for Apple targets, so
-  // we restrict the UUID to the "Uuid" field so we have a UUID we can use
-  // to match.
-  if (arch.GetTriple().getVendor() == llvm::Triple::Apple)
-return UUID::fromData(pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
-  else
-return UUID::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
+if (error.Fail())
+  return UUID();
+// If the age field is not zero, then include the entire pdb70_uuid struct
+if (pdb70_uuid->Age != 0)
+  return UUID::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
+
+// Many times UUIDs are all zeroes. This can cause more than one module
+// to claim it has a valid UUID of all zeroes and causes the files to all
+// merge into the first module that claims this valid zero UUID.
+bool all_zeroes = true;
+for (size_t i = 0; all_zeroes && i < sizeof(pdb70_uuid->Uuid); ++i)
+  all_zeroes = pdb70_uuid->Uuid[i] == 0;
+if (all_zeroes)
+  return UUID();
+
+if (GetArchitecture().GetTriple().getVendor() == llvm::Triple::Apple) {
+  // Breakpad incorrectly byte swaps the first 32 bit and next 2 16 bit
+  // values in the UUID field. Undo this so we can match things up
+  // with our symbol files
+  uint8_t apple_uuid[16];
+  // Byte swap the first 32 bits
+  apple_uuid[0] = pdb70_uuid->Uuid[3];
+  apple_uuid[1] = pdb70_uuid->Uuid[2];
+  apple_uuid[2] = pdb70_uuid->Uuid[1];
+  apple_uuid[3] = pdb70_uuid->Uuid[0];
+  // Byte swap the next 16 bit value
+  apple_uuid[4] = pdb70_uuid->Uuid[5];
+  apple_uuid[5] = pdb70_uuid->Uuid[4];
+  // Byte swap the next 16 bit value
+  apple_uuid[6] = pdb70_uuid->Uuid[7];
+  apple_uuid[7] = pdb70_uuid->Uuid[6];
+  for (size_t i = 8; i < sizeof(pdb70_uuid->Uuid); ++i)
+apple_uuid[i] = pdb70_uuid->Uuid[i];
+  return UUID::fromData(apple_uuid, sizeof(apple_uuid));
 }
+return UUID::fromData(pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
   } else if (cv_signature == CvSignature::ElfBuildId)
 return UUID::fromData(cv_record);
 
Index: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
===
--- packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -510

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

"MinidumpNew" is a little bit vague.  What's "new" about it?  Is there a way we 
could come up with a better name?

As an aside, since there's no interactivity in these tests, they seem like a 
good candidate for lit/Minidump, with 1 file for each test.  Something like:

  ; zero-uuid-modules.test
  ; RUN: lldb -core %S/Inputs/linux-arm-zero-uuids.dmp -S %p | FileCheck %s
  
  target modules list
  
  ; CHECK: [  0] -----  
  /tmp/a
  ; CHECK: [  1] -----  
  /tmp/b

Let's see what Pavel thinks though.


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

https://reviews.llvm.org/D59433



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

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D59433#1431488 , @zturner wrote:

> "MinidumpNew" is a little bit vague.  What's "new" about it?  Is there a way 
> we could come up with a better name?


This was an existing file that I just added new tests to, and it seemed to be 
where most of the minidump tests were.

> As an aside, since there's no interactivity in these tests, they seem like a 
> good candidate for lit/Minidump, with 1 file for each test.  Something like:
> 
>   ; zero-uuid-modules.test
>   ; RUN: lldb -core %S/Inputs/linux-arm-zero-uuids.dmp -S %p | FileCheck %s
>   
>   target modules list
>   
>   ; CHECK: [  0] -----
> /tmp/a
>   ; CHECK: [  1] -----
> /tmp/b
> 
> 
> Let's see what Pavel thinks though.

I wish the lit tests worked with the Xcode build


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

https://reviews.llvm.org/D59433



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