[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D59719#1442563 , @davide wrote:

> In D59719#1441254 , @labath wrote:
>
> > It sounds to me like you could achieve the same thing by generalizing the 
> > LLDB_PYTHON_HOME logic in LLDBConfig.cmake. This would have the advantage 
> > of centralizing the way we manage python-finding logic (instead of each OS 
> > doing it's own thing) and also enable those users, who know what they are 
> > doing, to override this logic and point lldb to a different python. (I 
> > don't know if there are any such users, but it does not sounds like an 
> > impossible scenario).
> >
> > I think all it would take is to do something like:
> >
> > - move LLDB_RELOCATABLE_PYTHON handling outside of `if(WINDOWS)`
> > - have the default value of `LLDB_RELOCATABLE_PYTHON` be `false` for darwin
> > - possibly tweak the python-finding logic so that it prefers the one in 
> > /System/Library/Frameworks/...
>
>
> Oh, this won't work with xcodebuild, I'm afraid.


Why not? The xcode build could hardcode the value for LLDB_PYTHON_HOME, just 
like you do here now. That could be done either in `lldb/Host/Config.h`, or by 
passing the appropriate `-D` flag to the compiler via the xcode project file. 
My preference would be to use lldb/Host/Config.h, and to also move the cmake 
build off of the explicit `-D`, and put this variable into Config.h.cmake as 
well.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59719



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

[Lldb-commits] [lldb] r356992 - Minidump: Use minidump types defined in llvm

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Mar 26 06:23:01 2019
New Revision: 356992

URL: http://llvm.org/viewvc/llvm-project?rev=356992&view=rev
Log:
Minidump: Use minidump types defined in llvm

This is the next step in moving the minidump parsing into llvm. I remove
the minidump structures already defined in the llvm Object library and
convert our parser to use those. NFC.

Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=356992&r1=356991&r2=356992&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Tue Mar 26 
06:23:01 2019
@@ -28,19 +28,34 @@ static llvm::Error stringError(llvm::Str
  llvm::inconvertibleErrorCode());
 }
 
+const Header *ParseHeader(llvm::ArrayRef &data) {
+  const Header *header = nullptr;
+  Status error = consumeObject(data, header);
+
+  uint32_t signature = header->Signature;
+  uint32_t version = header->Version & 0x;
+  // the high 16 bits of the version field are implementation specific
+
+  if (error.Fail() || signature != Header::MagicSignature ||
+  version != Header::MagicVersion)
+return nullptr;
+
+  return header;
+}
+
 llvm::Expected
 MinidumpParser::Create(const lldb::DataBufferSP &data_sp) {
-  if (data_sp->GetByteSize() < sizeof(MinidumpHeader))
+  if (data_sp->GetByteSize() < sizeof(Header))
 return stringError("Buffer too small.");
 
   llvm::ArrayRef header_data(data_sp->GetBytes(),
-  sizeof(MinidumpHeader));
-  const MinidumpHeader *header = MinidumpHeader::Parse(header_data);
+  sizeof(Header));
+  const Header *header = ParseHeader(header_data);
   if (!header)
 return stringError("invalid minidump: can't parse the header");
 
   // A minidump without at least one stream is clearly ill-formed
-  if (header->streams_count == 0)
+  if (header->NumberOfStreams == 0)
 return stringError("invalid minidump: no streams present");
 
   struct FileRange {
@@ -58,42 +73,42 @@ MinidumpParser::Create(const lldb::DataB
   // - truncation (streams pointing past the end of file)
   std::vector minidump_map;
 
-  minidump_map.emplace_back(0, sizeof(MinidumpHeader));
+  minidump_map.emplace_back(0, sizeof(Header));
 
   // Add the directory entries to the file map
-  FileRange directory_range(header->stream_directory_rva,
-header->streams_count * sizeof(MinidumpDirectory));
+  FileRange directory_range(header->StreamDirectoryRVA,
+header->NumberOfStreams * sizeof(Directory));
   if (directory_range.end() > file_size)
 return stringError("invalid minidump: truncated streams directory");
   minidump_map.push_back(directory_range);
 
-  llvm::DenseMap directory_map;
+  llvm::DenseMap directory_map;
 
   // Parse stream directory entries
   llvm::ArrayRef directory_data(
   data_sp->GetBytes() + directory_range.offset, directory_range.size);
-  for (uint32_t i = 0; i < header->streams_count; ++i) {
-const MinidumpDirectory *directory_entry = nullptr;
+  for (uint32_t i = 0; i < header->NumberOfStreams; ++i) {
+const Directory *directory_entry = nullptr;
 Status error = consumeObject(directory_data, directory_entry);
 if (error.Fail())
   return error.ToError();
-if (directory_entry->stream_type == 0) {
+if (directory_entry->Type == StreamType::Unused) {
   // Ignore dummy streams (technically ill-formed, but a number of
   // existing minidumps seem to contain such streams)
-  if (directory_entry->location.data_size == 0)
+  if (directory_entry->Location.DataSize == 0)
 continue;
   return stringError("invalid minidump: bad stream type");
 }
 // Update the streams map, checking for duplicate stream types
 if (!directory_map
- .insert({directory_entry->stream_type, directory_entry->location})
+ .insert({directory_entry->Type, directory_entry->Location})
  .second)
   return stringError("invalid minidump: duplicate stream type");
 
 // Ignore the zero-length streams for layout checks
-if (directory_entry->location.data_size != 0) {
-  minidump_map.emplace_back(directory_entry->location.rva,
-directory_entry->location.data_size);
+if (directory_entry->Location.DataSize != 0) {
+  minidump_map.emplace_back

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.
Herald added a subscriber: jdoerfert.

Kind reminder. I believe all discussions have been resolved.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55718



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

[Lldb-commits] [PATCH] D59495: Fix an out-of-bounds error in RegisterContextDarwin_arm64

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Jason, what do you make of this? I believe the fix is pretty straight-forward, 
but since this does actually change behaviour, I think it would be good if you 
had a quick look at it first.


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

https://reviews.llvm.org/D59495



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

[Lldb-commits] [lldb] r356993 - Remove the TypePair class

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Mar 26 06:35:54 2019
New Revision: 356993

URL: http://llvm.org/viewvc/llvm-project?rev=356993&view=rev
Log:
Remove the TypePair class

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

Reviewers: clayborg, jingham, zturner

Subscribers: mehdi_amini, dexonsmith, jdoerfert, lldb-commits

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

Modified:
lldb/trunk/include/lldb/DataFormatters/FormatClasses.h
lldb/trunk/include/lldb/Symbol/Type.h
lldb/trunk/include/lldb/lldb-forward.h
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=356993&r1=356992&r2=356993&view=diff
==
--- lldb/trunk/include/lldb/DataFormatters/FormatClasses.h (original)
+++ lldb/trunk/include/lldb/DataFormatters/FormatClasses.h Tue Mar 26 06:35:54 
2019
@@ -122,14 +122,14 @@ public:
   TypeNameSpecifierImpl(lldb::TypeSP type) : m_is_regex(false), m_type() {
 if (type) {
   m_type.m_type_name = type->GetName().GetStringRef();
-  m_type.m_type_pair.SetType(type);
+  m_type.m_compiler_type = type->GetForwardCompilerType();
 }
   }
 
   TypeNameSpecifierImpl(CompilerType type) : m_is_regex(false), m_type() {
 if (type.IsValid()) {
   m_type.m_type_name.assign(type.GetConstTypeName().GetCString());
-  m_type.m_type_pair.SetType(type);
+  m_type.m_compiler_type = type;
 }
   }
 
@@ -140,8 +140,8 @@ public:
   }
 
   CompilerType GetCompilerType() {
-if (m_type.m_type_pair.IsValid())
-  return m_type.m_type_pair.GetCompilerType();
+if (m_type.m_compiler_type.IsValid())
+  return m_type.m_compiler_type;
 return CompilerType();
   }
 
@@ -149,11 +149,10 @@ public:
 
 private:
   bool m_is_regex;
-  // this works better than TypeAndOrName because the latter only wraps a
-  // TypeSP whereas TypePair can also be backed by a CompilerType
+  // TODO: Replace this with TypeAndOrName.
   struct TypeOrName {
 std::string m_type_name;
-TypePair m_type_pair;
+CompilerType m_compiler_type;
   };
   TypeOrName m_type;
 

Modified: lldb/trunk/include/lldb/Symbol/Type.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/Type.h?rev=356993&r1=356992&r2=356993&view=diff
==
--- lldb/trunk/include/lldb/Symbol/Type.h (original)
+++ lldb/trunk/include/lldb/Symbol/Type.h Tue Mar 26 06:35:54 2019
@@ -237,81 +237,6 @@ protected:
   bool ResolveClangType(ResolveState compiler_type_resolve_state);
 };
 
-// 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() {}
-
-  TypePair(CompilerType type) : compiler_type(type) {}
-
-  TypePair(lldb::TypeSP type) : compiler_type(type->GetForwardCompilerType()) 
{}
-
-  bool IsValid() const { return compiler_type.IsValid(); }
-
-  explicit operator bool() const { return IsValid(); }
-
-  bool operator==(const TypePair &rhs) const {
-return compiler_type == rhs.compiler_type;
-  }
-
-  bool operator!=(const TypePair &rhs) const { return !(*this == rhs); }
-
-  void Clear() { compiler_type.Clear(); }
-
-  ConstString GetName() const {
-if (compiler_type)
-  return compiler_type.GetTypeName();
-return ConstString();
-  }
-
-  ConstString GetDisplayTypeName() const {
-if (compiler_type)
-  return compiler_type.GetDisplayTypeName();
-return ConstString();
-  }
-
-  void SetType(CompilerType type) {
-compiler_type = type;
-  }
-
-  void SetType(lldb::TypeSP type) {
-compiler_type = type->GetForwardCompilerType();
-  }
-
-  CompilerType GetCompilerType() const { return compiler_type; }
-
-  CompilerType GetPointerType() const { return compiler_type.GetPointerType(); 
}
-
-  CompilerType GetPointeeType() const { return compiler_type.GetPointeeType(); 
}
-
-  CompilerType GetReferenceType() const {
-return compiler_type.GetLValueReferenceType();
-  }
-
-  CompilerType GetTypedefedType() const {
-return compiler_type.GetTypedefedType();
-  }
-
-  CompilerType GetDereferencedType() const {
-return compiler_type.GetNonReferenceType();
-  }
-
-  CompilerType GetUnqualifiedType() const {
-return compiler_type.GetFullyUnqualifiedType();
-  }
-
-  CompilerType GetCanonicalType() const {
-return compiler_type.GetCanonicalType();
-  }
-
-  TypeSystem *GetTypeSystem() const { return compiler_type.GetTypeSystem(); }
-
-protected:
-  CompilerType compiler_type;
-};
-
 // the two classes here are used by the public API as a backend to the SBType
 // and SBT

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

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB356993: Remove the TypePair class (authored by labath, 
committed by ).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D59414?vs=190823&id=192266#toc

Repository:
  rLLDB LLDB

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

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();
   }
   return ConstString();
 }
@@ -

[Lldb-commits] [PATCH] D59495: Fix an out-of-bounds error in RegisterContextDarwin_arm64

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

lgtm


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

https://reviews.llvm.org/D59495



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

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

It would be really nice to get the GDB remote server for ARC so that it vends 
the correct regs to begin with, but if that isn't possible I guess we need to 
do what we are doing here. I would really like to not see more architectures 
have to add code to ProcessGDBRemote.cpp at all.  If that isn't possible this 
solution will need to be used.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55718



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

[Lldb-commits] [PATCH] D59819: Make operator==s consistent between c++ and python APIs

[Pavel Labath via Phabricator via lldb-commits]

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

modify-python-lldb.py had code to insert python equality operators to
some classes. Some of those classes already had c++ equality operators,
and some didn't.

This makes the situation more consistent, by removing all equality
handilng from modify-python-lldb. Instead, I add c++ operators to
classes where they were missing, and expose them in the swig interface
files so that they are available to python too.

The only tricky case was the SBAddress class, which had an operator==
defined as a free function, which is not handled by swig. This function
cannot be removed without breaking ABI, and we cannot add an extra
operator== member, as that would make equality comparisons ambiguous.
For this class, I define a python __eq__ function by hand and have it
delegate to the operator!=, which I have defined as a member function.

This isn't fully NFC, as the semantics of some equality functions in
python changes slightly, but I believe it changes for the better (e.g.,
previously SBBreakpoint.__eq__ would consider two breakpoints with the
same ID as equal, even if they belonged to different targets; now they
are only equal if they belong to the same target).


https://reviews.llvm.org/D59819

Files:
  include/lldb/API/SBAddress.h
  include/lldb/API/SBFileSpec.h
  include/lldb/API/SBWatchpoint.h
  scripts/Python/modify-python-lldb.py
  scripts/interface/SBAddress.i
  scripts/interface/SBBreakpoint.i
  scripts/interface/SBFileSpec.i
  scripts/interface/SBModule.i
  scripts/interface/SBWatchpoint.i
  scripts/lldb.swig
  source/API/SBAddress.cpp
  source/API/SBFileSpec.cpp
  source/API/SBWatchpoint.cpp

Index: source/API/SBWatchpoint.cpp
===
--- source/API/SBWatchpoint.cpp
+++ source/API/SBWatchpoint.cpp
@@ -70,6 +70,20 @@
   return bool(m_opaque_wp.lock());
 }
 
+bool SBWatchpoint::operator==(const SBWatchpoint &rhs) const {
+  LLDB_RECORD_METHOD_CONST(
+  bool, SBWatchpoint, operator==,(const SBWatchpoint &), rhs);
+
+  return GetSP() == rhs.GetSP();
+}
+
+bool SBWatchpoint::operator!=(const SBWatchpoint &rhs) const {
+  LLDB_RECORD_METHOD_CONST(
+  bool, SBWatchpoint, operator!=,(const SBWatchpoint &), rhs);
+
+  return !(*this == rhs);
+}
+
 SBError SBWatchpoint::GetError() {
   LLDB_RECORD_METHOD_NO_ARGS(lldb::SBError, SBWatchpoint, GetError);
 
@@ -303,6 +317,10 @@
   LLDB_REGISTER_METHOD(lldb::watch_id_t, SBWatchpoint, GetID, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBWatchpoint, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBWatchpoint, operator bool, ());
+  LLDB_REGISTER_METHOD_CONST(
+  bool, SBWatchpoint, operator==,(const lldb::SBWatchpoint &));
+  LLDB_REGISTER_METHOD_CONST(
+  bool, SBWatchpoint, operator!=,(const lldb::SBWatchpoint &));
   LLDB_REGISTER_METHOD(lldb::SBError, SBWatchpoint, GetError, ());
   LLDB_REGISTER_METHOD(int32_t, SBWatchpoint, GetHardwareIndex, ());
   LLDB_REGISTER_METHOD(lldb::addr_t, SBWatchpoint, GetWatchAddress, ());
Index: source/API/SBFileSpec.cpp
===
--- source/API/SBFileSpec.cpp
+++ source/API/SBFileSpec.cpp
@@ -62,6 +62,20 @@
   return *this;
 }
 
+bool SBFileSpec::operator==(const SBFileSpec &rhs) const {
+  LLDB_RECORD_METHOD_CONST(bool, SBFileSpec, operator==,(const SBFileSpec &rhs),
+   rhs);
+
+  return ref() == rhs.ref();
+}
+
+bool SBFileSpec::operator!=(const SBFileSpec &rhs) const {
+  LLDB_RECORD_METHOD_CONST(bool, SBFileSpec, operator!=,(const SBFileSpec &rhs),
+   rhs);
+
+  return !(*this == rhs);
+}
+
 bool SBFileSpec::IsValid() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBFileSpec, IsValid);
   return this->operator bool();
@@ -185,6 +199,10 @@
   LLDB_REGISTER_CONSTRUCTOR(SBFileSpec, (const char *, bool));
   LLDB_REGISTER_METHOD(const lldb::SBFileSpec &,
SBFileSpec, operator=,(const lldb::SBFileSpec &));
+  LLDB_REGISTER_METHOD_CONST(bool,
+ SBFileSpec, operator==,(const lldb::SBFileSpec &));
+  LLDB_REGISTER_METHOD_CONST(bool,
+ SBFileSpec, operator!=,(const lldb::SBFileSpec &));
   LLDB_REGISTER_METHOD_CONST(bool, SBFileSpec, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBFileSpec, operator bool, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBFileSpec, Exists, ());
Index: source/API/SBAddress.cpp
===
--- source/API/SBAddress.cpp
+++ source/API/SBAddress.cpp
@@ -69,6 +69,13 @@
   return false;
 }
 
+bool SBAddress::operator!=(const SBAddress &rhs) const {
+  LLDB_RECORD_METHOD_CONST(bool, SBAddress, operator!=,(const SBAddress &),
+   &rhs);
+
+  return !(*this == rhs);
+}
+
 bool SBAddress::IsValid() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBAddress, IsValid

[Lldb-commits] [PATCH] D59819: Make operator==s consistent between c++ and python APIs

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: include/lldb/API/SBAddress.h:34-37
+  // operator== is a free function
+
+  bool operator!=(const SBAddress &rhs) const;
+

A different solution to this could be to still keep the free operator== 
defined, but do not expose it in the header. Then we could define a member 
operator==, without things being ambiguous. The people who build against old 
headers will still find the free operator== in the shared library, and people 
who use the new headers will start using the member operator==.



Comment at: source/API/SBAddress.cpp:66-70
 bool lldb::operator==(const SBAddress &lhs, const SBAddress &rhs) {
   if (lhs.IsValid() && rhs.IsValid())
 return lhs.ref() == rhs.ref();
   return false;
 }

@JDevlieghere: you might be interested to know that this function is not 
intercepted by the api recorder. (This could be another argument for the 
"private free operator==" idea I mention above, as it would mean we don't need 
to add special code to intercept free functions.)


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

https://reviews.llvm.org/D59819



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

[Lldb-commits] [PATCH] D59775: Minidump: Add support for reading/writing strings

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks fine to me, but probably need a LLVM specific person for the final ok


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59775



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

[Lldb-commits] [PATCH] D59584: python 2/3 compat: commands vs subprocess

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356995: python 2/3 compat: commands vs subprocess (authored 
by serge_sans_paille, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59584?vs=191446&id=192279#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59584

Files:
  lldb/trunk/examples/python/delta.py
  lldb/trunk/examples/python/gdbremote.py
  lldb/trunk/examples/python/globals.py
  lldb/trunk/examples/python/memory.py
  lldb/trunk/examples/python/performance.py
  lldb/trunk/examples/python/process_events.py
  lldb/trunk/examples/python/stacks.py
  lldb/trunk/examples/python/types.py
  lldb/trunk/scripts/verify_api.py

Index: lldb/trunk/examples/python/memory.py
===
--- lldb/trunk/examples/python/memory.py
+++ lldb/trunk/examples/python/memory.py
@@ -9,7 +9,6 @@
 #   (lldb) command script import /path/to/cmdtemplate.py
 #--
 
-import commands
 from __future__ import print_function
 
 import platform
@@ -17,6 +16,11 @@
 import re
 import sys
 
+if sys.version_info.major == 2:
+import commands as subprocess
+else:
+import subprocess
+
 try:
 # Just try for LLDB in case PYTHONPATH is already correctly setup
 import lldb
@@ -26,7 +30,7 @@
 platform_system = platform.system()
 if platform_system == 'Darwin':
 # On Darwin, try the currently selected Xcode directory
-xcode_dir = commands.getoutput("xcode-select --print-path")
+xcode_dir = subprocess.getoutput("xcode-select --print-path")
 if xcode_dir:
 lldb_python_dirs.append(
 os.path.realpath(
@@ -53,7 +57,6 @@
 print("error: couldn't locate the 'lldb' module, please set PYTHONPATH correctly")
 sys.exit(1)
 
-import commands
 import optparse
 import shlex
 import string
Index: lldb/trunk/examples/python/types.py
===
--- lldb/trunk/examples/python/types.py
+++ lldb/trunk/examples/python/types.py
@@ -9,7 +9,6 @@
 #   (lldb) command script import /path/to/cmdtemplate.py
 #--
 
-import commands
 from __future__ import print_function
 
 import platform
@@ -18,6 +17,11 @@
 import signal
 import sys
 
+if sys.version_info.major == 2:
+import commands as subprocess
+else:
+import subprocess
+
 try:
 # Just try for LLDB in case PYTHONPATH is already correctly setup
 import lldb
@@ -27,7 +31,7 @@
 platform_system = platform.system()
 if platform_system == 'Darwin':
 # On Darwin, try the currently selected Xcode directory
-xcode_dir = commands.getoutput("xcode-select --print-path")
+xcode_dir = subprocess.getoutput("xcode-select --print-path")
 if xcode_dir:
 lldb_python_dirs.append(
 os.path.realpath(
@@ -54,7 +58,6 @@
 print("error: couldn't locate the 'lldb' module, please set PYTHONPATH correctly")
 sys.exit(1)
 
-import commands
 import optparse
 import shlex
 import time
Index: lldb/trunk/examples/python/process_events.py
===
--- lldb/trunk/examples/python/process_events.py
+++ lldb/trunk/examples/python/process_events.py
@@ -8,7 +8,6 @@
 #   export PYTHONPATH=/Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/Resources/Python
 #--
 
-import commands
 from __future__ import print_function
 
 import optparse
@@ -16,6 +15,11 @@
 import platform
 import sys
 
+if sys.version_info.major == 2:
+import commands as subprocess
+else:
+import subprocess
+
 #--
 # Code that auto imports LLDB
 #--
@@ -28,7 +32,7 @@
 platform_system = platform.system()
 if platform_system == 'Darwin':
 # On Darwin, try the currently selected Xcode directory
-xcode_dir = commands.getoutput("xcode-select --print-path")
+xcode_dir = subprocess.getoutput("xcode-select --print-path")
 if xcode_dir:
 lldb_python_dirs.append(
 os.path.realpath(
Index: lldb/trunk/examples/python/gdbremote.py
===
--- lldb/trunk/examples/python/gdbremote.py
+++ lldb/trunk/examples/python/gdbremote.py
@@ -17,7 +17,7 @@
 #--
 
 import binascii
-import commands
+import subprocess
 import json
 import math
 import optparse
Index: lldb/trunk/examples/python/performance.py

[Lldb-commits] [PATCH] D59826: [expression] Explicitly build the lookup table of any TagDecl's context

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

There is a lookup failure within the
ASTImporter during the expression evaluation. The clang::ASTImporter cannot
lookup previously created declarations thus it creates duplicated and redundant
declarations. These duplicated declarations then later can cause different
assertions.

More specifically, the problem occurs in the presence of a LinkageSpecDecl.
Here are the steps to reproduce:

- Compile a simple test binary (a.out)
- Launch LLDB with a.out
- set a breakpoint
- run
- expr -l objc++ -- @import Darwin; 3
- expr *fopen("/dev/zero", "w")

When we evaluate the `fopen` expression then we import the `__sFILE`
struct several times to the same ASTContext.
(To see these we have to attach to the original lldb process with
another debugger and add breakpoints to
ASTNodeImporter::VisitRecordDecl.)
After the first import we have this in the AST:

  TranslationUnitDecl
  `-LinkageSpecDecl 0x7ff9eab97618 <:76:1, line:78:1> 
line:76:8 C
|-CXXRecordDecl 0x7ff9eab97668
  
 col:16  struct __sFILE definition
| `-DefinitionData pass_in_registers aggregate standard_layout
  trivially_copyable pod trivial literal
|   |-DefaultConstructor exists trivial needs_implicit
|   |-CopyConstructor simple trivial has_const_param needs_implicit
  implicit_has_const_param
|   |-MoveConstructor exists simple trivial needs_implicit
|   |-CopyAssignment trivial has_const_param needs_implicit
  implicit_has_const_param
|   |-MoveAssignment exists simple trivial needs_implicit
|   `-Destructor simple irrelevant trivial needs_implicit
`-TypedefDecl 0x7ff9eab977f8  col:3 FILE 'struct
  __sFILE':'__sFILE'
  `-ElaboratedType 0x7ff9eab977a0 'struct __sFILE' sugar
`-RecordType 0x7ff9eab97710 '__sFILE'
  `-CXXRecord 0x7ff9eab97668 '__sFILE'

During the second import the lookup fails thus we end up with a
duplicated CXXRecordDecl of `__sFILE` in the AST:

  TranslationUnitDecl
  |-LinkageSpecDecl 0x7ff9eab97618 <:76:1, line:78:1> 
line:76:8 C
  | |-CXXRecordDecl 0x7ff9eab97668
  
 col:16  struct __sFILE definition
  | | `-DefinitionData pass_in_registers aggregate standard_layout
  trivially_copyable pod trivial literal
  | |   |-DefaultConstructor exists trivial needs_implicit
  | |   |-CopyConstructor simple trivial has_const_param needs_implicit
  implicit_has_const_param
  | |   |-MoveConstructor exists simple trivial needs_implicit
  | |   |-CopyAssignment trivial has_const_param needs_implicit
  implicit_has_const_param
  | |   |-MoveAssignment exists simple trivial needs_implicit
  | |   `-Destructor simple irrelevant trivial needs_implicit
  | `-TypedefDecl 0x7ff9eab977f8  col:3 FILE 'struct
  __sFILE':'__sFILE'
  |   `-ElaboratedType 0x7ff9eab977a0 'struct __sFILE' sugar
  | `-RecordType 0x7ff9eab97710 '__sFILE'
  |   `-CXXRecord 0x7ff9eab97668 '__sFILE'
  `-LinkageSpecDecl 0x7ff9eab97888 <:76:1, line:78:1> 
line:76:8 C
`-CXXRecordDecl 0x7ff9eab978d8
  
 col:16  struct __sFILE definition
  `-DefinitionData pass_in_registers aggregate standard_layout
  trivially_copyable pod trivial literal
|-DefaultConstructor exists trivial needs_implicit
|-CopyConstructor simple trivial has_const_param needs_implicit
  implicit_has_const_param
|-MoveConstructor exists simple trivial needs_implicit
|-CopyAssignment trivial has_const_param needs_implicit
  implicit_has_const_param
|-MoveAssignment exists simple trivial needs_implicit
`-Destructor simple irrelevant trivial needs_implicit

The reason behind this is the following:
ASTImporter uses DeclContext::localUncachedLookup on a redecl context,
because we must not perform a lookup in a transparent context (there
is an assertion for that in DeclContext).
The redecl context of the `__sFILE` CXXRecordDecl is the
TranslationUnitDecl, while the normal DC is the LinkageSpecDecl (which
is a transparent context).
localUncachedLookup searches via the lookup table (lookupPtr) of the
given DeclContext, but only if that DC does not have external lexical
storage!
However, in the LLDB case it does have, so we end up in the Slow case
of the localUncachedLookup:

  // Slow case: grovel through the declarations in our chain looking for
  // matches.
  // FIXME: If we have lazy external declarations, this will not find them!
  // FIXME: Should we CollectAllContexts and walk them all here?
  for (Decl *D = FirstDecl; D; D = D->getNextDeclInContext()) {
if (auto *ND = dyn_cast(D))
  if (ND->getDeclName() == Name)
Results.push_back(ND);
  }

Since the DC is the redecl DC (TranslationUnitDecl) and not the normal
DC (LinkageSpecDecl), we will not find the CXXRecordDecl with the name
`__sFILE`.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59826



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

[Lldb-commits] [PATCH] D59826: [expression] Explicitly build the lookup table of any TagDecl's context

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Might be good for Jim Ingham to look at this as well?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59826



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

[Lldb-commits] [lldb] r357006 - [ScriptInterpreterPython] Try to make the sanitizer bot green again.

[Davide Italiano via lldb-commits]

Author: davide
Date: Tue Mar 26 09:43:58 2019
New Revision: 357006

URL: http://llvm.org/viewvc/llvm-project?rev=357006&view=rev
Log:
[ScriptInterpreterPython] Try to make the sanitizer bot green again.

Removing a use-after-free error.

Modified:

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Modified: 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=357006&r1=357005&r2=357006&view=diff
==
--- 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
Tue Mar 26 09:43:58 2019
@@ -322,7 +322,7 @@ private:
 // priorities in the path, overriding PYTHONHOME and causing
 // problems/incompatibilities. In order to avoid confusion, always hardcode
 // the PythonHome to be right, as it's not going to change.
-char path[] = "/System/Library/Frameworks/Python.framework/Versions/2.7";
+static char path[] = 
"/System/Library/Frameworks/Python.framework/Versions/2.7";
 Py_SetPythonHome(path);
 #endif
 #endif


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

[Lldb-commits] [PATCH] D59828: Add lldb-vscode as a dependency of lldb tests.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe created this revision.
jgorbe added a reviewer: clayborg.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.

In the current state, 'ninja check-lldb' runs the lldb-vscode tests, but it
won't rebuild lldb-vscode if any of its sources has changed. This is very
confusing when you fix something and the tests keep failing, or vice versa.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59828

Files:
  lldb/CMakeLists.txt


Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -91,6 +91,10 @@
 list(APPEND LLDB_TEST_DEPS lldb-mi)
   endif()
 
+  if(TARGET lldb-vscode)
+list(APPEND LLDB_TEST_DEPS lldb-vscode)
+  endif()
+
   if(TARGET lldb-instr)
 list(APPEND LLDB_TEST_DEPS lldb-instr)
   endif()


Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -91,6 +91,10 @@
 list(APPEND LLDB_TEST_DEPS lldb-mi)
   endif()
 
+  if(TARGET lldb-vscode)
+list(APPEND LLDB_TEST_DEPS lldb-vscode)
+  endif()
+
   if(TARGET lldb-instr)
 list(APPEND LLDB_TEST_DEPS lldb-instr)
   endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r357016 - Add lldb-vscode as a dependency of lldb tests.

[Jorge Gorbe Moya via lldb-commits]

Author: jgorbe
Date: Tue Mar 26 11:36:44 2019
New Revision: 357016

URL: http://llvm.org/viewvc/llvm-project?rev=357016&view=rev
Log:
Add lldb-vscode as a dependency of lldb tests.

Summary:
In the current state, 'ninja check-lldb' runs the lldb-vscode tests, but it
won't rebuild lldb-vscode if any of its sources has changed. This is very
confusing when you fix something and the tests keep failing, or vice versa.

Reviewers: clayborg

Subscribers: mgorny, lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/CMakeLists.txt

Modified: lldb/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=357016&r1=357015&r2=357016&view=diff
==
--- lldb/trunk/CMakeLists.txt (original)
+++ lldb/trunk/CMakeLists.txt Tue Mar 26 11:36:44 2019
@@ -91,6 +91,10 @@ if(LLDB_INCLUDE_TESTS)
 list(APPEND LLDB_TEST_DEPS lldb-mi)
   endif()
 
+  if(TARGET lldb-vscode)
+list(APPEND LLDB_TEST_DEPS lldb-vscode)
+  endif()
+
   if(TARGET lldb-instr)
 list(APPEND LLDB_TEST_DEPS lldb-instr)
   endif()


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

[Lldb-commits] [PATCH] D59828: Add lldb-vscode as a dependency of lldb tests.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357016: Add lldb-vscode as a dependency of lldb tests. 
(authored by jgorbe, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59828?vs=192296&id=192306#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59828

Files:
  lldb/trunk/CMakeLists.txt


Index: lldb/trunk/CMakeLists.txt
===
--- lldb/trunk/CMakeLists.txt
+++ lldb/trunk/CMakeLists.txt
@@ -91,6 +91,10 @@
 list(APPEND LLDB_TEST_DEPS lldb-mi)
   endif()
 
+  if(TARGET lldb-vscode)
+list(APPEND LLDB_TEST_DEPS lldb-vscode)
+  endif()
+
   if(TARGET lldb-instr)
 list(APPEND LLDB_TEST_DEPS lldb-instr)
   endif()


Index: lldb/trunk/CMakeLists.txt
===
--- lldb/trunk/CMakeLists.txt
+++ lldb/trunk/CMakeLists.txt
@@ -91,6 +91,10 @@
 list(APPEND LLDB_TEST_DEPS lldb-mi)
   endif()
 
+  if(TARGET lldb-vscode)
+list(APPEND LLDB_TEST_DEPS lldb-vscode)
+  endif()
+
   if(TARGET lldb-instr)
 list(APPEND LLDB_TEST_DEPS lldb-instr)
   endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59831: [CMake] macOS: Find DebugSymbols.framework inside the SDK

[Jordan Rose via Phabricator via lldb-commits]

jordan_rose created this revision.
jordan_rose added reviewers: jingham, davide, sgraenitz.
jordan_rose added a project: LLDB.
Herald added a subscriber: mgorny.

It's always going to be at / on disk, but that's not what we should be linking 
against! This won't actually change anything in practice, but it's "more 
correct", and may be necessary if Apple ever decides to move the implementation 
of DebugSymbols.framework to another library.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59831

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -379,7 +379,7 @@
   if(NOT IOS)
 find_library(CARBON_LIBRARY Carbon)
 find_library(CORE_SERVICES_LIBRARY CoreServices)
-find_library(DEBUG_SYMBOLS_LIBRARY DebugSymbols PATHS 
"/System/Library/PrivateFrameworks")
+find_library(DEBUG_SYMBOLS_LIBRARY DebugSymbols PATHS 
"${CMAKE_OSX_SYSROOT}/System/Library/PrivateFrameworks")
   endif()
   find_library(FOUNDATION_LIBRARY Foundation)
   find_library(CORE_FOUNDATION_LIBRARY CoreFoundation)


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -379,7 +379,7 @@
   if(NOT IOS)
 find_library(CARBON_LIBRARY Carbon)
 find_library(CORE_SERVICES_LIBRARY CoreServices)
-find_library(DEBUG_SYMBOLS_LIBRARY DebugSymbols PATHS "/System/Library/PrivateFrameworks")
+find_library(DEBUG_SYMBOLS_LIBRARY DebugSymbols PATHS "${CMAKE_OSX_SYSROOT}/System/Library/PrivateFrameworks")
   endif()
   find_library(FOUNDATION_LIBRARY Foundation)
   find_library(CORE_FOUNDATION_LIBRARY CoreFoundation)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

[Alex Langford via Phabricator via lldb-commits]

xiaobai added inline comments.



Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:42
 /// This will compute the clang resource directory assuming that clang was
-/// installed with the same prefix as lldb.
+/// installed with the same prefix as lldb. Note: the verify paramter is
+/// currently only used for testing.

jingham wrote:
> spelling: parameter
> 
> But also, you should say something more informative, like:
> 
> If verify is true, the first candidate resource directory will be returned.  
> This mode is only used for testing
I'll adjust this before I commit. Thanks for pointing that out.


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

https://reviews.llvm.org/D59708



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

[Lldb-commits] [PATCH] D59495: Fix an out-of-bounds error in RegisterContextDarwin_arm64

[Jason Molenda via Phabricator via lldb-commits]

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

Sorry for not weighing in earlier, yes this is good.


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

https://reviews.llvm.org/D59495



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

[Lldb-commits] [PATCH] D59831: [CMake] macOS: Find DebugSymbols.framework inside the SDK

[Davide Italiano via Phabricator via lldb-commits]

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

LGTM


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59831



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

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D55718#1442965 , @clayborg wrote:

> It would be really nice to get the GDB remote server for ARC so that it vends 
> the correct regs to begin with, but if that isn't possible I guess we need to 
> do what we are doing here. I would really like to not see more architectures 
> have to add code to ProcessGDBRemote.cpp at all.  If that isn't possible this 
> solution will need to be used.


Unrelated to this patchset, but just last week I was working on something and 
thinking about how our system of "the RSP stub is the canonical source of 
register knowledge" is probably not the best architecture.  IMO the remote stub 
should teach us (1) the name of each register, (2) the numbers it will use for 
each register, (3) the size in bits of each register, and (4) the offset into 
the g/G packet of that register.

Everything else about registers should come from lldb in either 
architecture-specific definitions (rax has 64 bits, and it has a slice register 
eax of 32 bits, an ax of 16 bits etc; it is printed as Uint64/32/16), or ABI 
definitions (rdi is arg1 in the posix x86_64 ABI; rsp is eh_frame regnum 7, 
dwarf regnum 7).  Coordination between the RSP stub and lldb should be done by 
register name, confirmed by register size (the remote stub says it has a 64-bit 
r0 and I only know about a 32-bit r0, we have a problem).

Back in 2015 I added some code in r247121 that would fill in eh_frame and dwarf 
register numbers from the ABI plugin if the remote stub didn't provide them in 
ProcessGDBRemote's AugmentRegisterInfoViaABI.  It might be better for ARC to 
hardcode these generic register additions some place like that.  The change in 
247121 was a step in the direction of having lldb be the canonical source of 
information about registers, but only doing the easy first bit.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55718



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

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D55718#1443415 , @jasonmolenda 
wrote:

> In D55718#1442965 , @clayborg wrote:
>
> > It would be really nice to get the GDB remote server for ARC so that it 
> > vends the correct regs to begin with, but if that isn't possible I guess we 
> > need to do what we are doing here. I would really like to not see more 
> > architectures have to add code to ProcessGDBRemote.cpp at all.  If that 
> > isn't possible this solution will need to be used.
>
>
> Unrelated to this patchset, but just last week I was working on something and 
> thinking about how our system of "the RSP stub is the canonical source of 
> register knowledge" is probably not the best architecture.  IMO the remote 
> stub should teach us (1) the name of each register, (2) the numbers it will 
> use for each register, (3) the size in bits of each register, and (4) the 
> offset into the g/G packet of that register.
>
> Everything else about registers should come from lldb in either 
> architecture-specific definitions (rax has 64 bits, and it has a slice 
> register eax of 32 bits, an ax of 16 bits etc; it is printed as 
> Uint64/32/16), or ABI definitions (rdi is arg1 in the posix x86_64 ABI; rsp 
> is eh_frame regnum 7, dwarf regnum 7).  Coordination between the RSP stub and 
> lldb should be done by register name, confirmed by register size (the remote 
> stub says it has a 64-bit r0 and I only know about a 32-bit r0, we have a 
> problem).
>
> Back in 2015 I added some code in r247121 that would fill in eh_frame and 
> dwarf register numbers from the ABI plugin if the remote stub didn't provide 
> them in ProcessGDBRemote's AugmentRegisterInfoViaABI.  It might be better for 
> ARC to hardcode these generic register additions some place like that.  The 
> change in 247121 was a step in the direction of having lldb be the canonical 
> source of information about registers, but only doing the easy first bit.


+1000

This sounds really great to me.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55718



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

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

As long as the numbers _can_ still come from the GDB server, I am all for that. 
If they are not supplied, then we use arch defaults. That allows new 
system/arch bringups where the info might be changing often, and also allows 
the info to be locked down. The architecture plug-ins are a great place for 
this. We should also move the breakpoint opcode logic into the architecture 
plug-ins.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55718



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

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D55718#1443487 , @clayborg wrote:

> As long as the numbers _can_ still come from the GDB server, I am all for 
> that. If they are not supplied, then we use arch defaults. That allows new 
> system/arch bringups where the info might be changing often, and also allows 
> the info to be locked down. The architecture plug-ins are a great place for 
> this. We should also move the breakpoint opcode logic into the architecture 
> plug-ins.


I have no opinion about the RSP stub overriding lldb's definitions - fine by 
me.  I'm mostly interested in making lldb work better with rando RSP 
implementations in the wild which often give us very little information about 
the registers.

Moving the breakpoint opcode into lldb is interesting, would you want to move 
away from the Z0/z0 packets for setting/removing breakpoints then?  The 
set-breakpoint packet takes an address and iirc an instruction size.  So on 
armv7 processors, we know whether we're replacing a 2-byte or 4-byte 
instruction.  I don't think we want to move the logic of breakpoint setting & 
clearing into lldb and use generic read/write memory - the RSP stub may know 
how to clear instruction caches when we're modifying instruction data, for 
instance.  So we'd make a variation of the z0 packet which takes an address and 
a byte sequence to put at that address?  tbh it seems like the RSP stub is the 
correct place for this knowledge, even though we have to provide the hack of 
"use the 2-byte breakpoint instruction" / "use the 4-byte breakpoint 
instruction" on targets like armv7 because lldb has the knowledge of the 
underlying instructions.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55718



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

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D55718#1443487 , @clayborg wrote:

> As long as the numbers _can_ still come from the GDB server, I am all for 
> that. If they are not supplied, then we use arch defaults. That allows new 
> system/arch bringups where the info might be changing often, and also allows 
> the info to be locked down. The architecture plug-ins are a great place for 
> this. We should also move the breakpoint opcode logic into the architecture 
> plug-ins.


Which part of the information that Jason mentioned do you expect to change 
during bringup?

- eh/debug_frame numbers aren't something that the stub can unilaterally 
change, as it needs to match what the compiler thinks. Since we're not going to 
get the compiler to ask our stub about the eh_frame numbers, I don't see the 
problem in hardcoding that info. TBE, the best way would be no not even 
hardcode that info but ask llvm about these things. That way if somebody 
develops the whole platform (compiler, abi, and everything), and uses llvm 
technology everywhere, he would just need to change the numbers once and 
recompile -- lldb, lldb-server and clang would automatically pick that up
- the ABI is also something that needs to match with the compiler, and is 
something that is largely irrelevant for the operation of the rsp stub
- how to print a register is also completely irrelevant for the stub, and (IMO) 
also largely unimportant during early bringup stages

The things I can see changing frequently while developing a stub for a new 
platform are the register numbers and g packet offsets, but that's already 
something that Jason included in the list of things that the stub should 
provide.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55718



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

[Lldb-commits] [lldb] r357030 - [ExpressionParser] Add swift-lldb case for finding clang resource dir

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Tue Mar 26 14:00:42 2019
New Revision: 357030

URL: http://llvm.org/viewvc/llvm-project?rev=357030&view=rev
Log:
[ExpressionParser] Add swift-lldb case for finding clang resource dir

Summary:
I'm adding this to reduce the difference between swift-lldb and
llvm.org's lldb.

Reviewers: aprantl, davide, compnerd, JDevlieghere, jingham

Subscribers: lldb-commits

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

Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
lldb/trunk/unittests/Expression/ClangParserTest.cpp

Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangHost.cpp?rev=357030&r1=357029&r2=357030&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangHost.cpp (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangHost.cpp Tue Mar 26 
14:00:42 2019
@@ -16,6 +16,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 
+#include "lldb/Host/Config.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/FileSpec.h"
@@ -40,25 +41,43 @@ static bool VerifyClangPath(const llvm::
 /// This will compute the clang resource directory assuming that clang was
 /// installed with the same prefix as lldb.
 ///
+/// If verify is true, the first candidate resource directory will be returned.
+/// This mode is only used for testing.
+///
 static bool DefaultComputeClangResourceDirectory(FileSpec &lldb_shlib_spec,
- FileSpec &file_spec, bool verify) {
+ FileSpec &file_spec,
+ bool verify) {
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
   std::string raw_path = lldb_shlib_spec.GetPath();
   llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
 
-  llvm::SmallString<256> clang_dir(parent_dir);
-  llvm::SmallString<32> relative_path;
-  llvm::sys::path::append(relative_path,
-  llvm::Twine("lib") + CLANG_LIBDIR_SUFFIX, "clang",
-  CLANG_VERSION_STRING);
-
-  llvm::sys::path::append(clang_dir, relative_path);
-  if (!verify || VerifyClangPath(clang_dir)) {
-file_spec.GetDirectory().SetString(clang_dir);
-FileSystem::Instance().Resolve(file_spec);
-return true;
+  static const llvm::StringRef kResourceDirSuffixes[] = {
+  // LLVM.org's build of LLDB uses the clang resource directory placed
+  // in $install_dir/lib{,64}/clang/$clang_version.
+  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING,
+  // swift-lldb uses the clang resource directory copied from swift, which
+  // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
+  // it there, so we use LLDB_LIBDIR_SUFFIX.
+  "lib" LLDB_LIBDIR_SUFFIX "/lldb/clang",
+  };
+
+  for (const auto &Suffix : kResourceDirSuffixes) {
+llvm::SmallString<256> clang_dir(parent_dir);
+llvm::SmallString<32> relative_path(Suffix);
+llvm::sys::path::native(relative_path);
+llvm::sys::path::append(clang_dir, relative_path);
+if (!verify || VerifyClangPath(clang_dir)) {
+  if (log)
+log->Printf("DefaultComputeClangResourceDir: Setting ClangResourceDir "
+"to \"%s\", verify = %s",
+clang_dir.str().str().c_str(), verify ? "true" : "false");
+  file_spec.GetDirectory().SetString(clang_dir);
+  FileSystem::Instance().Resolve(file_spec);
+  return true;
+}
   }
 
-  return HostInfo::ComputePathRelativeToLibrary(file_spec, relative_path);
+  return false;
 }
 
 bool lldb_private::ComputeClangResourceDirectory(FileSpec &lldb_shlib_spec,

Modified: lldb/trunk/unittests/Expression/ClangParserTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Expression/ClangParserTest.cpp?rev=357030&r1=357029&r2=357030&view=diff
==
--- lldb/trunk/unittests/Expression/ClangParserTest.cpp (original)
+++ lldb/trunk/unittests/Expression/ClangParserTest.cpp Tue Mar 26 14:00:42 2019
@@ -50,9 +50,8 @@ TEST_F(ClangHostTest, ComputeClangResour
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
   // The path doesn't really exist, so setting verify to true should make
-  // ComputeClangResourceDir to not give you path_to_clang_dir.
-  EXPECT_NE(ComputeClangResourceDir(path_to_liblldb, true),
-ComputeClangResourceDir(path_to_liblldb));
+  // ComputeClangResourceDir not give you path_to_clang_dir.
+  EXPECT_NE(ComputeClangResourceDir(path_to_liblldb, true), path_to_clang_dir);
 }
 
 #if defined(__APPLE__)


___
lldb-commits mailing list
lldb-commi

[Lldb-commits] [PATCH] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB357030: [ExpressionParser] Add swift-lldb case for 
finding clang resource dir (authored by xiaobai, committed by ).
Herald added a subscriber: teemperor.
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D59708?vs=192204&id=192352#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59708

Files:
  source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  unittests/Expression/ClangParserTest.cpp


Index: unittests/Expression/ClangParserTest.cpp
===
--- unittests/Expression/ClangParserTest.cpp
+++ unittests/Expression/ClangParserTest.cpp
@@ -50,9 +50,8 @@
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
   // The path doesn't really exist, so setting verify to true should make
-  // ComputeClangResourceDir to not give you path_to_clang_dir.
-  EXPECT_NE(ComputeClangResourceDir(path_to_liblldb, true),
-ComputeClangResourceDir(path_to_liblldb));
+  // ComputeClangResourceDir not give you path_to_clang_dir.
+  EXPECT_NE(ComputeClangResourceDir(path_to_liblldb, true), path_to_clang_dir);
 }
 
 #if defined(__APPLE__)
Index: source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 
+#include "lldb/Host/Config.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/FileSpec.h"
@@ -40,25 +41,43 @@
 /// This will compute the clang resource directory assuming that clang was
 /// installed with the same prefix as lldb.
 ///
+/// If verify is true, the first candidate resource directory will be returned.
+/// This mode is only used for testing.
+///
 static bool DefaultComputeClangResourceDirectory(FileSpec &lldb_shlib_spec,
- FileSpec &file_spec, bool verify) {
+ FileSpec &file_spec,
+ bool verify) {
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
   std::string raw_path = lldb_shlib_spec.GetPath();
   llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
 
-  llvm::SmallString<256> clang_dir(parent_dir);
-  llvm::SmallString<32> relative_path;
-  llvm::sys::path::append(relative_path,
-  llvm::Twine("lib") + CLANG_LIBDIR_SUFFIX, "clang",
-  CLANG_VERSION_STRING);
-
-  llvm::sys::path::append(clang_dir, relative_path);
-  if (!verify || VerifyClangPath(clang_dir)) {
-file_spec.GetDirectory().SetString(clang_dir);
-FileSystem::Instance().Resolve(file_spec);
-return true;
+  static const llvm::StringRef kResourceDirSuffixes[] = {
+  // LLVM.org's build of LLDB uses the clang resource directory placed
+  // in $install_dir/lib{,64}/clang/$clang_version.
+  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING,
+  // swift-lldb uses the clang resource directory copied from swift, which
+  // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
+  // it there, so we use LLDB_LIBDIR_SUFFIX.
+  "lib" LLDB_LIBDIR_SUFFIX "/lldb/clang",
+  };
+
+  for (const auto &Suffix : kResourceDirSuffixes) {
+llvm::SmallString<256> clang_dir(parent_dir);
+llvm::SmallString<32> relative_path(Suffix);
+llvm::sys::path::native(relative_path);
+llvm::sys::path::append(clang_dir, relative_path);
+if (!verify || VerifyClangPath(clang_dir)) {
+  if (log)
+log->Printf("DefaultComputeClangResourceDir: Setting ClangResourceDir "
+"to \"%s\", verify = %s",
+clang_dir.str().str().c_str(), verify ? "true" : "false");
+  file_spec.GetDirectory().SetString(clang_dir);
+  FileSystem::Instance().Resolve(file_spec);
+  return true;
+}
   }
 
-  return HostInfo::ComputePathRelativeToLibrary(file_spec, relative_path);
+  return false;
 }
 
 bool lldb_private::ComputeClangResourceDirectory(FileSpec &lldb_shlib_spec,


Index: unittests/Expression/ClangParserTest.cpp
===
--- unittests/Expression/ClangParserTest.cpp
+++ unittests/Expression/ClangParserTest.cpp
@@ -50,9 +50,8 @@
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
   // The path doesn't really exist, so setting verify to true should make
-  // ComputeClangResourceDir to not give you path_to_clang_dir.
-  EXPECT_NE(ComputeClangResourceDir(path_to_liblldb, true),
-ComputeClangResourceDir(path_to_liblldb));
+  // ComputeClangResourceDir not g

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: aprantl, friss, teemperor.
Herald added a subscriber: jdoerfert.

https://reviews.llvm.org/D59845 added a fix for the `IsStructuralMatch(...)` 
specialization for `EnumDecl` this test should pass once this fix is committed.


https://reviews.llvm.org/D59847

Files:
  packages/Python/lldbsuite/test/expression_command/vector_of_enums/Makefile
  
packages/Python/lldbsuite/test/expression_command/vector_of_enums/TestVectorOfEnums.py
  packages/Python/lldbsuite/test/expression_command/vector_of_enums/main.cpp


Index: 
packages/Python/lldbsuite/test/expression_command/vector_of_enums/main.cpp
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/vector_of_enums/main.cpp
@@ -0,0 +1,14 @@
+#include 
+
+enum E {
+a,
+b,
+c,
+d
+} ;
+
+int main() {
+  std::vector v = {E::a, E::b, E::c};
+
+  return v.size(); // break here
+}
Index: 
packages/Python/lldbsuite/test/expression_command/vector_of_enums/TestVectorOfEnums.py
===
--- /dev/null
+++ 
packages/Python/lldbsuite/test/expression_command/vector_of_enums/TestVectorOfEnums.py
@@ -0,0 +1,28 @@
+"""
+Test Expression Parser regression test to ensure that we handle enums
+correctly, in this case specifically std::vector of enums.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestVectorOfEnums(TestBase):
+
+  mydir = TestBase.compute_mydir(__file__)
+
+  def test_vector_of_enums(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+self.expect("expr v", substrs=[
+ 'size=3',
+ '[0] = a',
+ '[1] = b',
+ '[2] = c',
+ '}'
+])
Index: 
packages/Python/lldbsuite/test/expression_command/vector_of_enums/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/vector_of_enums/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules


Index: packages/Python/lldbsuite/test/expression_command/vector_of_enums/main.cpp
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/vector_of_enums/main.cpp
@@ -0,0 +1,14 @@
+#include 
+
+enum E {
+a,
+b,
+c,
+d
+} ;
+
+int main() {
+  std::vector v = {E::a, E::b, E::c};
+
+  return v.size(); // break here
+}
Index: packages/Python/lldbsuite/test/expression_command/vector_of_enums/TestVectorOfEnums.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/vector_of_enums/TestVectorOfEnums.py
@@ -0,0 +1,28 @@
+"""
+Test Expression Parser regression test to ensure that we handle enums
+correctly, in this case specifically std::vector of enums.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestVectorOfEnums(TestBase):
+
+  mydir = TestBase.compute_mydir(__file__)
+
+  def test_vector_of_enums(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+self.expect("expr v", substrs=[
+ 'size=3',
+ '[0] = a',
+ '[1] = b',
+ '[2] = c',
+ '}'
+])
Index: packages/Python/lldbsuite/test/expression_command/vector_of_enums/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/vector_of_enums/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59849: [lldb-vscode] Add logic to handle EOF when reading from lldb-vscode stdout.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe created this revision.
jgorbe added reviewers: clayborg, zturner.
Herald added a project: LLDB.

This change prevents the lldb-vscode test harness from hanging up waiting for
new messages when the lldb-vscode subprocess crashes.

Now, when an EOF from the subprocess pipe is detected we enqueue a `None` packet
in the received packets list. Then, during the message processing loop, we can
use this `None` packet to tell apart the case where lldb-vscode has terminated
unexpectedly from the normal situation where no pending messages means blocking
and waiting for more data.

I believe this should be enough to fix the issues with these tests hanging on
multiple platforms. Once this lands, I'll prepare and test a separate change
removing the @skipIfLinux annotations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59849

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -52,11 +52,11 @@
 
 def read_packet(f, verbose=False, trace_file=None):
 '''Decode a JSON packet that starts with the content length and is
-   followed by the JSON bytes from a file 'f'
+   followed by the JSON bytes from a file 'f'. Returns None on EOF.
 '''
 line = f.readline().decode("utf-8")
 if len(line) == 0:
-return None
+return None  # EOF.
 
 # Watch for line that starts with the prefix
 prefix = 'Content-Length: '
@@ -91,10 +91,10 @@
 done = False
 while not done:
 packet = read_packet(vs_comm.recv, trace_file=vs_comm.trace_file)
-if packet:
-done = not vs_comm.handle_recv_packet(packet)
-else:
-done = True
+# `packet` will be `None` on EOF. We want to pass it down to
+# handle_recv_packet anyway so the main thread can handle unexpected
+# termination of lldb-vscode and stop waiting for new packets.
+done = not vs_comm.handle_recv_packet(packet)
 
 
 class DebugCommunication(object):
@@ -146,6 +146,12 @@
 self.output_condition.release()
 return output
 
+def enqueue_recv_packet(self, packet):
+self.recv_condition.acquire()
+self.recv_packets.append(packet)
+self.recv_condition.notify()
+self.recv_condition.release()
+
 def handle_recv_packet(self, packet):
 '''Called by the read thread that is waiting for all incoming packets
to store the incoming packet in "self.recv_packets" in a thread safe
@@ -153,6 +159,11 @@
indicate a new packet is available. Returns True if the caller
should keep calling this function for more packets.
 '''
+# If EOF, notify the read thread by enqueing a None.
+if not packet:
+self.enqueue_recv_packet(None)
+return False
+
 # Check the packet to see if is an event packet
 keepGoing = True
 packet_type = packet['type']
@@ -191,10 +202,7 @@
 elif packet_type == 'response':
 if packet['command'] == 'disconnect':
 keepGoing = False
-self.recv_condition.acquire()
-self.recv_packets.append(packet)
-self.recv_condition.notify()
-self.recv_condition.release()
+self.enqueue_recv_packet(packet)
 return keepGoing
 
 def send_packet(self, command_dict, set_sequence=True):
@@ -222,27 +230,33 @@
function will wait for the packet to arrive and return it when
it does.'''
 while True:
-self.recv_condition.acquire()
-packet = None
-while True:
-for (i, curr_packet) in enumerate(self.recv_packets):
-packet_type = curr_packet['type']
-if filter_type is None or packet_type in filter_type:
-if (filter_event is None or
-(packet_type == 'event' and
- curr_packet['event'] in filter_event)):
-packet = self.recv_packets.pop(i)
-break
-if packet:
-break
-# Sleep until packet is received
-len_before = len(self.recv_packets)
-self.recv_condition.wait(timeout)
-len_after = len(self.recv_packets)
-if len_before == len_after:
-return None  # Timed out
-self.recv_condition.release()
-return packet
+try:
+self.recv_condition.acquire()
+packet = None
+while True:
+for (i, curr_packet) in enumerate(self.recv_packets):
+if no

[Lldb-commits] [lldb] r357034 - [Python] Remove dynamic indirection

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Mar 26 14:57:02 2019
New Revision: 357034

URL: http://llvm.org/viewvc/llvm-project?rev=357034&view=rev
Log:
[Python] Remove dynamic indirection

Now that the Python plugin relies on the SWIG symbols, we no longer need
to dynamically resolve these functions.

Modified:
lldb/trunk/source/API/SystemInitializerFull.cpp
lldb/trunk/source/API/SystemInitializerFull.h

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

Modified: lldb/trunk/source/API/SystemInitializerFull.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SystemInitializerFull.cpp?rev=357034&r1=357033&r2=357034&view=diff
==
--- lldb/trunk/source/API/SystemInitializerFull.cpp (original)
+++ lldb/trunk/source/API/SystemInitializerFull.cpp Tue Mar 26 14:57:02 2019
@@ -142,7 +142,6 @@ llvm::Error SystemInitializerFull::Initi
 #endif
 
 #if !defined(LLDB_DISABLE_PYTHON)
-  ScriptInterpreterPython::InitializeSWIG();
   ScriptInterpreterPython::Initialize();
 #endif
 

Modified: lldb/trunk/source/API/SystemInitializerFull.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SystemInitializerFull.h?rev=357034&r1=357033&r2=357034&view=diff
==
--- lldb/trunk/source/API/SystemInitializerFull.h (original)
+++ lldb/trunk/source/API/SystemInitializerFull.h Tue Mar 26 14:57:02 2019
@@ -27,9 +27,6 @@ public:
 
   llvm::Error Initialize() override;
   void Terminate() override;
-
-private:
-  void InitializeSWIG();
 };
 
 } // namespace lldb_private

Modified: 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=357034&r1=357033&r2=357034&view=diff
==
--- 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
Tue Mar 26 14:57:02 2019
@@ -71,10 +71,9 @@ extern "C" void init_lldb(void);
 #define LLDBSwigPyInit init_lldb
 #endif
 
-// these are the Pythonic implementations of the required callbacks these are
-// scripting-language specific, which is why they belong here we still need to
-// use function pointers to them instead of relying on linkage-time resolution
-// because the SWIG stuff and this file get built at different times
+// These prototypes are the Pythonic implementations of the required callbacks.
+// Although these are scripting-language specific, their definition depends on
+// the public API.
 extern "C" bool LLDBSwigPythonBreakpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP &sb_frame,
@@ -194,67 +193,6 @@ LLDBSWIGPython_GetDynamicSetting(void *m
 
 #endif
 
-ScriptInterpreterPython::SWIGInitCallback
-ScriptInterpreterPython::g_swig_init_callback = nullptr;
-ScriptInterpreterPython::SWIGBreakpointCallbackFunction
-ScriptInterpreterPython::g_swig_breakpoint_callback = nullptr;
-ScriptInterpreterPython::SWIGWatchpointCallbackFunction
-ScriptInterpreterPython::g_swig_watchpoint_callback = nullptr;
-ScriptInterpreterPython::SWIGPythonTypeScriptCallbackFunction
-ScriptInterpreterPython::g_swig_typescript_callback = nullptr;
-ScriptInterpreterPython::SWIGPythonCreateSyntheticProvider
-ScriptInterpreterPython::g_swig_synthetic_script = nullptr;
-ScriptInterpreterPython::SWIGPythonCreateCommandObject
-ScriptInterpreterPython::g_swig_create_cmd = nullptr;
-ScriptInterpreterPython::SWIGPythonCalculateNumChildren
-ScriptInterpreterPython::g_swig_calc_children = nullptr;
-ScriptInterpreterPython::SWIGPythonGetChildAtIndex
-ScriptInterpreterPython::g_swig_get_child_index = nullptr;
-ScriptInterpreterPython::SWIGPythonGetIndexOfChildWithName
-ScriptInterpreterPython::g_swig_get_index_child = nullptr;
-ScriptInterpreterPython::SWIGPythonCastPyObjectToSBValue
-ScriptInterpreterPython::g_swig_cast_to_sbvalue = nullptr;
-ScriptInterpreterPython::SWIGPythonGetValueObjectSPFromSBValue
-ScriptInterpreterPython::g_swig_get_valobj_sp_from_sbvalue = nullptr;
-ScriptInterpreterPython::SWIGPythonUpdateSynthProviderInstance
-ScriptInterpreterPython::g_swig_update_provider = nullptr;
-ScriptInterpreterPython::SWIGPythonMightHaveChildrenSynthProviderInstance
-ScriptInterpreterPython::g_swig_mighthavechildren_provider = nullptr;
-ScriptInterpreterPython::SWIGPythonGetValueSynthProviderInstance
-ScriptInterpreterPython::g_swig_getvalue_provider = nullptr;
-ScriptInterpreterPython::SWIGPythonCallCommand
-ScriptInterpreterPython::g_swig_call_command = nullptr;
-ScriptIn

[Lldb-commits] [PATCH] D59850: [Platform] Remove Kalimba Platform

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added subscribers: jdoerfert, MaskRay, arichardson, mgorny, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLDB.

Yesterday I stumbled upon the initialization code for the "Kalimba" platform. 
It looks like this was added in 2014 and never had any tests. If nobody is 
relying on this platform, I propose to remove it.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59850

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  
lldb/packages/Python/lldbsuite/test/functionalities/object-file/TestImageListMultiArchitecture.py
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/Platform/CMakeLists.txt
  lldb/source/Plugins/Platform/Kalimba/CMakeLists.txt
  lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
  lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/tools/lldb-test/SystemInitializerTest.cpp

Index: lldb/tools/lldb-test/SystemInitializerTest.cpp
===
--- lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -57,7 +57,6 @@
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
 #include "Plugins/Platform/Android/PlatformAndroid.h"
 #include "Plugins/Platform/FreeBSD/PlatformFreeBSD.h"
-#include "Plugins/Platform/Kalimba/PlatformKalimba.h"
 #include "Plugins/Platform/Linux/PlatformLinux.h"
 #include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
 #include "Plugins/Platform/MacOSX/PlatformRemoteiOS.h"
@@ -129,7 +128,6 @@
   platform_netbsd::PlatformNetBSD::Initialize();
   platform_openbsd::PlatformOpenBSD::Initialize();
   PlatformWindows::Initialize();
-  PlatformKalimba::Initialize();
   platform_android::PlatformAndroid::Initialize();
   PlatformRemoteiOS::Initialize();
   PlatformMacOSX::Initialize();
@@ -325,7 +323,6 @@
   platform_netbsd::PlatformNetBSD::Terminate();
   platform_openbsd::PlatformOpenBSD::Terminate();
   PlatformWindows::Terminate();
-  PlatformKalimba::Terminate();
   platform_android::PlatformAndroid::Terminate();
   PlatformMacOSX::Terminate();
   PlatformRemoteiOS::Terminate();
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -214,13 +214,7 @@
  ArchSpec::eCore_uknownMach32, "unknown-mach-32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::UnknownArch,
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
-
-{eByteOrderBig, 4, 1, 1, llvm::Triple::kalimba, ArchSpec::eCore_kalimba3,
- "kalimba3"},
-{eByteOrderLittle, 4, 1, 1, llvm::Triple::kalimba, ArchSpec::eCore_kalimba4,
- "kalimba4"},
-{eByteOrderLittle, 4, 1, 1, llvm::Triple::kalimba, ArchSpec::eCore_kalimba5,
- "kalimba5"}};
+};
 
 // Ensure that we have an entry in the g_core_definitions for each core. If you
 // comment out an entry above, you will need to comment out the corresponding
@@ -452,12 +446,6 @@
  ArchSpec::eMIPSSubType_mips64r6el, 0xu, 0xu}, // mips64r6el
 {ArchSpec::eCore_hexagon_generic, llvm::ELF::EM_HEXAGON,
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
-{ArchSpec::eCore_kalimba3, llvm::ELF::EM_CSR_KALIMBA,
- llvm::Triple::KalimbaSubArch_v3, 0xu, 0xu}, // KALIMBA
-{ArchSpec::eCore_kalimba4, llvm::ELF::EM_CSR_KALIMBA,
- llvm::Triple::KalimbaSubArch_v4, 0xu, 0xu}, // KALIMBA
-{ArchSpec::eCore_kalimba5, llvm::ELF::EM_CSR_KALIMBA,
- llvm::Triple::KalimbaSubArch_v5, 0xu, 0xu} // KALIMBA
 };
 
 static const ArchDefinition g_elf_arch_def = {
@@ -728,30 +716,10 @@
 }
 
 uint32_t ArchSpec::GetDataByteSize() const {
-  switch (m_core) {
-  case eCore_kalimba3:
-return 4;
-  case eCore_kalimba4:
-return 1;
-  case eCore_kalimba5:
-return 4;
-  default:
-return 1;
-  }
   return 1;
 }
 
 uint32_t ArchSpec::GetCodeByteSize() const {
-  switch (m_core) {
-  case eCore_kalimba3:
-return 4;
-  case eCore_kalimba4:
-return 1;
-  case eCore_kalimba5:
-return 1;
-  default:
-return 1;
-  }
   return 1;
 }
 
@@ -942,13 +910,13 @@
   m_triple.setVendor(llvm::Triple::Apple);
 
   // Don't set the OS.  It could be simulator, macosx, ios, watchos,
-  // tvos, bridgeos.  We could get close with the cpu type - but we 
-  // can't get it right all of the time.  Better to leave this unset 
-  // so other sections of code will set it when they have more 
-  // information. NB: don't call m_triple.setOS (llvm::Triple::UnknownOS).  
-  // That sets the OSName to "unknown" and the 
-  // ArchSpec::TripleVendorWasSpecified() method says that any OSName 
-  // setting means it was specified.
+  // tvos, bridgeos.  We could get clos

[Lldb-commits] [lldb] r357037 - Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

[Duncan P. N. Exon Smith via lldb-commits]

Author: dexonsmith
Date: Tue Mar 26 15:18:52 2019
New Revision: 357037

URL: http://llvm.org/viewvc/llvm-project?rev=357037&view=rev
Log:
Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

Remove CompilerInstance::VirtualFileSystem and
CompilerInstance::setVirtualFileSystem, instead relying on the VFS in
the FileManager.  CompilerInstance and its clients already went to some
trouble to make these match.  Now they are guaranteed to match.

As part of this, I added a VFS parameter (defaults to nullptr) to
CompilerInstance::createFileManager, to avoid repeating construction
logic in clients that just wanted to customize the VFS.

https://reviews.llvm.org/D59377

Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Modified: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=357037&r1=357036&r2=357037&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
Tue Mar 26 15:18:52 2019
@@ -309,8 +309,7 @@ ClangExpressionParser::ClangExpressionPa
   }
 
   // Make sure clang uses the same VFS as LLDB.
-  m_compiler->setVirtualFileSystem(
-  FileSystem::Instance().GetVirtualFileSystem());
+  m_compiler->createFileManager(FileSystem::Instance().GetVirtualFileSystem());
 
   lldb::LanguageType frame_lang =
   expr.Language(); // defaults to lldb::eLanguageTypeUnknown

Modified: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp?rev=357037&r1=357036&r2=357037&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp 
Tue Mar 26 15:18:52 2019
@@ -679,7 +679,7 @@ ClangModulesDeclVendor::Create(Target &t
   }
 
   // Make sure clang uses the same VFS as LLDB.
-  
instance->setVirtualFileSystem(FileSystem::Instance().GetVirtualFileSystem());
+  instance->createFileManager(FileSystem::Instance().GetVirtualFileSystem());
   instance->setDiagnostics(diagnostics_engine.get());
   instance->setInvocation(invocation);
 


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

Re: [Lldb-commits] [lldb] r357034 - [Python] Remove dynamic indirection

[Davide Italiano via lldb-commits]

This is great!

On Tue, Mar 26, 2019 at 2:55 PM Jonas Devlieghere via lldb-commits
 wrote:
>
> Author: jdevlieghere
> Date: Tue Mar 26 14:57:02 2019
> New Revision: 357034
>
> URL: http://llvm.org/viewvc/llvm-project?rev=357034&view=rev
> Log:
> [Python] Remove dynamic indirection
>
> Now that the Python plugin relies on the SWIG symbols, we no longer need
> to dynamically resolve these functions.
>
> Modified:
> lldb/trunk/source/API/SystemInitializerFull.cpp
> lldb/trunk/source/API/SystemInitializerFull.h
> 
> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
> 
> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
>
> Modified: lldb/trunk/source/API/SystemInitializerFull.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SystemInitializerFull.cpp?rev=357034&r1=357033&r2=357034&view=diff
> ==
> --- lldb/trunk/source/API/SystemInitializerFull.cpp (original)
> +++ lldb/trunk/source/API/SystemInitializerFull.cpp Tue Mar 26 14:57:02 2019
> @@ -142,7 +142,6 @@ llvm::Error SystemInitializerFull::Initi
>  #endif
>
>  #if !defined(LLDB_DISABLE_PYTHON)
> -  ScriptInterpreterPython::InitializeSWIG();
>ScriptInterpreterPython::Initialize();
>  #endif
>
>
> Modified: lldb/trunk/source/API/SystemInitializerFull.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SystemInitializerFull.h?rev=357034&r1=357033&r2=357034&view=diff
> ==
> --- lldb/trunk/source/API/SystemInitializerFull.h (original)
> +++ lldb/trunk/source/API/SystemInitializerFull.h Tue Mar 26 14:57:02 2019
> @@ -27,9 +27,6 @@ public:
>
>llvm::Error Initialize() override;
>void Terminate() override;
> -
> -private:
> -  void InitializeSWIG();
>  };
>
>  } // namespace lldb_private
>
> Modified: 
> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=357034&r1=357033&r2=357034&view=diff
> ==
> --- 
> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
>  (original)
> +++ 
> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
>  Tue Mar 26 14:57:02 2019
> @@ -71,10 +71,9 @@ extern "C" void init_lldb(void);
>  #define LLDBSwigPyInit init_lldb
>  #endif
>
> -// these are the Pythonic implementations of the required callbacks these are
> -// scripting-language specific, which is why they belong here we still need 
> to
> -// use function pointers to them instead of relying on linkage-time 
> resolution
> -// because the SWIG stuff and this file get built at different times
> +// These prototypes are the Pythonic implementations of the required 
> callbacks.
> +// Although these are scripting-language specific, their definition depends 
> on
> +// the public API.
>  extern "C" bool LLDBSwigPythonBreakpointCallbackFunction(
>  const char *python_function_name, const char *session_dictionary_name,
>  const lldb::StackFrameSP &sb_frame,
> @@ -194,67 +193,6 @@ LLDBSWIGPython_GetDynamicSetting(void *m
>
>  #endif
>
> -ScriptInterpreterPython::SWIGInitCallback
> -ScriptInterpreterPython::g_swig_init_callback = nullptr;
> -ScriptInterpreterPython::SWIGBreakpointCallbackFunction
> -ScriptInterpreterPython::g_swig_breakpoint_callback = nullptr;
> -ScriptInterpreterPython::SWIGWatchpointCallbackFunction
> -ScriptInterpreterPython::g_swig_watchpoint_callback = nullptr;
> -ScriptInterpreterPython::SWIGPythonTypeScriptCallbackFunction
> -ScriptInterpreterPython::g_swig_typescript_callback = nullptr;
> -ScriptInterpreterPython::SWIGPythonCreateSyntheticProvider
> -ScriptInterpreterPython::g_swig_synthetic_script = nullptr;
> -ScriptInterpreterPython::SWIGPythonCreateCommandObject
> -ScriptInterpreterPython::g_swig_create_cmd = nullptr;
> -ScriptInterpreterPython::SWIGPythonCalculateNumChildren
> -ScriptInterpreterPython::g_swig_calc_children = nullptr;
> -ScriptInterpreterPython::SWIGPythonGetChildAtIndex
> -ScriptInterpreterPython::g_swig_get_child_index = nullptr;
> -ScriptInterpreterPython::SWIGPythonGetIndexOfChildWithName
> -ScriptInterpreterPython::g_swig_get_index_child = nullptr;
> -ScriptInterpreterPython::SWIGPythonCastPyObjectToSBValue
> -ScriptInterpreterPython::g_swig_cast_to_sbvalue = nullptr;
> -ScriptInterpreterPython::SWIGPythonGetValueObjectSPFromSBValue
> -ScriptInterpreterPython::g_swig_get_valobj_sp_from_sbvalue = nullptr;
> -ScriptInterpreterPython::SWIGPythonUpdateSynthProviderInstance
> -ScriptInterpreterPython::g_swig_update_provider = nullptr;
> -ScriptInterpreterPython::SWIGPythonMightHaveChildrenSynthProviderInstance
> -ScriptInterpreterPytho

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: zturner, stella.stamenova, davide.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.
JDevlieghere added a reviewer: labath.

Since there's no dlopen on Windows, we need a cross platform abstraction. The 
LibraryLoader is exactly that.

I've kept it as simple as possible for now.

As per usual I don't have access to a Windows machine. If anybody could test 
this before we check this in (looks at @zturner and @stella.stamenova) that 
would be great! ;-)


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59854

Files:
  lldb/include/lldb/Host/LibraryLoader.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/posix/LibraryLoader.cpp
  lldb/source/Host/windows/LibraryLoader.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/LibraryLoaderTest.cpp

Index: lldb/unittests/Host/LibraryLoaderTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/LibraryLoaderTest.cpp
@@ -0,0 +1,37 @@
+//===-- LibraryLoaderTest.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/Host/LibraryLoader.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+typedef size_t strlen_t(const char *str);
+
+TEST(LibraryLoaderTest, OperatorBool) {
+  // Loading a bogus library fails.
+  LibraryLoader bogus("bogus");
+  EXPECT_FALSE(static_cast(bogus));
+
+  // Loading ourselves succeeds.
+  LibraryLoader self(nullptr);
+  EXPECT_TRUE(static_cast(self));
+}
+
+TEST(LibraryLoaderTest, GetSymbol) {
+  LibraryLoader loader(nullptr);
+  EXPECT_TRUE(static_cast(loader));
+
+  strlen_t *test = loader.GetSymbol("strlen");
+
+  ASSERT_NE(nullptr, test);
+  EXPECT_EQ((size_t)4, test("test"));
+
+  strlen_t *bogus = loader.GetSymbol("bogus");
+  EXPECT_EQ(nullptr, bogus);
+}
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -3,6 +3,7 @@
   FileSystemTest.cpp
   HostInfoTest.cpp
   HostTest.cpp
+  LibraryLoaderTest.cpp
   MainLoopTest.cpp
   NativeProcessProtocolTest.cpp
   ProcessLaunchInfoTest.cpp
Index: lldb/source/Host/windows/LibraryLoader.cpp
===
--- /dev/null
+++ lldb/source/Host/windows/LibraryLoader.cpp
@@ -0,0 +1,31 @@
+//===-- LibraryLoader.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/Host/LibraryLoader.h"
+#include 
+
+using namespace lldb_private;
+using namespace llvm;
+
+LibraryLoader::LibraryLoader(const char *library, bool close)
+: m_handle(nullptr), m_close(close) {
+  m_handle = reinterpret_cast(LoadLibrary(library));
+}
+
+LibraryLoader::~LibraryLoader() {
+  if (m_close && m_handle) {
+FreeLibrary(m_handle);
+m_handle = nullptr;
+  }
+}
+
+void *LibraryLoader::GetSymbolImpl(llvm::StringRef symbol) {
+  if (!m_handle)
+return nullptr;
+  return GetProcAddress(reinterpret_cast(m_handle), symbol.data());
+}
Index: lldb/source/Host/posix/LibraryLoader.cpp
===
--- /dev/null
+++ lldb/source/Host/posix/LibraryLoader.cpp
@@ -0,0 +1,31 @@
+//===-- LibraryLoader.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/Host/LibraryLoader.h"
+#include 
+
+using namespace lldb_private;
+using namespace llvm;
+
+LibraryLoader::LibraryLoader(const char *library, bool close)
+: m_handle(nullptr), m_close(close) {
+  m_handle = dlopen(library, RTLD_LAZY);
+}
+
+LibraryLoader::~LibraryLoader() {
+  if (m_close && m_handle) {
+dlclose(m_handle);
+m_handle = nullptr;
+  }
+}
+
+void *LibraryLoader::GetSymbolImpl(llvm::StringRef symbol) {
+  if (!m_handle)
+return nullptr;
+  return dlsym(m_handle, symbol.data());
+}
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -70,6 +70,7 @@
 windows/HostInfoWindows.cpp
 windows/HostPr

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

The Windows code looks correct to me, but I've added a couple inline 
questions/comments.

I can patch it in and test it for you tomorrow morning if nobody else has done 
it by then.  I think zturner's still on vacation for another day or two.




Comment at: lldb/include/lldb/Host/LibraryLoader.h:17
+public:
+  LibraryLoader(const char *library, bool close = true);
+  ~LibraryLoader();

I'm curious why anyone would choose `close = false`



Comment at: lldb/include/lldb/Host/LibraryLoader.h:17
+public:
+  LibraryLoader(const char *library, bool close = true);
+  ~LibraryLoader();

amccarth wrote:
> I'm curious why anyone would choose `close = false`
Why `const char *` here when the GetSymbol method takes a StringRef?



Comment at: lldb/include/lldb/Host/LibraryLoader.h:20
+
+  operator bool() { return m_handle != nullptr; }
+

Probably should be a const method.

This will work on Windows, since we know this handle is from LoadLibrary.  Some 
other Windows handles can signal an error with INVALID_HANDLE_VALUE rather than 
a null pointer, but LoadLibrary doesn't.  If you wanted to be extra careful, 
you could delegate the validity check to the concrete type, but that doesn't 
seem necessary and it looks like we could change it later without affecting the 
interface.



Comment at: lldb/source/Host/windows/LibraryLoader.cpp:17
+: m_handle(nullptr), m_close(close) {
+  m_handle = reinterpret_cast(LoadLibrary(library));
+}

Since you're passing a char string to LoadLibrary, you might want to explicitly 
call LoadLibraryA (note the -A suffix).

I know that we generally don't support Unicode well in filenames throughout the 
LLVM system.  If you know the encoded string is UTF-8, you could convert it to 
UTF-16 and call LoadLibraryW.  If you leave it to LoadLibraryA, it'll use the 
user's current default code page, which almost certainly is not UTF-8.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59854



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

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

It isn't safe to pass "data()" of a StringRef to dlsym.

Also, it's a little weird that the library name is a "const char *" but the 
symbol name is a StringRef?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59854



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

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 192388.
JDevlieghere marked 4 inline comments as done.

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

https://reviews.llvm.org/D59854

Files:
  lldb/include/lldb/Host/LibraryLoader.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/posix/LibraryLoader.cpp
  lldb/source/Host/windows/LibraryLoader.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/LibraryLoaderTest.cpp

Index: lldb/unittests/Host/LibraryLoaderTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/LibraryLoaderTest.cpp
@@ -0,0 +1,42 @@
+//===-- LibraryLoaderTest.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/Host/LibraryLoader.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+typedef size_t strlen_t(const char *str);
+
+class TestLibraryLoader : public LibraryLoader {
+public:
+  TestLibraryLoader(const char *library) : LibraryLoader(library) {}
+};
+
+TEST(LibraryLoaderTest, OperatorBool) {
+  // Loading a bogus library fails.
+  TestLibraryLoader bogus("bogus");
+  EXPECT_FALSE(static_cast(bogus));
+
+  // Loading ourselves succeeds.
+  TestLibraryLoader self(nullptr);
+  EXPECT_TRUE(static_cast(self));
+}
+
+TEST(LibraryLoaderTest, GetSymbol) {
+  TestLibraryLoader loader(nullptr);
+  EXPECT_TRUE(static_cast(loader));
+
+  strlen_t *test = loader.GetSymbol("strlen");
+
+  ASSERT_NE(nullptr, test);
+  EXPECT_EQ((size_t)4, test("test"));
+
+  strlen_t *bogus = loader.GetSymbol("bogus");
+  EXPECT_EQ(nullptr, bogus);
+}
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -3,6 +3,7 @@
   FileSystemTest.cpp
   HostInfoTest.cpp
   HostTest.cpp
+  LibraryLoaderTest.cpp
   MainLoopTest.cpp
   NativeProcessProtocolTest.cpp
   ProcessLaunchInfoTest.cpp
Index: lldb/source/Host/windows/LibraryLoader.cpp
===
--- /dev/null
+++ lldb/source/Host/windows/LibraryLoader.cpp
@@ -0,0 +1,31 @@
+//===-- LibraryLoader.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/Host/LibraryLoader.h"
+#include 
+
+using namespace lldb_private;
+using namespace llvm;
+
+LibraryLoader::LibraryLoader(const char *library)
+: m_handle(nullptr) {
+  m_handle = reinterpret_cast(LoadLibraryA(library));
+}
+
+LibraryLoader::~LibraryLoader() {
+  if (m_handle) {
+FreeLibrary(m_handle);
+m_handle = nullptr;
+  }
+}
+
+void *LibraryLoader::GetSymbolImpl(llvm::StringRef symbol) {
+  if (!m_handle)
+return nullptr;
+  return GetProcAddress(reinterpret_cast(m_handle), symbol.data());
+}
Index: lldb/source/Host/posix/LibraryLoader.cpp
===
--- /dev/null
+++ lldb/source/Host/posix/LibraryLoader.cpp
@@ -0,0 +1,31 @@
+//===-- LibraryLoader.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/Host/LibraryLoader.h"
+#include 
+
+using namespace lldb_private;
+using namespace llvm;
+
+LibraryLoader::LibraryLoader(const char *library)
+: m_handle(nullptr) {
+  m_handle = dlopen(library, RTLD_LAZY);
+}
+
+LibraryLoader::~LibraryLoader() {
+  if (m_handle) {
+dlclose(m_handle);
+m_handle = nullptr;
+  }
+}
+
+void *LibraryLoader::GetSymbolImpl(llvm::StringRef symbol) {
+  if (!m_handle)
+return nullptr;
+  return dlsym(m_handle, symbol.data());
+}
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -70,6 +70,7 @@
 windows/HostInfoWindows.cpp
 windows/HostProcessWindows.cpp
 windows/HostThreadWindows.cpp
+windows/LibraryLoader.cpp
 windows/LockFileWindows.cpp
 windows/PipeWindows.cpp
 windows/ProcessLauncherWindows.cpp
@@ -83,6 +84,7 @@
 posix/HostInfoPosix.cpp
 posix/HostProcessPosix.cpp
 posix/HostThreadPosix.cpp
+posix/LibraryLoader.cpp
 p

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Thanks for the review, Adrian!




Comment at: lldb/include/lldb/Host/LibraryLoader.h:17
+public:
+  LibraryLoader(const char *library, bool close = true);
+  ~LibraryLoader();

amccarth wrote:
> amccarth wrote:
> > I'm curious why anyone would choose `close = false`
> Why `const char *` here when the GetSymbol method takes a StringRef?
I changed it so I could pass a nullptr in the unittest. 



Comment at: lldb/include/lldb/Host/LibraryLoader.h:17
+public:
+  LibraryLoader(const char *library, bool close = true);
+  ~LibraryLoader();

JDevlieghere wrote:
> amccarth wrote:
> > amccarth wrote:
> > > I'm curious why anyone would choose `close = false`
> > Why `const char *` here when the GetSymbol method takes a StringRef?
> I changed it so I could pass a nullptr in the unittest. 
I'm not sure about the semantics of "closing" the library. For the python 
plugin we call initialize once, so it's fine that the symbol is gone, as long 
as the library remains in memory. 


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

https://reviews.llvm.org/D59854



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

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D59854#1443891 , @jingham wrote:

> It isn't safe to pass "data()" of a StringRef to dlsym.


Why?


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

https://reviews.llvm.org/D59854



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

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

As we're just adding test coverage, could we add a little more?

- Anonymous enum
- Enum through a typedef
- class enum
- enum declared inside of the function rather than at the top-level
- nested enum in a record type
- enum nested in a templated class
- anything else I haven't thought about...


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

https://reviews.llvm.org/D59847



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

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D59854#1443904 , @JDevlieghere 
wrote:

> In D59854#1443891 , @jingham wrote:
>
> > It isn't safe to pass "data()" of a StringRef to dlsym.
>
>
> Why?


StringRef's aren't guaranteed to be NULL terminated.


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

https://reviews.llvm.org/D59854



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

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In D59854#1443906 , @jingham wrote:

> In D59854#1443904 , @JDevlieghere 
> wrote:
>
> > In D59854#1443891 , @jingham wrote:
> >
> > > It isn't safe to pass "data()" of a StringRef to dlsym.
> >
> >
> > Why?
>
>
> StringRef's aren't guaranteed to be NULL terminated.  Or rather, the Data 
> that gets returned by the data() method is just the raw pointer in the 
> StringRef, which isn't guaranteed to be NULL terminated.


While true, I don't think this is a good reason to make the interface accept a 
`const char*`, because that means that the interface is exposing an 
implementation detail.  Instead, the interface should accept a `StringRef`, and 
the implementation can pass `foo.str().c_str()`.  It might be slightly less 
efficient (almost to the point of minutia), but this isn't really performance 
sensitive code, as loading the library will dominate the call to malloc / 
memcpy.


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

https://reviews.llvm.org/D59854



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

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D59854#1443915 , @zturner wrote:

> In D59854#1443906 , @jingham wrote:
>
> > In D59854#1443904 , @JDevlieghere 
> > wrote:
> >
> > > In D59854#1443891 , @jingham 
> > > wrote:
> > >
> > > > It isn't safe to pass "data()" of a StringRef to dlsym.
> > >
> > >
> > > Why?
> >
> >
> > StringRef's aren't guaranteed to be NULL terminated.  Or rather, the Data 
> > that gets returned by the data() method is just the raw pointer in the 
> > StringRef, which isn't guaranteed to be NULL terminated.
>
>
> While true, I don't think this is a good reason to make the interface accept 
> a `const char*`, because that means that the interface is exposing an 
> implementation detail.  Instead, the interface should accept a `StringRef`, 
> and the implementation can pass `foo.str().c_str()`.  It might be slightly 
> less efficient (almost to the point of minutia), but this isn't really 
> performance sensitive code, as loading the library will dominate the call to 
> malloc / memcpy.


I don't actually care one way or the other about using a StringRef here (though 
I still think it is odd to have the library be a char * and the symbol a 
StringRef...).  I was only concerned that you can't pass the data() of a 
StringRef to anything that expects a c-string.


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

https://reviews.llvm.org/D59854



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

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 192392.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

`data()` -> `str().c_str()`


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

https://reviews.llvm.org/D59854

Files:
  lldb/include/lldb/Host/LibraryLoader.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/posix/LibraryLoader.cpp
  lldb/source/Host/windows/LibraryLoader.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/LibraryLoaderTest.cpp

Index: lldb/unittests/Host/LibraryLoaderTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/LibraryLoaderTest.cpp
@@ -0,0 +1,42 @@
+//===-- LibraryLoaderTest.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/Host/LibraryLoader.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+typedef size_t strlen_t(const char *str);
+
+class TestLibraryLoader : public LibraryLoader {
+public:
+  TestLibraryLoader(const char *library) : LibraryLoader(library) {}
+};
+
+TEST(LibraryLoaderTest, OperatorBool) {
+  // Loading a bogus library fails.
+  TestLibraryLoader bogus("bogus");
+  EXPECT_FALSE(static_cast(bogus));
+
+  // Loading ourselves succeeds.
+  TestLibraryLoader self(nullptr);
+  EXPECT_TRUE(static_cast(self));
+}
+
+TEST(LibraryLoaderTest, GetSymbol) {
+  TestLibraryLoader loader(nullptr);
+  EXPECT_TRUE(static_cast(loader));
+
+  strlen_t *test = loader.GetSymbol("strlen");
+
+  ASSERT_NE(nullptr, test);
+  EXPECT_EQ((size_t)4, test("test"));
+
+  strlen_t *bogus = loader.GetSymbol("bogus");
+  EXPECT_EQ(nullptr, bogus);
+}
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -3,6 +3,7 @@
   FileSystemTest.cpp
   HostInfoTest.cpp
   HostTest.cpp
+  LibraryLoaderTest.cpp
   MainLoopTest.cpp
   NativeProcessProtocolTest.cpp
   ProcessLaunchInfoTest.cpp
Index: lldb/source/Host/windows/LibraryLoader.cpp
===
--- /dev/null
+++ lldb/source/Host/windows/LibraryLoader.cpp
@@ -0,0 +1,31 @@
+//===-- LibraryLoader.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/Host/LibraryLoader.h"
+#include 
+
+using namespace lldb_private;
+using namespace llvm;
+
+LibraryLoader::LibraryLoader(const char *library) : m_handle(nullptr) {
+  m_handle = reinterpret_cast(LoadLibraryA(library));
+}
+
+LibraryLoader::~LibraryLoader() {
+  if (m_handle) {
+FreeLibrary(m_handle);
+m_handle = nullptr;
+  }
+}
+
+void *LibraryLoader::GetSymbolImpl(llvm::StringRef symbol) {
+  if (!m_handle)
+return nullptr;
+  return GetProcAddress(reinterpret_cast(m_handle),
+symbol.str().c_str());
+}
Index: lldb/source/Host/posix/LibraryLoader.cpp
===
--- /dev/null
+++ lldb/source/Host/posix/LibraryLoader.cpp
@@ -0,0 +1,30 @@
+//===-- LibraryLoader.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/Host/LibraryLoader.h"
+#include 
+
+using namespace lldb_private;
+using namespace llvm;
+
+LibraryLoader::LibraryLoader(const char *library) : m_handle(nullptr) {
+  m_handle = dlopen(library, RTLD_LAZY);
+}
+
+LibraryLoader::~LibraryLoader() {
+  if (m_handle) {
+dlclose(m_handle);
+m_handle = nullptr;
+  }
+}
+
+void *LibraryLoader::GetSymbolImpl(llvm::StringRef symbol) {
+  if (!m_handle)
+return nullptr;
+  return dlsym(m_handle, symbol.str().c_str());
+}
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -70,6 +70,7 @@
 windows/HostInfoWindows.cpp
 windows/HostProcessWindows.cpp
 windows/HostThreadWindows.cpp
+windows/LibraryLoader.cpp
 windows/LockFileWindows.cpp
 windows/PipeWindows.cpp
 windows/ProcessLauncherWindows.cpp
@@ -83,6 +84,7 @@
 posix/HostInfoPosix.cpp
 posi

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Is there a reason you can't reuse/extend `DynamicLibrary` from `LLVMSupport`?


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

https://reviews.llvm.org/D59854



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