[Lldb-commits] [PATCH] D71489: [lldb][NFC] Remove unnecessary includes in source/Commands

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG068325012796: [lldb][NFC] Remove unnecessary includes in 
source/Commands (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71489

Files:
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectApropos.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpoint.h
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.h
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectCommands.h
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectFrame.h
  lldb/source/Commands/CommandObjectGUI.cpp
  lldb/source/Commands/CommandObjectHelp.cpp
  lldb/source/Commands/CommandObjectLanguage.cpp
  lldb/source/Commands/CommandObjectLanguage.h
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/CommandObjectLog.h
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMultiword.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectPlatform.h
  lldb/source/Commands/CommandObjectPlugin.cpp
  lldb/source/Commands/CommandObjectPlugin.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectReproducer.h
  lldb/source/Commands/CommandObjectSettings.h
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Commands/CommandObjectSource.h
  lldb/source/Commands/CommandObjectStats.cpp
  lldb/source/Commands/CommandObjectStats.h
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectTarget.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectType.h
  lldb/source/Commands/CommandObjectVersion.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/CommandObjectWatchpoint.h
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.h

Index: lldb/source/Commands/CommandObjectWatchpointCommand.h
===
--- lldb/source/Commands/CommandObjectWatchpointCommand.h
+++ lldb/source/Commands/CommandObjectWatchpointCommand.h
@@ -10,8 +10,6 @@
 #define liblldb_CommandObjectWatchpointCommand_h_
 
 #include "lldb/Interpreter/CommandObjectMultiword.h"
-#include "lldb/Interpreter/Options.h"
-#include "lldb/lldb-types.h"
 
 namespace lldb_private {
 
Index: lldb/source/Commands/CommandObjectWatchpointCommand.cpp
===
--- lldb/source/Commands/CommandObjectWatchpointCommand.cpp
+++ lldb/source/Commands/CommandObjectWatchpointCommand.cpp
@@ -18,8 +18,6 @@
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Target/Target.h"
-#include "lldb/Target/Thread.h"
-#include "lldb/Utility/State.h"
 
 using namespace lldb;
 using namespace lldb_private;
Index: lldb/source/Commands/CommandObjectWatchpoint.h
===
--- lldb/source/Commands/CommandObjectWatchpoint.h
+++ lldb/source/Commands/CommandObjectWatchpoint.h
@@ -11,7 +11,6 @@
 
 #include "lldb/Interpreter/CommandObjectMultiword.h"
 #include "lldb/Interpreter/OptionGroupWatchpoint.h"
-#include "lldb/Interpreter/Options.h"
 
 namespace lldb_private {
 
Index: lldb/source/Commands/CommandObjectWatchpoint.cpp
===
--- lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -16,9 +16,7 @@
 #include "lldb/Breakpoint/Watchpoint.h"
 #include "lldb/Breakpoint/WatchpointList.h"
 #include "lldb/Core/ValueObject.h"
-#include "lldb/Core/ValueObjectVariable.h"
 #include "lldb/Host/OptionParser.h"
-#include "lldb/Interpreter/CommandCompletions.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Symbol/Variable.h"
Index: lldb/source/Commands/CommandObjectVersion.cpp
===
--- lldb/source/Commands/CommandObjectVersion.cpp
+++ lldb/source/Commands/CommandObjectVersion.cpp
@@ -8,7 +8,6 @@
 
 #include "CommandObjectVersion.h"
 
-#include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/lldb-private.h"
 
Index: lldb/source/Commands/CommandObjectType.h

[Lldb-commits] [lldb] 64678ef - [lldb][NFC] Remove ClangASTImporter::ResolveDeclOrigin

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-16T09:16:33+01:00
New Revision: 64678ef9f289e9c1951fee5dbcacde583f3d6576

URL: 
https://github.com/llvm/llvm-project/commit/64678ef9f289e9c1951fee5dbcacde583f3d6576
DIFF: 
https://github.com/llvm/llvm-project/commit/64678ef9f289e9c1951fee5dbcacde583f3d6576.diff

LOG: [lldb][NFC] Remove ClangASTImporter::ResolveDeclOrigin

ResolveDeclOrigin was just an inconvenience method around GetDeclOrigin.

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangASTImporter.h
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
lldb/source/Symbol/ClangASTImporter.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTImporter.h 
b/lldb/include/lldb/Symbol/ClangASTImporter.h
index 098091f7167f..cd01bed624cc 100644
--- a/lldb/include/lldb/Symbol/ClangASTImporter.h
+++ b/lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -96,19 +96,6 @@ class ClangASTImporter {
 
   bool RequireCompleteType(clang::QualType type);
 
-  bool ResolveDeclOrigin(const clang::Decl *decl, clang::Decl **original_decl,
- clang::ASTContext **original_ctx) {
-DeclOrigin origin = GetDeclOrigin(decl);
-
-if (original_decl)
-  *original_decl = origin.decl;
-
-if (original_ctx)
-  *original_ctx = origin.ctx;
-
-return origin.Valid();
-  }
-
   void SetDeclOrigin(const clang::Decl *decl, clang::Decl *original_decl);
 
   ClangASTMetadata *GetDeclMetadata(const clang::Decl *decl);

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index d05c074991ee..7a23606f57bc 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -438,13 +438,11 @@ void 
ClangASTSource::CompleteType(clang::ObjCInterfaceDecl *interface_decl) {
 return;
   }
 
-  Decl *original_decl = nullptr;
-  ASTContext *original_ctx = nullptr;
+  ClangASTImporter::DeclOrigin original = 
m_ast_importer_sp->GetDeclOrigin(interface_decl);
 
-  if (m_ast_importer_sp->ResolveDeclOrigin(interface_decl, &original_decl,
-   &original_ctx)) {
+  if (original.Valid()) {
 if (ObjCInterfaceDecl *original_iface_decl =
-dyn_cast(original_decl)) {
+dyn_cast(original.decl)) {
   ObjCInterfaceDecl *complete_iface_decl =
   GetCompleteObjCInterface(original_iface_decl);
 
@@ -565,40 +563,38 @@ void ClangASTSource::FindExternalLexicalDecls(
   current_id, static_cast(m_ast_context));
   }
 
-  Decl *original_decl = nullptr;
-  ASTContext *original_ctx = nullptr;
+  ClangASTImporter::DeclOrigin original = 
m_ast_importer_sp->GetDeclOrigin(context_decl);
 
-  if (!m_ast_importer_sp->ResolveDeclOrigin(context_decl, &original_decl,
-&original_ctx))
+  if (!original.Valid())
 return;
 
   LLDB_LOG(
   log, "  FELD[{0}] Original decl (ASTContext*){1:x} (Decl*){2:x}:\n{3}",
-  current_id, static_cast(original_ctx),
-  static_cast(original_decl), ClangUtil::DumpDecl(original_decl));
+  current_id, static_cast(original.ctx),
+  static_cast(original.decl), ClangUtil::DumpDecl(original.decl));
 
   if (ObjCInterfaceDecl *original_iface_decl =
-  dyn_cast(original_decl)) {
+  dyn_cast(original.decl)) {
 ObjCInterfaceDecl *complete_iface_decl =
 GetCompleteObjCInterface(original_iface_decl);
 
 if (complete_iface_decl && (complete_iface_decl != original_iface_decl)) {
-  original_decl = complete_iface_decl;
-  original_ctx = &complete_iface_decl->getASTContext();
+  original.decl = complete_iface_decl;
+  original.ctx = &complete_iface_decl->getASTContext();
 
   m_ast_importer_sp->SetDeclOrigin(context_decl, complete_iface_decl);
 }
   }
 
-  if (TagDecl *original_tag_decl = dyn_cast(original_decl)) {
-ExternalASTSource *external_source = original_ctx->getExternalSource();
+  if (TagDecl *original_tag_decl = dyn_cast(original.decl)) {
+ExternalASTSource *external_source = original.ctx->getExternalSource();
 
 if (external_source)
   external_source->CompleteType(original_tag_decl);
   }
 
   const DeclContext *original_decl_context =
-  dyn_cast(original_decl);
+  dyn_cast(original.decl);
 
   if (!original_decl_context)
 return;
@@ -1036,11 +1032,10 @@ template  class DeclFromUser : public 
TaggedASTDecl {
 
 template 
 DeclFromUser DeclFromParser::GetOrigin(ClangASTSource &source) {
-  DeclFromUser<> origin_decl;
-  source.ResolveDeclOrigin(this->decl, &origin_decl.decl, nullptr);
-  if (origin_decl.IsInvalid())
+  ClangASTImporter::DeclOrigin origin = source.GetDeclOrigin(this->decl);
+  if (!origin.Valid())
 return DeclFromUser();
-  return DeclF

[Lldb-commits] [PATCH] D71482: [lldb/CMake] Rename LLDB_DISABLE_PYTHON to LLDB_ENABLE_PYTHON

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/cmake/modules/LLDBConfig.cmake:43
+  else()
+set(default_enable_python ON)
+  endif()

JDevlieghere wrote:
> mgorny wrote:
> > JDevlieghere wrote:
> > > mgorny wrote:
> > > > JDevlieghere wrote:
> > > > > mgorny wrote:
> > > > > > I know that the same thing is done above but… what's the point of 
> > > > > > setting it to `ON` again?
> > > > > It's just setting the default to the old value. If the value was set, 
> > > > > we should honor it, both if it was `OFF` or `ON`.
> > > > But it's already set to `ON` a few lines above.
> > > Do you mean the `DEFINED`? That just checks if the variable exists, not 
> > > its value. 
> > No, I mean you have:
> > 
> > ```
> > set(default_enable_python ON)
> > #...
> > if (DEFINED LLDB_DISABLE_PYTHON)
> >   #...
> >   else()
> > set(default_enable_python ON)
> > ```
> > 
> > This is superfluous since the default is already `ON`.
> Got it, thanks. Simplified the code. 
Yeah, I added the curses stuff without too much optimization to keep the debian 
bot working until the master restarts (so the configuration change takes 
effect). I don't think we have any bots using LLDB_DISABLE_PYTHON, so this 
probably wasn't even needed, but this all isn't very important -- the code will 
be gone in a few days anyway...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71482



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

[Lldb-commits] [lldb] 959ed0e - [lldb][NFC] Fix file header of TestClangASTContext.cpp

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-16T09:34:16+01:00
New Revision: 959ed0e2944c454a3df3aa3bc8ab551c8b587e9b

URL: 
https://github.com/llvm/llvm-project/commit/959ed0e2944c454a3df3aa3bc8ab551c8b587e9b
DIFF: 
https://github.com/llvm/llvm-project/commit/959ed0e2944c454a3df3aa3bc8ab551c8b587e9b.diff

LOG: [lldb][NFC] Fix file header of TestClangASTContext.cpp

Added: 


Modified: 
lldb/unittests/Symbol/TestClangASTContext.cpp

Removed: 




diff  --git a/lldb/unittests/Symbol/TestClangASTContext.cpp 
b/lldb/unittests/Symbol/TestClangASTContext.cpp
index 8fb24acc7a6a..b5c7542887e3 100644
--- a/lldb/unittests/Symbol/TestClangASTContext.cpp
+++ b/lldb/unittests/Symbol/TestClangASTContext.cpp
@@ -1,6 +1,4 @@
-//===-- TestClangASTContext.cpp ---*- C++
-//-*-===//
-
+//===-- TestClangASTContext.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.



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

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for member function linked with lld

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This definitely needs a more targeted fix. Requiring a symbol is not good, and 
it will be particularly problematic on windows, as there we don't have the 
equivalent of a .symtab (only .dynsym).

Looking at your example, I'm pretty sure that the DW_AT_object_pointer business 
is a red herring. That's just the normal way of describing member functions -- 
you first describe the interface of the function withing the containing 
DW_TAG_structure_type, and then you (outside of the struct) define the precise 
details of the method (code location, etc.). The second definition references 
the first one via DW_AT_specification, and lldb should already know how to read 
these things -- if not then a fix needs to be made there. But I don't think 
that's necessary, because the choice of linker should not impact this.

What I actually think is happening is that there are *two* definitions of the 
"four" function (because its an inline function used from two compilation 
units). This means the linker has to merge them, but unfortunately it cannot 
also "merge" the debug info (this is a long standing problem with dwarf). So, 
what the linker does is set the DW_AT_low_pc of the function which got dropped 
to zero. Lldb does not detect this, and so it ends up trying to call the 
address zero, and the expression crashes.

The reason that the choice of linker matters here is because they have 
different behavior regarding which copy gets dropped, and lld seems to drop the 
one which lldb tries first. This is also why your fix kind of works -- because 
there is no symbol at address zero (usually).

So, I think the fix for this should be in DWARF code, to make sure these kinds 
of functions don't even get considered. Somewhere in 
SymbolFileDWARF::FindFunctions, or maybe even deeper. One way to detect these 
stripped functions would be to check that the purported address of the function 
actually lies within the boundaries of one of the sections of the object file.

After that, we can probably come up with a simpler way to test this too...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71487



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

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

@jankratochvil I have verified both D71498  
and D71514 . They **do not fix** the issues 
introduced by D63540 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63540



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

[Lldb-commits] [lldb] f49d15b - [lldb][NFC] Move definition of ClangASTMetadata out of ClangExternalASTSourceCommon.h

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-16T10:52:31+01:00
New Revision: f49d15b3f8ccd7737a62d40adfe5ff1e710788d4

URL: 
https://github.com/llvm/llvm-project/commit/f49d15b3f8ccd7737a62d40adfe5ff1e710788d4
DIFF: 
https://github.com/llvm/llvm-project/commit/f49d15b3f8ccd7737a62d40adfe5ff1e710788d4.diff

LOG: [lldb][NFC] Move definition of ClangASTMetadata out of 
ClangExternalASTSourceCommon.h

Changing metadata of a ClangASTContext currently requires to include
the unrelated ClangExternalASTSourceCommon.h header because it actually defines
the ClangASTMetadata class.

This also removes the dependency from ClangASTImporter to 
ClangExternalASTSourceCommon.

Added: 
lldb/include/lldb/Symbol/ClangASTMetadata.h
lldb/source/Symbol/ClangASTMetadata.cpp

Modified: 
lldb/include/lldb/Symbol/ClangExternalASTSourceCommon.h
lldb/source/Symbol/CMakeLists.txt
lldb/source/Symbol/ClangASTImporter.cpp
lldb/source/Symbol/ClangExternalASTSourceCommon.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTMetadata.h 
b/lldb/include/lldb/Symbol/ClangASTMetadata.h
new file mode 100644
index ..fdf4388f0847
--- /dev/null
+++ b/lldb/include/lldb/Symbol/ClangASTMetadata.h
@@ -0,0 +1,100 @@
+//===-- ClangASTMetadata.h --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_ClangASTMetadata_h
+#define liblldb_ClangASTMetadata_h
+
+#include "lldb/Core/dwarf.h"
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+
+namespace lldb_private {
+
+class ClangASTMetadata {
+public:
+  ClangASTMetadata()
+  : m_user_id(0), m_union_is_user_id(false), m_union_is_isa_ptr(false),
+m_has_object_ptr(false), m_is_self(false), m_is_dynamic_cxx(true) {}
+
+  bool GetIsDynamicCXXType() const { return m_is_dynamic_cxx; }
+
+  void SetIsDynamicCXXType(bool b) { m_is_dynamic_cxx = b; }
+
+  void SetUserID(lldb::user_id_t user_id) {
+m_user_id = user_id;
+m_union_is_user_id = true;
+m_union_is_isa_ptr = false;
+  }
+
+  lldb::user_id_t GetUserID() const {
+if (m_union_is_user_id)
+  return m_user_id;
+else
+  return LLDB_INVALID_UID;
+  }
+
+  void SetISAPtr(uint64_t isa_ptr) {
+m_isa_ptr = isa_ptr;
+m_union_is_user_id = false;
+m_union_is_isa_ptr = true;
+  }
+
+  uint64_t GetISAPtr() const {
+if (m_union_is_isa_ptr)
+  return m_isa_ptr;
+else
+  return 0;
+  }
+
+  void SetObjectPtrName(const char *name) {
+m_has_object_ptr = true;
+if (strcmp(name, "self") == 0)
+  m_is_self = true;
+else if (strcmp(name, "this") == 0)
+  m_is_self = false;
+else
+  m_has_object_ptr = false;
+  }
+
+  lldb::LanguageType GetObjectPtrLanguage() const {
+if (m_has_object_ptr) {
+  if (m_is_self)
+return lldb::eLanguageTypeObjC;
+  else
+return lldb::eLanguageTypeC_plus_plus;
+}
+return lldb::eLanguageTypeUnknown;
+  }
+
+  const char *GetObjectPtrName() const {
+if (m_has_object_ptr) {
+  if (m_is_self)
+return "self";
+  else
+return "this";
+} else
+  return nullptr;
+  }
+
+  bool HasObjectPtr() const { return m_has_object_ptr; }
+
+  void Dump(Stream *s);
+
+private:
+  union {
+lldb::user_id_t m_user_id;
+uint64_t m_isa_ptr;
+  };
+
+  bool m_union_is_user_id : 1, m_union_is_isa_ptr : 1, m_has_object_ptr : 1,
+  m_is_self : 1, m_is_dynamic_cxx : 1;
+};
+
+} // namespace lldb_private
+
+#endif // liblldb_ClangASTMetadata_h

diff  --git a/lldb/include/lldb/Symbol/ClangExternalASTSourceCommon.h 
b/lldb/include/lldb/Symbol/ClangExternalASTSourceCommon.h
index 5da486540bb9..dada61d7d026 100644
--- a/lldb/include/lldb/Symbol/ClangExternalASTSourceCommon.h
+++ b/lldb/include/lldb/Symbol/ClangExternalASTSourceCommon.h
@@ -36,91 +36,12 @@
 #include "clang/AST/ExternalASTSource.h"
 
 #include "lldb/Core/dwarf.h"
+#include "lldb/Symbol/ClangASTMetadata.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
 
 namespace lldb_private {
 
-class ClangASTMetadata {
-public:
-  ClangASTMetadata()
-  : m_user_id(0), m_union_is_user_id(false), m_union_is_isa_ptr(false),
-m_has_object_ptr(false), m_is_self(false), m_is_dynamic_cxx(true) {}
-
-  bool GetIsDynamicCXXType() const { return m_is_dynamic_cxx; }
-
-  void SetIsDynamicCXXType(bool b) { m_is_dynamic_cxx = b; }
-
-  void SetUserID(lldb::user_id_t user_id) {
-m_user_id = user_id;
-m_union_is_user_id = true;
-m_union_is_isa_ptr = false;
-  }
-
-  lldb::user_id_t GetUserID() const {
-if (m_union_is_user_id)
-  return m_user_id;
-else
-  return LLDB_

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yeah, this looks like it was fun to track down. I'm going to comment on both 
patches here, since they are really similar...

Overall, I'm not very enthusiastic about the addr_t wrapper -- I'm not sure it 
would be even possible because that's a part of the public API. It may be 
possible to have something like lldb_private::addr_t, but I am not sure about 
the usefulness of that either. Maybe it would make sense to create two helper 
functions to convert pointers to and from lldb::addr_t "the right way".

As for these patches, instead of piling on casts, we should try to look for 
other solutions where it makes sense. For instance, in many cases we have 
parallel printf and formatv apis, and formatv usually does not require any 
casts (vs. two casts with printf).

In other places you're replacing a reinterpret_cast with two c casts. I 
guess this was done because two c++ casts were too long (?). In these cases the 
second cast is not really needed, as uintptr_t->addr_t should convert 
automatically. I think I'd prefer that we just have a single 
reinterpret_cast instead of two c casts. Then our rule can be 
"always convert a pointer to uintptr_t". I don't know if there's a clang-tidy 
check for that, but it sounds like that could be something which could be 
checked/enforced there...




Comment at: lldb/source/Expression/IRMemoryMap.cpp:577-586
   if (lldb_private::Log *log =
   lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS)) {
 LLDB_LOGF(log,
   "IRMemoryMap::WriteMemory (0x%" PRIx64 ", 0x%" PRIx64
   ", 0x%" PRId64 ") went to [0x%" PRIx64 "..0x%" PRIx64 ")",
-  (uint64_t)process_address, (uint64_t)bytes, (uint64_t)size,
-  (uint64_t)allocation.m_process_start,
+  (uint64_t)process_address, (uint64_t)(uintptr_t)bytes,
+  (uint64_t)size, (uint64_t)allocation.m_process_start,

This could be something 
`LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS), "({0:x}, {1:x}, 
{2:x}) went to [{3:x}, {4:x})", process_address, bytes, 
allocation.m_process_start, allocation.m_process_start + allocation.m_size)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [PATCH] D71372: [lldb] Add additional validation on return address in 'thread step-out'

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D71372#1785241 , @mossberg wrote:

> I wanted to also mention that this patch won't address buggy behavior if, for 
> example, the stub takes a function pointer on the stack (vs a normal data 
> pointer). In this case, the executable check will succeed, and the breakpoint 
> will be written, but to a potentially arbitrary code address. This may or may 
> not lead to the debugger "randomly" breaking out later in a debug session if 
> or when that code happens to be executed (I've reproduced this, but also seen 
> some very confusing behavior where in some situations the breakpoint logging 
> will say a breakpoint is written, but it doesn't seem like this was actually 
> the case. The presence of instructions that manipulate the stack pointer 
> seemed to affect this also..)


That's perfectly fine. This is all best effort -- if someone really wants us to 
do something bogus here, he'll always find a way. And OTOH, if he really wants 
us to do the right thing for functions like these, he should write unwind info 
for them.




Comment at: lldb/source/Target/ThreadPlanStepOut.cpp:136-137
+  m_return_addr);
+  LLDB_LOGF(log, "ThreadPlanStepOut(%p): %s", static_cast(this),
+m_constructor_errors.GetData());
+  return;

mossberg wrote:
> labath wrote:
> > I'd probably remove the log statements now that we have the real error 
> > output.
> I included both based on @jingham's feedback here: 
> https://reviews.llvm.org/D71372#1782270
Yeah, sorry, I missed that part. Jim is the thread plan master, so lets do what 
he thinks best.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71372



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

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

I still seem to get the same issue after applying this patch and D63540 
.

echo -e '#include \nint main(void){\nsync();return 0;}'|./bin/clang 
-g -x c -;./bin/lldb -o 'file ./a.out' -o 'b main' -o r -o 'p (void)sync()'
(lldb) file ./a.out
Current executable set to '/home/omair.javaid/work/lldb/build/armhf/a.out' 
(arm).
(lldb) b main
Breakpoint 1: where = a.out`main + 20 at :3:1, address = 0x000103c4
(lldb) r
Process 33825 stopped

- thread #1, name = 'a.out', stop reason = breakpoint 1.1 frame #0: 0x000103c4 
a.out`main at :3:1

Process 33825 launched: '/home/omair.javaid/work/lldb/build/armhf/a.out' (arm)
(lldb) p (void)sync()
error: Can't run the expression locally: Interpreter doesn't handle one of the 
expression's opcodes
(lldb)

I am trying to track it down will update you later today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [PATCH] D71405: [lldb] Centralize desugaring of decltype-like types in ClangASTContext

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The unit test sounds like a good idea. I'll do that instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71405



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

[Lldb-commits] [lldb] ea2805a - [lldb] Centralize desugaring of decltype-like types in ClangASTContext

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-16T12:02:32+01:00
New Revision: ea2805a04b65331ca568ff76761ef8a52337e42b

URL: 
https://github.com/llvm/llvm-project/commit/ea2805a04b65331ca568ff76761ef8a52337e42b
DIFF: 
https://github.com/llvm/llvm-project/commit/ea2805a04b65331ca568ff76761ef8a52337e42b.diff

LOG: [lldb] Centralize desugaring of decltype-like types in ClangASTContext

Summary:
These types were handled in some places, but not others. This resulted
in (for example) not being able to display members of structs whose
types were defined using these constructs.

Using getLocallyUnqualifiedSingleStepDesugaredType for these types is
not fully equivalent, as it will only desugar them if the types are not
instantiation-dependent, whereas previously we did that unconditionally.

It's not clear to me which behavior is correct here, but the test suite
does not seem to care either way.

Reviewers: teemperor, shafik

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Symbol/ClangASTContext.cpp
lldb/unittests/Symbol/TestClangASTContext.cpp

Removed: 




diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTContext.cpp
index ed613528a2f4..95bb4551f2c8 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -2499,9 +2499,12 @@ RemoveWrappingTypes(QualType type, 
ArrayRef mask = {}) {
   type = cast(type)->getValueType();
   break;
 case clang::Type::Auto:
+case clang::Type::Decltype:
 case clang::Type::Elaborated:
 case clang::Type::Paren:
 case clang::Type::Typedef:
+case clang::Type::TypeOf:
+case clang::Type::TypeOfExpr:
   type = type->getLocallyUnqualifiedSingleStepDesugaredType();
   break;
 default:
@@ -3765,11 +3768,6 @@ 
ClangASTContext::GetTypeInfo(lldb::opaque_compiler_type_t type,
 return eTypeHasChildren | eTypeIsVector;
   case clang::Type::DependentTemplateSpecialization:
 return eTypeIsTemplate;
-  case clang::Type::Decltype:
-return CompilerType(this, llvm::cast(qual_type)
-  ->getUnderlyingType()
-  .getAsOpaquePtr())
-.GetTypeInfo(pointee_or_element_clang_type);
 
   case clang::Type::Enum:
 if (pointee_or_element_clang_type)
@@ -3837,17 +3835,6 @@ 
ClangASTContext::GetTypeInfo(lldb::opaque_compiler_type_t type,
   ->getUnderlyingType()
   .getAsOpaquePtr())
.GetTypeInfo(pointee_or_element_clang_type);
-  case clang::Type::TypeOfExpr:
-return CompilerType(this, llvm::cast(qual_type)
-  ->getUnderlyingExpr()
-  ->getType()
-  .getAsOpaquePtr())
-.GetTypeInfo(pointee_or_element_clang_type);
-  case clang::Type::TypeOf:
-return CompilerType(this, llvm::cast(qual_type)
-  ->getUnderlyingType()
-  .getAsOpaquePtr())
-.GetTypeInfo(pointee_or_element_clang_type);
   case clang::Type::UnresolvedUsing:
 return 0;
 
@@ -3966,8 +3953,11 @@ 
ClangASTContext::GetTypeClass(lldb::opaque_compiler_type_t type) {
   switch (qual_type->getTypeClass()) {
   case clang::Type::Atomic:
   case clang::Type::Auto:
+  case clang::Type::Decltype:
   case clang::Type::Elaborated:
   case clang::Type::Paren:
+  case clang::Type::TypeOf:
+  case clang::Type::TypeOfExpr:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
@@ -4049,22 +4039,6 @@ 
ClangASTContext::GetTypeClass(lldb::opaque_compiler_type_t type) {
   case clang::Type::PackExpansion:
 break;
 
-  case clang::Type::TypeOfExpr:
-return CompilerType(this, llvm::cast(qual_type)
-  ->getUnderlyingExpr()
-  ->getType()
-  .getAsOpaquePtr())
-.GetTypeClass();
-  case clang::Type::TypeOf:
-return CompilerType(this, llvm::cast(qual_type)
-  ->getUnderlyingType()
-  .getAsOpaquePtr())
-.GetTypeClass();
-  case clang::Type::Decltype:
-return CompilerType(this, llvm::cast(qual_type)
-  ->getUnderlyingType()
-  .getAsOpaquePtr())
-.GetTypeClass();
   case clang::Type::TemplateSpecialization:
 break;
   case clang::Type::DeducedTemplateSpecialization:
@@ -4678,9 +4652,12 @@ lldb::Encoding 
ClangASTContext::GetEncoding(lldb::opaque_compiler_type_t type,
   switch (qual_type->getTypeClass()) {
   case clang::Type::Atomic:
   case clang::Type::Auto:
+  case clang::Type::Decltype:
   case clang::Type::Elaborated:
   case clang::T

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D71498#1785486 , @omjavaid wrote:

> error: Can't run the expression locally: Interpreter doesn't handle one of 
> the expression's opcodes


OK, I see it is a different error.  I will try to reproduce also that one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [PATCH] D71405: [lldb] Centralize desugaring of decltype-like types in ClangASTContext

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGea2805a04b65: [lldb] Centralize desugaring of decltype-like 
types in ClangASTContext (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D71405?vs=233556&id=234015#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71405

Files:
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/unittests/Symbol/TestClangASTContext.cpp

Index: lldb/unittests/Symbol/TestClangASTContext.cpp
===
--- lldb/unittests/Symbol/TestClangASTContext.cpp
+++ lldb/unittests/Symbol/TestClangASTContext.cpp
@@ -6,15 +6,14 @@
 //
 //===--===//
 
-#include "gtest/gtest.h"
-
-#include "clang/AST/DeclCXX.h"
-
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Symbol/ClangUtil.h"
 #include "lldb/Symbol/Declaration.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
+#include "gtest/gtest.h"
 
 using namespace clang;
 using namespace lldb;
@@ -447,3 +446,36 @@
 EXPECT_EQ(int_type, result->type);
   }
 }
+
+static QualType makeConstInt(clang::ASTContext &ctxt) {
+  QualType result(ctxt.IntTy);
+  result.addConst();
+  return result;
+}
+
+TEST_F(TestClangASTContext, TestGetTypeClassDeclType) {
+  clang::ASTContext &ctxt = *m_ast->getASTContext();
+  auto *nullptr_expr = new (ctxt) CXXNullPtrLiteralExpr(ctxt.NullPtrTy, SourceLocation());
+  QualType t = ctxt.getDecltypeType(nullptr_expr, makeConstInt(ctxt));
+  EXPECT_EQ(lldb::eTypeClassBuiltin, m_ast->GetTypeClass(t.getAsOpaquePtr()));
+}
+
+TEST_F(TestClangASTContext, TestGetTypeClassTypeOf) {
+  clang::ASTContext &ctxt = *m_ast->getASTContext();
+  QualType t = ctxt.getTypeOfType(makeConstInt(ctxt));
+  EXPECT_EQ(lldb::eTypeClassBuiltin, m_ast->GetTypeClass(t.getAsOpaquePtr()));
+}
+
+TEST_F(TestClangASTContext, TestGetTypeClassTypeOfExpr) {
+  clang::ASTContext &ctxt = *m_ast->getASTContext();
+  auto *nullptr_expr = new (ctxt) CXXNullPtrLiteralExpr(ctxt.NullPtrTy, SourceLocation());
+  QualType t = ctxt.getTypeOfExprType(nullptr_expr);
+  EXPECT_EQ(lldb::eTypeClassBuiltin, m_ast->GetTypeClass(t.getAsOpaquePtr()));
+}
+
+TEST_F(TestClangASTContext, TestGetTypeClassNested) {
+  clang::ASTContext &ctxt = *m_ast->getASTContext();
+  QualType t_base = ctxt.getTypeOfType(makeConstInt(ctxt));
+  QualType t = ctxt.getTypeOfType(t_base);
+  EXPECT_EQ(lldb::eTypeClassBuiltin, m_ast->GetTypeClass(t.getAsOpaquePtr()));
+}
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -2499,9 +2499,12 @@
   type = cast(type)->getValueType();
   break;
 case clang::Type::Auto:
+case clang::Type::Decltype:
 case clang::Type::Elaborated:
 case clang::Type::Paren:
 case clang::Type::Typedef:
+case clang::Type::TypeOf:
+case clang::Type::TypeOfExpr:
   type = type->getLocallyUnqualifiedSingleStepDesugaredType();
   break;
 default:
@@ -3765,11 +3768,6 @@
 return eTypeHasChildren | eTypeIsVector;
   case clang::Type::DependentTemplateSpecialization:
 return eTypeIsTemplate;
-  case clang::Type::Decltype:
-return CompilerType(this, llvm::cast(qual_type)
-  ->getUnderlyingType()
-  .getAsOpaquePtr())
-.GetTypeInfo(pointee_or_element_clang_type);
 
   case clang::Type::Enum:
 if (pointee_or_element_clang_type)
@@ -3837,17 +3835,6 @@
   ->getUnderlyingType()
   .getAsOpaquePtr())
.GetTypeInfo(pointee_or_element_clang_type);
-  case clang::Type::TypeOfExpr:
-return CompilerType(this, llvm::cast(qual_type)
-  ->getUnderlyingExpr()
-  ->getType()
-  .getAsOpaquePtr())
-.GetTypeInfo(pointee_or_element_clang_type);
-  case clang::Type::TypeOf:
-return CompilerType(this, llvm::cast(qual_type)
-  ->getUnderlyingType()
-  .getAsOpaquePtr())
-.GetTypeInfo(pointee_or_element_clang_type);
   case clang::Type::UnresolvedUsing:
 return 0;
 
@@ -3966,8 +3953,11 @@
   switch (qual_type->getTypeClass()) {
   case clang::Type::Atomic:
   case clang::Type::Auto:
+  case clang::Type::Decltype:
   case clang::Type::Elaborated:
   case clang::Type::Paren:
+  case clang::Type::TypeOf:
+  case clang::Type::TypeOfExpr:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
@@ -4049,22 +4039,6 @@
   case clang::Type::PackExpansion:
 b

[Lldb-commits] [lldb] 75e8a91 - [lldb][NFC] Remove all overloads of Copy/DeportType in ClangASTImporter

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-16T12:09:05+01:00
New Revision: 75e8a91cf84fce2432f70949ab9e353ff2322a5f

URL: 
https://github.com/llvm/llvm-project/commit/75e8a91cf84fce2432f70949ab9e353ff2322a5f
DIFF: 
https://github.com/llvm/llvm-project/commit/75e8a91cf84fce2432f70949ab9e353ff2322a5f.diff

LOG: [lldb][NFC] Remove all overloads of Copy/DeportType in ClangASTImporter

The overloads that don't take a CompilerType serve no purpose as we
always have a CompilerType in the scope where we call them. Instead
just call the overload that takes a CompilerType and delete the
now unused other overloaded methods.

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangASTImporter.h
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Symbol/ClangASTImporter.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTImporter.h 
b/lldb/include/lldb/Symbol/ClangASTImporter.h
index cd01bed624cc..58e068658ad8 100644
--- a/lldb/include/lldb/Symbol/ClangASTImporter.h
+++ b/lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -48,21 +48,12 @@ class ClangASTImporter {
   : m_file_manager(clang::FileSystemOptions(),
FileSystem::Instance().GetVirtualFileSystem()) {}
 
-  clang::QualType CopyType(clang::ASTContext *dst_ctx,
-   clang::ASTContext *src_ctx, clang::QualType type);
-
-  lldb::opaque_compiler_type_t CopyType(clang::ASTContext *dst_ctx,
-clang::ASTContext *src_ctx,
-lldb::opaque_compiler_type_t type);
-
   CompilerType CopyType(ClangASTContext &dst, const CompilerType &src_type);
 
   clang::Decl *CopyDecl(clang::ASTContext *dst_ctx, clang::ASTContext *src_ctx,
 clang::Decl *decl);
 
-  lldb::opaque_compiler_type_t DeportType(clang::ASTContext *dst_ctx,
-  clang::ASTContext *src_ctx,
-  lldb::opaque_compiler_type_t type);
+  CompilerType DeportType(ClangASTContext &dst, const CompilerType &src_type);
 
   clang::Decl *DeportDecl(clang::ASTContext *dst_ctx,
   clang::ASTContext *src_ctx, clang::Decl *decl);

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index 7a23606f57bc..58db9d4e7424 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -2022,9 +2022,8 @@ CompilerType ClangASTSource::GuardedCopyType(const 
CompilerType &src_type) {
   QualType copied_qual_type;
 
   if (m_ast_importer_sp) {
-copied_qual_type =
-m_ast_importer_sp->CopyType(m_ast_context, src_ast->getASTContext(),
-ClangUtil::GetQualType(src_type));
+copied_qual_type = ClangUtil::GetQualType(
+m_ast_importer_sp->CopyType(*m_clang_ast_context, src_type));
   } else if (m_merger_up) {
 copied_qual_type =
 CopyTypeWithMerger(*src_ast->getASTContext(), *m_merger_up,

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index b16c1815caa1..192e43a46061 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -206,10 +206,7 @@ TypeFromUser 
ClangExpressionDeclMap::DeportType(ClangASTContext &target,
   assert(source.getASTContext() == m_ast_context);
 
   if (m_ast_importer_sp) {
-return TypeFromUser(m_ast_importer_sp->DeportType(
-target.getASTContext(), source.getASTContext(),
-parser_type.GetOpaqueQualType()),
-&target);
+return TypeFromUser(m_ast_importer_sp->DeportType(target, parser_type));
   } else if (m_merger_up) {
 clang::FileID source_file =
 source.getASTContext()->getSourceManager().getFileID(

diff  --git a/lldb/source/Symbol/ClangASTImporter.cpp 
b/lldb/source/Symbol/ClangASTImporter.cpp
index a6125ce24f86..1cfec2531d8b 100644
--- a/lldb/source/Symbol/ClangASTImporter.cpp
+++ b/lldb/source/Symbol/ClangASTImporter.cpp
@@ -25,52 +25,42 @@
 using namespace lldb_private;
 using namespace clang;
 
-clang::QualType ClangASTImporter::CopyType(clang::ASTContext *dst_ast,
-   clang::ASTContext *src_ast,
-   clang::QualType type) {
-  ImporterDelegateSP delegate_sp(GetDelegate(dst_ast, src_ast));
+CompilerType ClangASTImporter::CopyType(ClangASTContext &dst_ast,
+const CompilerType &src_type) {
+  clang::ASTContext *dst_clang_

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

This issue is being caused by wrong address being written to memory somewhere 
while single stepping though i have reached the exact problem but the logs seem 
to suggest it.

LLDB Log without  D63540  
https://paste.ubuntu.com/p/zdXFrfN4MJ/

LLDB Log without  D63540 +D71498 
+D71514  
https://paste.ubuntu.com/p/RCcSdpYkRd/

Problem appears immediately after applying D63540 
 and has no effect even with remaining two 
patches.

**Look for Correct behavior log without any patches:

-

0xe4a8d0: tid = 0x8e07: stop info = run-to-address (stop_id = 7)
Turning off notification of new threads while single stepping a thread.

ThreadList::ShouldStop: 1 threads, 1 unsuspended threads
Thread::ShouldStop(0xe4a8d0) for tid = 0x8e07 0x8e07, pc = 0x000102f0
 Thread::ShouldStop Begin 
Plan stack initial state:

  thread #1: tid = 0x8e07:
Active plan stack:
  Element 0: Base thread plan.
  Element 1: Thread plan to call 0xf77d8ee8
  Element 2: Run to address: 0x000102f0 using breakpoint: -2 - 

th1/fr0 with pc value of 0x102f0, symbol name is '_start'
th1/fr0 frame uses EmulateInstructionARM for full UnwindPlan because this is 
the non-call site unwind plan and this is a zeroth frame
th1/fr0 0x000102f0: CFA=sp +0 =>

**vs the wrong behavior here:**

0x1cd38e8: tid = 0x90d4: stop info =  (stop_id = 7)
**0x1cd38e8: tid = 0x90d4: stop info = signal SIGSEGV: invalid address (fault 
address: 0xfe52) (stop_id = 7)
**Turning off notification of new threads while single stepping a thread.

ThreadList::ShouldStop: 1 threads, 1 unsuspended threads
Thread::ShouldStop(0x1cd38e8) for tid = 0x90d4 0x90d4, pc = 0xfe52
 Thread::ShouldStop Begin 
Plan stack initial state:

  thread #1: tid = 0x90d4:
Active plan stack:
  Element 0: Base thread plan.
  Element 1: Thread plan to call 0xf77d8ee8
  Element 2: Run to address: 0x000102f0 using breakpoint: -2 - 

th1/fr0 using architectural default unwind method
th1/fr0 with pc value of 0xfe52, no symbol/function name is known.
0x01CA20E0 Communication::Write (src = 0x5C600CE0, src_len = %llu) connection = 
26
0x1c7c968 ConnectionFileDescriptor::Write (src = 0x5c600ce0, src_len = 26)
0x1c7c968 ConnectionFileDescriptor::Write(fd = 5, src = 0x5c600ce0, src_len = 
26) => 26 (error = (null))
this = 0x01CA20E0, dst = 0xA37BD440, dst_len = 8192, timeout = 500 us, 
connection = 0x01C7C968
this = 0x01C7C968, timeout = 500 us
0x1c7c968 ConnectionFileDescriptor::Read()  fd = 5, dst = 0xa37bd440, dst_len = 
8192) => 24, error = (null)
th1/fr0 0xfe52: CFA=sp +0 => pc=lr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [lldb] 22caa3c - [lldb] Add unit test for ClangASTImporter

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-16T12:43:55+01:00
New Revision: 22caa3cfbcf5762a47acc40c425d9fe0c40da621

URL: 
https://github.com/llvm/llvm-project/commit/22caa3cfbcf5762a47acc40c425d9fe0c40da621
DIFF: 
https://github.com/llvm/llvm-project/commit/22caa3cfbcf5762a47acc40c425d9fe0c40da621.diff

LOG: [lldb] Add unit test for ClangASTImporter

Added: 
lldb/unittests/Symbol/TestClangASTImporter.cpp

Modified: 
lldb/unittests/Symbol/CMakeLists.txt

Removed: 




diff  --git a/lldb/unittests/Symbol/CMakeLists.txt 
b/lldb/unittests/Symbol/CMakeLists.txt
index aa86986f4e0e..02875b8b53c1 100644
--- a/lldb/unittests/Symbol/CMakeLists.txt
+++ b/lldb/unittests/Symbol/CMakeLists.txt
@@ -2,6 +2,7 @@ add_lldb_unittest(SymbolTests
   LocateSymbolFileTest.cpp
   PostfixExpressionTest.cpp
   TestClangASTContext.cpp
+  TestClangASTImporter.cpp
   TestDWARFCallFrameInfo.cpp
   TestType.cpp
   TestLineEntry.cpp

diff  --git a/lldb/unittests/Symbol/TestClangASTImporter.cpp 
b/lldb/unittests/Symbol/TestClangASTImporter.cpp
new file mode 100644
index ..17a0dfb6a348
--- /dev/null
+++ b/lldb/unittests/Symbol/TestClangASTImporter.cpp
@@ -0,0 +1,220 @@
+//===-- TestClangASTImporter.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 "gtest/gtest.h"
+
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/ClangASTImporter.h"
+#include "lldb/Symbol/ClangASTMetadata.h"
+#include "lldb/Symbol/ClangUtil.h"
+#include "lldb/Symbol/Declaration.h"
+#include "clang/AST/DeclCXX.h"
+
+using namespace clang;
+using namespace lldb;
+using namespace lldb_private;
+
+class TestClangASTImporter : public testing::Test {
+public:
+  static void SetUpTestCase() {
+FileSystem::Initialize();
+HostInfo::Initialize();
+  }
+
+  static void TearDownTestCase() {
+HostInfo::Terminate();
+FileSystem::Terminate();
+  }
+
+protected:
+  std::unique_ptr createAST() {
+return std::make_unique(HostInfo::GetTargetTriple());
+  }
+
+  CompilerType createRecord(ClangASTContext &ast, const char *name) {
+return ast.CreateRecordType(ast.getASTContext()->getTranslationUnitDecl(),
+lldb::AccessType::eAccessPublic, name, 0,
+lldb::LanguageType::eLanguageTypeC);
+  }
+};
+
+TEST_F(TestClangASTImporter, CanImportInvalidType) {
+  ClangASTImporter importer;
+  EXPECT_FALSE(importer.CanImport(CompilerType()));
+}
+
+TEST_F(TestClangASTImporter, ImportInvalidType) {
+  ClangASTImporter importer;
+  EXPECT_FALSE(importer.Import(CompilerType()));
+}
+
+TEST_F(TestClangASTImporter, CopyDeclTagDecl) {
+  // Tests that the ClangASTImporter::CopyDecl can copy TagDecls.
+  std::unique_ptr source_ast = createAST();
+  CompilerType source_type = createRecord(*source_ast, "Source");
+  clang::TagDecl *source = ClangUtil::GetAsTagDecl(source_type);
+
+  std::unique_ptr target_ast = createAST();
+
+  ClangASTImporter importer;
+  clang::Decl *imported = importer.CopyDecl(
+  target_ast->getASTContext(), source_ast->getASTContext(), source);
+  ASSERT_NE(nullptr, imported);
+
+  // Check that we got the correct decl by just comparing their qualified name.
+  clang::TagDecl *imported_tag_decl = llvm::cast(imported);
+  EXPECT_EQ(source->getQualifiedNameAsString(),
+imported_tag_decl->getQualifiedNameAsString());
+
+  // Check that origin was set for the imported declaration.
+  ClangASTImporter::DeclOrigin origin = importer.GetDeclOrigin(imported);
+  EXPECT_TRUE(origin.Valid());
+  EXPECT_EQ(origin.ctx, source_ast->getASTContext());
+  EXPECT_EQ(origin.decl, source);
+}
+
+TEST_F(TestClangASTImporter, CopyTypeTagDecl) {
+  // Tests that the ClangASTImporter::CopyType can copy TagDecls types.
+  std::unique_ptr source_ast = createAST();
+  CompilerType source_type = createRecord(*source_ast, "Source");
+  clang::TagDecl *source = ClangUtil::GetAsTagDecl(source_type);
+
+  std::unique_ptr target_ast = createAST();
+
+  ClangASTImporter importer;
+  CompilerType imported = importer.CopyType(*target_ast, source_type);
+  ASSERT_TRUE(imported.IsValid());
+
+  // Check that we got the correct decl by just comparing their qualified name.
+  clang::TagDecl *imported_tag_decl = ClangUtil::GetAsTagDecl(imported);
+  EXPECT_EQ(source->getQualifiedNameAsString(),
+imported_tag_decl->getQualifiedNameAsString());
+
+  // Check that origin was set for the imported declaration.
+  ClangASTImporter::DeclOrigin origin =
+  importer.GetDeclOrigin(imported_tag_decl);
+  EXPECT_TRUE(origin.Valid());
+  EXPE

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

Could you check symtabs what symbols are located at:

  th1/fr0 with pc value of 0x102f0, symbol name is '_start'

vs.

  th1/fr0 with pc value of 0xfe52, no symbol/function name is known.

? Or maybe just attached the main executable as `_start=0x102f0` is there I 
hope.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

Symbols sizes are being zeroed out

Correct without D63540 : 
https://pastebin.ubuntu.com/p/fKGxFfwyxV/

Incorrect with D63540 : 
https://pastebin.ubuntu.com/p/XBTScYYkhq/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [lldb] 755a66e - [lldb] Use file-based synchronization in TestVSCode_attach

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-16T14:10:42+01:00
New Revision: 755a66ebdeda38669f5498565cbc6af331b47bad

URL: 
https://github.com/llvm/llvm-project/commit/755a66ebdeda38669f5498565cbc6af331b47bad
DIFF: 
https://github.com/llvm/llvm-project/commit/755a66ebdeda38669f5498565cbc6af331b47bad.diff

LOG: [lldb] Use file-based synchronization in TestVSCode_attach

The is the best method we have at the moment for attach-style tests.

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
index ed014392f598..cb2ac355df53 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
@@ -79,19 +79,22 @@ def test_by_name(self):
 shutil.copyfile(orig_program, program)
 shutil.copymode(orig_program, program)
 
+# Use a file as a synchronization point between test and inferior.
+pid_file_path = lldbutil.append_to_process_working_directory(self,
+"pid_file_%d" % (int(time.time(
+
 def cleanup():
 if os.path.exists(program):
 os.unlink(program)
+self.run_platform_command("rm %s" % (pid_file_path))
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
 
-self.process = subprocess.Popen([program],
-stdin=subprocess.PIPE,
-stdout=subprocess.PIPE,
-stderr=subprocess.PIPE)
-# Wait for a bit to ensure the process is launched, but not for so long
-# that the process has already finished by the time we attach.
-time.sleep(3)
+popen = self.spawnSubprocess(program, [pid_file_path])
+self.addTearDownHook(self.cleanupSubprocesses)
+
+pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
+
 self.attach(program=program)
 self.set_and_hit_breakpoint(continueToExit=True)
 
@@ -143,7 +146,7 @@ def test_commands(self):
 # and use it for debugging
 attachCommands = [
 'target create -d "%s"' % (program),
-'process launch -- arg1'
+'process launch'
 ]
 initCommands = ['target list', 'platform list']
 preRunCommands = ['image list a.out', 'image dump sections a.out']

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c
index 4f50f7546155..64d86583ada6 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c
@@ -1,11 +1,20 @@
 #include 
 #include 
 
-int main(int argc, char const *argv[])
-{
-lldb_enable_attach();
+int main(int argc, char const *argv[]) {
+  lldb_enable_attach();
 
-printf("pid = %i\n", getpid());
-sleep(10);
-return 0; // breakpoint 1
+  if (argc >= 2) {
+// Create the synchronization token.
+FILE *f = fopen(argv[1], "wx");
+if (!f)
+  return 1;
+fputs("\n", f);
+fflush(f);
+fclose(f);
+  }
+
+  printf("pid = %i\n", getpid());
+  sleep(10);
+  return 0; // breakpoint 1
 }



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

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Seems like it might be nice to have a macro defined in a LLDB header file like 
lldb-defines.h. Something like:

  #define PTR_TO_U64(x) ((uint64_t)(uintptr_t)(x))

Then we can make a unit test to verify it works on all systems. Seem like this 
issue will pop up all over the place in the LLDB code base.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

> In other places you're replacing a reinterpret_cast with two c casts. 
> I guess this was done because two c++ casts were too long (?). In these cases 
> the second cast is not really needed, as uintptr_t->addr_t should convert 
> automatically. I think I'd prefer that we just have a single 
> reinterpret_cast instead of two c casts. Then our rule can be 
> "always convert a pointer to uintptr_t". I don't know if there's a clang-tidy 
> check for that, but it sounds like that could be something which could be 
> checked/enforced there...

For the printf style statement, we can't use just one cast to "uintptr_t" 
because on 32 bit systems it won't be converted to 64 bit. If we switch for 
LLDB_LOG() that uses the llvm formats, we are good, but not with printf




Comment at: lldb/source/Expression/IRExecutionUnit.cpp:351-352
+m_jitted_functions.push_back(
+JittedFunction(function.getName().str().c_str(), external,
+   (lldb::addr_t)(uintptr_t)fun_ptr));
   }

This can probably just be:

```
 JittedFunction(function.getName().str().c_str(), external, 
(uintptr_t)fun_ptr));
```
since this is a function call and "fun_ptr" will be correctly converted to a 
lldb::addr_t


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D71498#1786319 , @clayborg wrote:

> For the printf style statement, we can't use just one cast to "uintptr_t" 
> because on 32 bit systems it won't be converted to 64 bit.


That pointer-to-`uint64_t` is now used for `printf` with `PRIx64` so if we use 
pointer-to-`uintptr_t` we could use `PRIxPTR`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

As I am reading this, I just wanted to send out a note of something else that 
can cause crashes in ARM/Thumb code. For anyone working with ARM/Thumb on 
systems that don't use the ARM and Thumb BKPT instruction when setting software 
breakpoints (like all lldb linux and android flavors IIRC): if you try to 
overwrite a 32 bit thumb instruction that is a conditional instruction in a 
Thumb IT instruction with a 16 bit trap or illegal instruction you can crash 
your program. The issue arises for code like:

  0x1000: xx xx ITTTEE
  0x1002: 00 11 22 33   32 bit thumb instruction (if condition)
  0x1006: 44 55 66 77   32 bit thumb instruction (if condition)
  0x100a: 88 99 aa bb   32 bit thumb instruction (else condition) 
  0x100e: cc dd ee ff   32 bit thumb instruction (else condition)

If you try to set a breakpoint at any of the instructions in [0x1002-0x100e) 
using a 16 bit trap or illegal instruction (I use "bb bb" below for this trap 
for example purposes), you change the size of the instructions and which 
instructions are conditional. If we try to write "bb bb" to 0x1002 we now have:

  0x1000: xx xx ITTTEE
  0x1002: bb bb (if condition) the first conditional instruction is now 
16 bit instead of 32 bit
  0x1004: 22 33 44 55   (if condition) this has the last half of the previous 
instruction 
  0x1008: 66 77 88 99   (else condition) this has the last half of the previous 
instruction 
  0x100c: aa bb (else condition) this has the last half of the previous 
instruction 
  0x100e: cc dd ee ff   32 bit thumb instruction (NOT conditional anymore)

This will work if using the BKPT instruction only. Sorry for the noise if 
lldb-server is already using the BKPT instruction. But I just wanted to throw 
this out there in case this issue if affecting anyone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D71498#1786343 , @jankratochvil 
wrote:

> In D71498#1786319 , @clayborg wrote:
>
> > For the printf style statement, we can't use just one cast to "uintptr_t" 
> > because on 32 bit systems it won't be converted to 64 bit.
>
>
> That pointer-to-`uint64_t` is now used for `printf` with `PRIx64` so if we 
> use pointer-to-`uintptr_t` we could use `PRIxPTR`.


that will work too!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498



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

[Lldb-commits] [PATCH] D71562: [lldb] Remove modern-type-lookup

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

@martong I added you as a reviewer because I assume you were interested in that 
development. I'm curious what you think should happen to the clang-import-test. 
We could either rewrite the tests as unit tests in the ASTImporterTest you guys 
are already using or we move the necessary parts of the ExternalASTMerger into 
the clang-import-test (which will mostly likely just be the lookup code). I 
leave it up to you guys to decide what should happen with that testing code.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D71562



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

[Lldb-commits] [PATCH] D71562: [lldb] Remove modern-type-lookup

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: shafik, martong.
Herald added subscribers: lldb-commits, usaxena95, JDevlieghere, arphaman, 
christof, rnkovacs.
Herald added a project: LLDB.
teemperor added a comment.

@martong I added you as a reviewer because I assume you were interested in that 
development. I'm curious what you think should happen to the clang-import-test. 
We could either rewrite the tests as unit tests in the ASTImporterTest you guys 
are already using or we move the necessary parts of the ExternalASTMerger into 
the clang-import-test (which will mostly likely just be the lookup code). I 
leave it up to you guys to decide what should happen with that testing code.


As discussed on the mailing list [1] we have to make a decision for how to 
proceed with the modern-type-lookup.

This patch removes modern-type-lookup from LLDB. This just removes all the code 
behind the modern-type-lookup
setting but it does *not* remove any code from Clang (i.e., the 
ExternalASTMerger and the clang-import-test stay around
for now).

The motivation for this is that I don't think that the current approach of 
implementing modern-type-lookup
will work out. Especially creating a completely new lookup system behind some 
setting that is never turned on by anyone
and then one day make one big switch to the new system seems wrong. It doesn't 
fit into the way LLVM is developed and has
so far made the transition work much more complicated than it has to be.

A lot of the benefits that were supposed to come with the modern-type-lookup 
are related to having a better organization
in the way types move across LLDB and having less dependencies on unrelated 
LLDB code. By just looking at the current code (mostly
the ClangASTImporter) I think we can reach the same goals by just incrementally 
cleaning up, documenting, refactoring
and actually testing the existing code we have.

[1] http://lists.llvm.org/pipermail/lldb-dev/2019-December/015831.html


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D71562

Files:
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/include/lldb/Target/Target.h
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic-objc/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic-objc/TestBasicObjcModernTypeLookup.py
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic-objc/main.m
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic/TestBasicModernTypeLookup.py
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/objc-modules/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/objc-modules/TestObjModulesModernTypeLookup.py
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/objc-modules/main.m
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -4,9 +4,6 @@
   def InjectLocalVars : Property<"inject-local-vars", "Boolean">,
 Global, DefaultTrue,
 Desc<"If true, inject local variables explicitly into the expression text. This will fix symbol resolution when there are name collisions between ivars and local variables. But it can make expressions run much more slowly.">;
-  def UseModernTypeLookup : Property<"use-modern-type-lookup", "Boolean">,
-Global, DefaultFalse,
-Desc<"If true, use Clang's modern type lookup infrastructure.">;
 }
 
 let Definition = "target" in {
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3545,18 +3545,6 @@
 true);
 }
 
-bool TargetProperties::GetUseModernTypeLookup() con

[Lldb-commits] [PATCH] D71440: Extending step-over range past calls was causing deadlocks, fix that.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D71440#1783197 , @labath wrote:

> This looks like a tricky bug. Thanks for tracking it down.
>
> I have two questions though: :D
>
> - could you use c++11 locking primitives in the test? pthreads do not work on 
> windows


I copied the test from somewhere else, so I'll have to change the original too. 
 Let me give this a go.

> - what exactly will be the effect of this? Will we start all threads 
> immediately, or will we do the usual dance of trying to run one thread for a 
> short period of time, and then resume all of them if that times out? If it's 
> the latter, I'm wondering if we really need this call detection logic -- we 
> could theoretically just use the "run all" method all the time (except if 
> explicitly disabled), with the knowledge that all reasonable call-less 
> sequences will finish before the "run single thread" timeout expires. And it 
> would automatically also handle weird call-less sequences with spinlocks or 
> what not...

Currently, the actually stepping plans don't do "try a bit on one thread, then 
resume all".  They are conservative, and will let all threads run anytime they 
run "arbitrary" code.  It's only running expressions that does the one and then 
many dance.

The original idea was that the "Process::RunThreadPlan" would be used for all 
the thread plans that might stall if run on only one thread.  But before it got 
wired up for that it became dedicated to expression evaluation.  It's more 
important to try to stay on one thread for expressions, since the user stopped 
the process somehow and has a reasonable expectation that it will stay stopped. 
 But as a side effect, I never got around to wiring that dance up to the 
regular stepping.

However, since I've been playing around with this trick for expression 
evaluation I'm a little less sure I want to extend it to regular operation.  
OTOH, that WOULD fix problems with stepping over call-less spinlocks.  More 
importantly - since I've never had any reports of that theoretical problem 
being an actual problem - we would get better at keeping stepping operations on 
one thread.

OTOH, interrupting a process is not cost-free.  We have to send a signal to 
interrupt it and that can cause EINTR's in places people weren't expecting.  
This has caused problems in the past, where the interrupt from running an 
expression will cause a read call to raise an unhandled EINTR.  Of course you 
should always check for EINTR etc so these probably are actually bugs in code, 
and you'd think people would thank me for surfacing the bug.  But when I point 
that out it doesn't seem to mollify the users who have reported the problem.

This is a common enough "mis-pattern" that I am hesitant to add a stepping 
method that would really spam the inferior with interrupts.  I think our 
current method is the best compromise between trying to keep focus on the 
active thread, and changing normal flow of execution as little as possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71440



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

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Johannes Altmanninger via Phabricator via lldb-commits]

johannes updated this revision to Diff 234134.
johannes retitled this revision from "[LLDB] Fix address computation for member 
function linked with lld" to "[LLDB] Fix address computation for inline 
function".
johannes edited the summary of this revision.
johannes added a comment.

different approach (WIP): ignore functions with DW_AT_low_pc = 0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71487

Files:
  lldb/include/lldb/Core/Module.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/Expr/Inputs/function-address-lib.cpp
  lldb/test/Shell/Expr/Inputs/function-address-main.cpp
  lldb/test/Shell/Expr/Inputs/function-address-shared.h
  lldb/test/Shell/Expr/TestFunctionAddress.lldb

Index: lldb/test/Shell/Expr/TestFunctionAddress.lldb
===
--- /dev/null
+++ lldb/test/Shell/Expr/TestFunctionAddress.lldb
@@ -0,0 +1,9 @@
+# RUN: %clangxx_host %p/Inputs/function-address-main.cpp %p/Inputs/function-address-lib.cpp -g -fuse-ld=lld -o %t
+# RUN: %lldb %t -s %s 2>&1 | FileCheck %s
+b main
+run
+next
+expr argv.four()
+# CHECK: expr argv.four()
+# CHECK-NEXT: (int) $0 = 4
+# CHECK-NOT: SIGSEGV
Index: lldb/test/Shell/Expr/Inputs/function-address-shared.h
===
--- /dev/null
+++ lldb/test/Shell/Expr/Inputs/function-address-shared.h
@@ -0,0 +1,23 @@
+#ifndef function_address_shared_h
+#define function_address_shared_h
+namespace llvm_namespace {
+struct TinyVectorBase {
+  int SomeInlineInitializedMember = 0;
+};
+struct TinyVector : TinyVectorBase {
+  TinyVector *Self;
+
+  TinyVector() : Self(getFirstEl()) {}
+  TinyVector(bool) : TinyVector() { nop(four()); }
+  int four() const { return 4; }
+  TinyVector *getFirstEl() { return this; }
+  char *begin() { return nullptr; }
+  char *end() { return begin() + four(); }
+  void clear() { nop(begin()), nop(end()); }
+
+  void nop(void *) {}
+  void nop(int) {}
+};
+} // end namespace llvm_namespace
+using llvm_namespace::TinyVector;
+#endif // function_address_shared_h
Index: lldb/test/Shell/Expr/Inputs/function-address-main.cpp
===
--- /dev/null
+++ lldb/test/Shell/Expr/Inputs/function-address-main.cpp
@@ -0,0 +1,4 @@
+#include "function-address-shared.h"
+int main() {
+TinyVector argv(true);
+}
Index: lldb/test/Shell/Expr/Inputs/function-address-lib.cpp
===
--- /dev/null
+++ lldb/test/Shell/Expr/Inputs/function-address-lib.cpp
@@ -0,0 +1,4 @@
+#include "function-address-shared.h"
+void (*ErrorHandler)();
+void LLVMErrorHandler() { TinyVector().clear(); }
+void SetErrorHandler() { ErrorHandler = LLVMErrorHandler; }
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2355,6 +2355,9 @@
   std::vector dies;
   m_index->GetFunctions(name, *this, *parent_decl_ctx, name_type_mask, dies);
   for (const DWARFDIE &die : dies) {
+if (die.GetAttributeValueAsAddress(DW_AT_low_pc, /*fail_value=*/1) == 0 &&
+!GetObjectFile()->GetModule()->HaveSectionAtAddressZero())
+  continue;
 if (resolved_dies.insert(die.GetDIE()).second)
   ResolveFunction(die, include_inlines, sc_list);
   }
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1256,10 +1256,21 @@
 ObjectFile *obj_file = GetObjectFile();
 if (obj_file != nullptr)
   obj_file->CreateSections(*GetUnifiedSectionList());
+for (const auto &Section : *m_sections_up) {
+if (Section->GetFileOffset() == 0) {
+m_have_section_at_address_zero = true;
+break;
+}
+}
   }
   return m_sections_up.get();
 }
 
+bool Module::HaveSectionAtAddressZero() {
+GetSectionList();
+return m_have_section_at_address_zero;
+}
+
 void Module::SectionFileAddressesChanged() {
   ObjectFile *obj_file = GetObjectFile();
   if (obj_file)
Index: lldb/include/lldb/Core/Module.h
===
--- lldb/include/lldb/Core/Module.h
+++ lldb/include/lldb/Core/Module.h
@@ -859,6 +859,8 @@
   /// Update the ArchSpec to a more specific variant.
   bool MergeArchitecture(const ArchSpec &arch_spec);
 
+  bool HaveSectionAtAddressZero();
+
   /// \class LookupInfo Module.h "lldb/Core/Module.h"
   /// A class that encapsulates name lookup information.
   ///
@@ -977,6 +979,7 @@
   std::atomic m_did_load_objfile{false};
   std::atomic m_did_load_symfile{false};
   std::atomic m_did_set_uuid{false};
+  bool m_have_section_at_

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Johannes Altmanninger via Phabricator via lldb-commits]

johannes added a comment.

Thanks for the very useful feedback! Now I can finally see why this is 
happening.

Below is the relevant excerpt of the input DWARF:
one of the DW_AT_low_pc attributes is NULL, which does not happen when I link 
with GNU ld.

  clang lldb/test/Shell/Expr/Inputs/function-address-{main,lib}.cpp -g 
-fuse-ld=lld -o - |
  llvm-dwarfdump -debug-info /dev/stdin
  
  0x0203:   DW_TAG_subprogram
  DW_AT_low_pc  (0x00201940)
  DW_AT_high_pc (0x0020194f)
  DW_AT_frame_base  (DW_OP_reg6 RBP)
  DW_AT_object_pointer  (0x021a)
  DW_AT_specification   (0x0069 
"_ZNK14llvm_namespace10TinyVector4fourEv")
  
  0x021a: DW_TAG_formal_parameter
DW_AT_location  (DW_OP_fbreg -8)
DW_AT_name  ("this")
DW_AT_type  (0x027a "const TinyVector*")
DW_AT_artificial(true)
  
  0x0226: NULL
  
  0x053a:   DW_TAG_subprogram
  DW_AT_low_pc  (0x)
  DW_AT_high_pc (0x000f)
  DW_AT_frame_base  (DW_OP_reg6 RBP)
  DW_AT_object_pointer  (0x0551)
  DW_AT_specification   (0x0309 
"_ZNK14llvm_namespace10TinyVector4fourEv")
  
  0x0551: DW_TAG_formal_parameter
DW_AT_location  (DW_OP_fbreg -8)
DW_AT_name  ("this")
DW_AT_type  (0x0563 "const TinyVector*")
DW_AT_artificial(true)
  
  0x055d: NULL

I think I found how GDB handles this case:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/dwarf2read.c;h=ecfae68427716333ab060ef1c44b3a25df9d7ac0;hb=HEAD#l14756

Now I have a rough implementation of the same idea.
I can probably come up with a unit test in `SymbolFileDWARFTests` that
checks that the function address is correct after parsing above DWARF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71487



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

[Lldb-commits] [lldb] 3fbe518 - [lldb] Respect previously set values of LLDB_TABLEGEN_EXE

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2019-12-16T14:31:42-08:00
New Revision: 3fbe518a102a344abbd837e364a404c0c695d183

URL: 
https://github.com/llvm/llvm-project/commit/3fbe518a102a344abbd837e364a404c0c695d183
DIFF: 
https://github.com/llvm/llvm-project/commit/3fbe518a102a344abbd837e364a404c0c695d183.diff

LOG: [lldb] Respect previously set values of LLDB_TABLEGEN_EXE

If you set LLDB_TABLEGEN_EXE in a CMake cache file or in the CMake
invocation line, your setting isn't respected. Setting up the tablegen
for the host will overwrite the value that we set LLDB_TABLEGEN_EXE to,
which defeats the whole point of setting it in the first place.

Added: 


Modified: 
lldb/utils/TableGen/CMakeLists.txt

Removed: 




diff  --git a/lldb/utils/TableGen/CMakeLists.txt 
b/lldb/utils/TableGen/CMakeLists.txt
index 2e8aec1770af..47a6400b4287 100644
--- a/lldb/utils/TableGen/CMakeLists.txt
+++ b/lldb/utils/TableGen/CMakeLists.txt
@@ -1,16 +1,18 @@
 # tablegen targets get exported via llvm for LLVMConfig.cmake. So standalone
 # builds of lldb can potentially import this via LLVMConfig and also attempt to
 # build it in tree. So only build it if it doesn't exist.
-if (TARGET lldb-tblgen)
-  set(LLDB_TABLEGEN_EXE $ CACHE STRING "")
-else()
-  set(LLVM_LINK_COMPONENTS Support)
+if (NOT DEFINED LLDB_TABLEGEN_EXE)
+  if (TARGET lldb-tblgen)
+set(LLDB_TABLEGEN_EXE $ CACHE STRING "")
+  else()
+set(LLVM_LINK_COMPONENTS Support)
 
-  add_tablegen(lldb-tblgen LLDB
-LLDBOptionDefEmitter.cpp
-LLDBPropertyDefEmitter.cpp
-LLDBTableGen.cpp
-LLDBTableGenUtils.cpp
-)
-  set_target_properties(lldb-tblgen PROPERTIES FOLDER "LLDB tablegenning")
+add_tablegen(lldb-tblgen LLDB
+  LLDBOptionDefEmitter.cpp
+  LLDBPropertyDefEmitter.cpp
+  LLDBTableGen.cpp
+  LLDBTableGenUtils.cpp
+  )
+set_target_properties(lldb-tblgen PROPERTIES FOLDER "LLDB tablegenning")
+  endif()
 endif()



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add initial support for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev created this revision.
paolosev added reviewers: jasonmolenda, clayborg.
paolosev added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits, sunfish, aheejin, 
jgravelle-google, sbc100, aprantl, mgorny, dschuff.

Current versions of Clang emit (partial) DWARF debug information in WebAssembly 
modules and we can leverage this debug information to give LLDB the ability to 
do source-level debugging of Wasm code that runs in a WebAssembly engine.

A way to do this could be to use the remote debugging functionalities provided 
by LLDB via the GDB-remote protocol. Remote debugging can indeed be useful not 
only to connect a debugger to a process running on a remote machine, but also to
connect the debugger to a managed VM or script engine that runs locally, 
provided that the engine implements a GDB-remote stub that offers the ability 
to access the engine runtime internal state.

To make this work, the GDB-remote protocol would need to be extended with a few 
Wasm-specific custom query commands, used to access aspects of the Wasm engine 
state (like the Wasm memory, Wasm local and global variables, and so on).
Furthermore, the DWARF format would need to be enriched with a few 
Wasm-specific extensions, here detailed: 
https://yurydelendik.github.io/webassembly-dwarf.

This CL only introduces classes ObjectFileWasm, SymbolVendorWasm and 
DynamicLoaderWasmDYLD as a first step to enable LLDB debugging of WebAssembly 
targets. In more detail:

- Class **ObjectFileWasm** represents a WebAssembly module loaded in the 
debuggee process. It knows how to parse Wasm module and store the Code section 
and the DWARF-specific sections.

- Class **SymbolVendorWasm** takes care of DWARF data for a ObjectFileWasm, 
which can be  either embedded in the modules, or stripped into a separate Wasm 
module which only contains DWARF sections.

- In WebAssembly, linear memory is disjointed from code space. The VM can load 
multiple instances of a module, which logically share the same code.

Class **DynamicLoaderWasmDYLD** manages the mapping of Wasm code address in the 
debuggee address space.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/WASM/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/WASM/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/WASM/SymbolVendorWasm.cpp
  lldb/source/Plugins/SymbolVendor/WASM/SymbolVendorWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/unittests/ObjectFile/CMakeLists.txt
  lldb/unittests/ObjectFile/WASM/CMakeLists.txt
  lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp
  llvm/include/llvm/BinaryFormat/ELF.h

Index: llvm/include/llvm/BinaryFormat/ELF.h
===
--- llvm/include/llvm/BinaryFormat/ELF.h
+++ llvm/include/llvm/BinaryFormat/ELF.h
@@ -311,6 +311,7 @@
   EM_RISCV = 243, // RISC-V
   EM_LANAI = 244, // Lanai 32-bit processor
   EM_BPF = 247,   // Linux kernel bpf virtual machine
+  EM_WASM = 248,  // WebAssembly
 };
 
 // Object file classes.
Index: lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp
===
--- /dev/null
+++ lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp
@@ -0,0 +1,282 @@
+//===-- TestObjectFileWasm.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 "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/WASM/ObjectFileWasm.h"
+#include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+class ObjectFileWasmTest : public testing::Test {
+public:
+  void Se

[Lldb-commits] [PATCH] D71575: [LLDB] Add initial support for WebAssembly debugging

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Sounds exciting. My comments are all about formatting and coding style, if 
someone has something technical to say, too that would be appreciated.




Comment at: 
lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp:1
+//===-- DynamicLoaderWasmDYLD.cpp *- C++
+//-*-===//

1. This hangs over the line
2. a -*- C++ -*- is only necessary for .h files where C vs. C++ is ambiguous




Comment at: 
lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp:77
+// This method should be called for each Wasm module loaded in the debuggee,
+// with base_addr = 0x{module_id}| (for example 0x0001).
+

This should be a doxygen comment.



Comment at: 
lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp:132
+  // offset in the Wasm module.
+  if (section_sp->GetName() == "code") {
+if (m_process->GetTarget().SetSectionLoadAddress(

Can you convert this to early exits? The deep nesting makes the control flow 
difficult to read.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:1
+//===-- ObjectFileWasm.cpp  -*- C++ -*-===//
+//

ditto



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:32
+const char *magic = reinterpret_cast(data_sp->GetBytes());
+if (strncmp(magic, llvm::wasm::WasmMagic, sizeof(llvm::wasm::WasmMagic)) !=
+0) {

Is a StringRef comparison easier to read here?



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:72
+  assert(data_sp);
+  if (!ValidateModuleHeader(data_sp)) {
+return nullptr;

Do you want any form error logging/handling here?



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:76
+
+  // Update the data to contain the entire file if it doesn't already
+  if (data_sp->GetByteSize() < length) {

Please use full sentences in comments with a trailing `.`.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:99
+ addr_t header_addr) {
+  if (ValidateModuleHeader(data_sp)) {
+std::unique_ptr objfile_up(

early-exitify?



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:150
+  // - the actual contents.
+  if (GetVaruint7(section_header_data, &offset, §ion_id) &&
+  GetVaruint32(section_header_data, &offset, &payload_len)) {

Again early exits would make this easier to read.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:394
+// These 64-bit addresses will be used to request code ranges for a specific
+// module from the WebAssembly engine.
+

doxygen comment.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.h:46
+
+  // PluginInterface protocol
+  ConstString GetPluginName() override { return GetPluginNameStatic(); }

FYI you can group doxygen comments like this:
```
/// PluginInterface protocol
/// \{
...
/// \}
```



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.h:102
+
+  static bool GetVaruint7(DataExtractor §ion_header_data,
+  lldb::offset_t *offset_ptr, uint8_t *result);

`llvm::Optional GetVaruint7()`



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.h:104
+  lldb::offset_t *offset_ptr, uint8_t *result);
+  static bool GetVaruint32(DataExtractor §ion_header_data,
+   lldb::offset_t *offset_ptr, uint32_t *result);

same here



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.h:119
+
+  void DumpSectionHeader(Stream *s, const section_info_t &sh);
+  void DumpSectionHeaders(Stream *s);

doxygen comments on all (most) non-override function would be useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add initial support for WebAssembly debugging

[Sam Clegg via Phabricator via lldb-commits]

sbc100 added a comment.

I don't know the lldb codebase, but from a webassembly perspective this looks 
promising.

I suppose we are long way from having a webassebly VM that exports that correct 
wire protocol to actually test this?




Comment at: lldb/include/lldb/Utility/ArchSpec.h:191
 
+eCore_wasm32,
 kNumCores,

Add another newline below to the follow the existing grouping pattern?



Comment at: lldb/source/API/SystemInitializerFull.cpp:73
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+#include "Plugins/ObjectFile/WASM/ObjectFileWasm.h"
 #include "Plugins/OperatingSystem/Python/OperatingSystemPython.h"

Can you name this directory "wasm" rather than "WASM" since its not an acronym. 
  



Comment at: lldb/source/API/SystemInitializerFull.cpp:179
   ObjectFilePECOFF::Initialize();
+  wasm::ObjectFileWasm::Initialize();
 

Why is the namespace needed here for wasm but not the other three above.. seems 
inconsistent.



Comment at: 
lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.h:1
+//===-- DynamicLoaderWasmDYLD.h --*- C++ -*-===//
+//

Again, avoid WASM in the directory name.   I suppose "Wasm" would also be 
acceptable, but I'm trying to push for "wasm" or "WebAssembly".



Comment at: lldb/source/Utility/ArchSpec.cpp:107
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm, ArchSpec::eCore_arm_armv8l,
+ "armv8l"},
 {eByteOrderLittle, 4, 4, 4, llvm::Triple::aarch64_32,

Is this just clang format being greedy?



Comment at: lldb/unittests/ObjectFile/WASM/CMakeLists.txt:11
+  )
+

trailing newline



Comment at: lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp:2
+//===-- TestObjectFileWasm.cpp ---*- C++
+//-*-===//
+//

somethings up with the ling wrapping here.



Comment at: llvm/include/llvm/BinaryFormat/ELF.h:314
   EM_BPF = 247,   // Linux kernel bpf virtual machine
+  EM_WASM = 248,  // WebAssembly
 };

This seems like an odd place to add this, given that are not using or relying 
on ELF anywhere.  Does this make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [lldb] 434905b - Run all threads when extending a next range over a call.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2019-12-16T17:45:21-08:00
New Revision: 434905b97d961531286d4b49c7ee1969f7cbea0e

URL: 
https://github.com/llvm/llvm-project/commit/434905b97d961531286d4b49c7ee1969f7cbea0e
DIFF: 
https://github.com/llvm/llvm-project/commit/434905b97d961531286d4b49c7ee1969f7cbea0e.diff

LOG: Run all threads when extending a next range over a call.

If you don't do this you end up running arbitrary code with
only one thread allowed to run, which can cause deadlocks.



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

Added: 

lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.cpp
lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/Makefile

lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/TestStepOverDoesntBlock.py
lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/locking.cpp

Modified: 
lldb/include/lldb/Core/Disassembler.h
lldb/include/lldb/Target/ThreadPlanStepRange.h
lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile

lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/TestExprDoesntBlock.py
lldb/source/Core/Disassembler.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/ThreadPlanStepRange.cpp

Removed: 

lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.c



diff  --git a/lldb/include/lldb/Core/Disassembler.h 
b/lldb/include/lldb/Core/Disassembler.h
index ba9ca87832f6..7ece0eeb708c 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -287,6 +287,10 @@ class InstructionList {
   /// a function call (a branch that calls and returns to the next
   /// instruction). If false, find the instruction index of any 
   /// branch in the list.
+  /// 
+  /// @param[out] found_calls
+  /// If non-null, this will be set to true if any calls were found in 
+  /// extending the range.
   ///
   /// @return
   /// The instruction index of the first branch that is at or past
@@ -295,7 +299,8 @@ class InstructionList {
   //--
   uint32_t GetIndexOfNextBranchInstruction(uint32_t start,
Target &target,
-   bool ignore_calls) const;
+   bool ignore_calls,
+   bool *found_calls) const;
 
   uint32_t GetIndexOfInstructionAtLoadAddress(lldb::addr_t load_addr,
   Target &target);

diff  --git a/lldb/include/lldb/Target/ThreadPlanStepRange.h 
b/lldb/include/lldb/Target/ThreadPlanStepRange.h
index 93d54ad7dfd5..28209623a1e1 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepRange.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepRange.h
@@ -76,6 +76,12 @@ class ThreadPlanStepRange : public ThreadPlan {
   lldb::BreakpointSP m_next_branch_bp_sp;
   bool m_use_fast_step;
   bool m_given_ranges_only;
+  bool m_found_calls = false; // When we set the next branch breakpoint for
+  // step over, we now extend them past call insns
+  // that directly return.  But if we do that we
+  // need to run all threads, or we might cause
+  // deadlocks.  This tells us whether we found
+  // any calls in setting the next branch 
breakpoint.
 
 private:
   std::vector m_instruction_ranges;

diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile 
b/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile
index f63adb4f5d2a..ee0d4690d834 100644
--- 
a/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile
@@ -1,4 +1,4 @@
-C_SOURCES := locking.c
+CXX_SOURCES := locking.cpp
 ENABLE_THREADS := YES
 
 include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/TestExprDoesntBlock.py
 
b/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/TestExprDoesntBlock.py
index 5b5042b63e4b..d7d963390b05 100644
--- 
a/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/TestExprDoesntBlock.py
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/TestExprDoesntBlock.py
@@ -16,9 +16,6 @@ class ExprDoesntDeadlockTestCase(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr17946')
-@expectedFailureAll(
-oslist=["windows"],
-bugnumber="Windows doesn't have pthreads, test needs to be ported")
 @add_test_categories(["basic_process"])
 def test_with_run_com

[Lldb-commits] [PATCH] D71440: Extending step-over range past calls was causing deadlocks, fix that.

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG434905b97d96: Run all threads when extending a next range 
over a call. (authored by jingham).

Changed prior to commit:
  https://reviews.llvm.org/D71440?vs=234206&id=234207#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71440

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Target/ThreadPlanStepRange.h
  lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/TestExprDoesntBlock.py
  lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.c
  
lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.cpp
  lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/Makefile
  
lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/TestStepOverDoesntBlock.py
  lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/locking.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/ThreadPlanStepRange.cpp

Index: lldb/source/Target/ThreadPlanStepRange.cpp
===
--- lldb/source/Target/ThreadPlanStepRange.cpp
+++ lldb/source/Target/ThreadPlanStepRange.cpp
@@ -238,8 +238,18 @@
 }
 
 bool ThreadPlanStepRange::StopOthers() {
-  return (m_stop_others == lldb::eOnlyThisThread ||
-  m_stop_others == lldb::eOnlyDuringStepping);
+  switch (m_stop_others) {
+  case lldb::eOnlyThisThread:
+return true;
+  case lldb::eOnlyDuringStepping:
+// If there is a call in the range of the next branch breakpoint,
+// then we should always run all threads, since a call can execute
+// arbitrary code which might for instance take a lock that's held
+// by another thread.
+return !m_found_calls;
+  case lldb::eAllThreads:
+return false;
+  }
 }
 
 InstructionList *ThreadPlanStepRange::GetInstructionsForAddress(
@@ -292,6 +302,7 @@
 GetTarget().RemoveBreakpointByID(m_next_branch_bp_sp->GetID());
 m_next_branch_bp_sp.reset();
 m_could_not_resolve_hw_bp = false;
+m_found_calls = false;
   }
 }
 
@@ -305,6 +316,9 @@
   if (!m_use_fast_step)
 return false;
 
+  // clear the m_found_calls, we'll rediscover it for this range.
+  m_found_calls = false;
+  
   lldb::addr_t cur_addr = GetThread().GetRegisterContext()->GetPC();
   // Find the current address in our address ranges, and fetch the disassembly
   // if we haven't already:
@@ -317,9 +331,11 @@
   else {
 Target &target = GetThread().GetProcess()->GetTarget();
 const bool ignore_calls = GetKind() == eKindStepOverRange;
+bool found_calls;
 uint32_t branch_index =
 instructions->GetIndexOfNextBranchInstruction(pc_index, target,
-  ignore_calls);
+  ignore_calls, 
+  &m_found_calls);
 
 Address run_to_address;
 
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -5800,7 +5800,8 @@
 
   uint32_t branch_index =
   insn_list->GetIndexOfNextBranchInstruction(insn_offset, target,
- false /* ignore_calls*/);
+ false /* ignore_calls*/,
+ nullptr);
   if (branch_index == UINT32_MAX) {
 return retval;
   }
Index: lldb/source/Core/Disassembler.cpp
===
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -1101,15 +1101,22 @@
 uint32_t
 InstructionList::GetIndexOfNextBranchInstruction(uint32_t start,
  Target &target,
- bool ignore_calls) const {
+ bool ignore_calls,
+ bool *found_calls) const {
   size_t num_instructions = m_instructions.size();
 
   uint32_t next_branch = UINT32_MAX;
   size_t i;
+  
+  if (found_calls)
+*found_calls = false;
   for (i = start; i < num_instructions; i++) {
 if (m_instructions[i]->DoesBranch()) {
-  if (ignore_calls && m_instructions[i]->IsCall())
+  if (ignore_calls && m_instructions[i]->IsCall()) {
+if (found_calls)
+  *found_calls = true;
 continue;
+  }
   next_branch = i;
   break;
 }
Index: lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/locking.cpp
===
--- /dev/null
+++ lldb/packages/Python/

[Lldb-commits] [PATCH] D71440: Extending step-over range past calls was causing deadlocks, fix that.

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 234206.
jingham added a comment.

Changed the test case from locking.c - using pthreads directly - to locking.cpp 
using std::thread & Co.

I also changed the test I cobbed this one from to use std::thread as well.  I 
prospectively removed the expected fail Windows from the expression test, since 
this should build now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71440

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Target/ThreadPlanStepRange.h
  lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/TestExprDoesntBlock.py
  lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.c
  
lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.cpp
  lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/Makefile
  
lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/TestStepOverDoesntBlock.py
  lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/locking.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/ThreadPlanStepRange.cpp

Index: lldb/source/Target/ThreadPlanStepRange.cpp
===
--- lldb/source/Target/ThreadPlanStepRange.cpp
+++ lldb/source/Target/ThreadPlanStepRange.cpp
@@ -238,8 +238,18 @@
 }
 
 bool ThreadPlanStepRange::StopOthers() {
-  return (m_stop_others == lldb::eOnlyThisThread ||
-  m_stop_others == lldb::eOnlyDuringStepping);
+  switch (m_stop_others) {
+  case lldb::eOnlyThisThread:
+return true;
+  case lldb::eOnlyDuringStepping:
+// If there is a call in the range of the next branch breakpoint,
+// then we should always run all threads, since a call can execute
+// arbitrary code which might for instance take a lock that's held
+// by another thread.
+return !m_found_calls;
+  case lldb::eAllThreads:
+return false;
+  }
 }
 
 InstructionList *ThreadPlanStepRange::GetInstructionsForAddress(
@@ -292,6 +302,7 @@
 GetTarget().RemoveBreakpointByID(m_next_branch_bp_sp->GetID());
 m_next_branch_bp_sp.reset();
 m_could_not_resolve_hw_bp = false;
+m_found_calls = false;
   }
 }
 
@@ -305,6 +316,9 @@
   if (!m_use_fast_step)
 return false;
 
+  // clear the m_found_calls, we'll rediscover it for this range.
+  m_found_calls = false;
+  
   lldb::addr_t cur_addr = GetThread().GetRegisterContext()->GetPC();
   // Find the current address in our address ranges, and fetch the disassembly
   // if we haven't already:
@@ -317,9 +331,11 @@
   else {
 Target &target = GetThread().GetProcess()->GetTarget();
 const bool ignore_calls = GetKind() == eKindStepOverRange;
+bool found_calls;
 uint32_t branch_index =
 instructions->GetIndexOfNextBranchInstruction(pc_index, target,
-  ignore_calls);
+  ignore_calls, 
+  &m_found_calls);
 
 Address run_to_address;
 
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -5800,7 +5800,8 @@
 
   uint32_t branch_index =
   insn_list->GetIndexOfNextBranchInstruction(insn_offset, target,
- false /* ignore_calls*/);
+ false /* ignore_calls*/,
+ nullptr);
   if (branch_index == UINT32_MAX) {
 return retval;
   }
Index: lldb/source/Core/Disassembler.cpp
===
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -1101,15 +1101,22 @@
 uint32_t
 InstructionList::GetIndexOfNextBranchInstruction(uint32_t start,
  Target &target,
- bool ignore_calls) const {
+ bool ignore_calls,
+ bool *found_calls) const {
   size_t num_instructions = m_instructions.size();
 
   uint32_t next_branch = UINT32_MAX;
   size_t i;
+  
+  if (found_calls)
+*found_calls = false;
   for (i = start; i < num_instructions; i++) {
 if (m_instructions[i]->DoesBranch()) {
-  if (ignore_calls && m_instructions[i]->IsCall())
+  if (ignore_calls && m_instructions[i]->IsCall()) {
+if (found_calls)
+  *found_calls = true;
 continue;
+  }
   next_branch = i;
   break;
 }
Index: lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/locking.cpp
=

[Lldb-commits] [lldb] 9e9c5f0 - Explicitly specify -std=c++11 and include and .

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2019-12-16T18:09:24-08:00
New Revision: 9e9c5f0a6346ef02e31d5e8b91e6aab16a2e9370

URL: 
https://github.com/llvm/llvm-project/commit/9e9c5f0a6346ef02e31d5e8b91e6aab16a2e9370
DIFF: 
https://github.com/llvm/llvm-project/commit/9e9c5f0a6346ef02e31d5e8b91e6aab16a2e9370.diff

LOG: Explicitly specify -std=c++11 and include  and .

These files built on macos but not on Debian Linux.  Let's see if this fixes it.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile

lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.cpp
lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/Makefile
lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/locking.cpp

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile 
b/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile
index ee0d4690d834..4b3467bc4e82 100644
--- 
a/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/Makefile
@@ -1,4 +1,5 @@
 CXX_SOURCES := locking.cpp
+CXXFLAGS_EXTRAS := -std=c++11
 ENABLE_THREADS := YES
 
 include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.cpp
 
b/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.cpp
index fab3aa8c5635..8288a668fe86 100644
--- 
a/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.cpp
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/no-deadlock/locking.cpp
@@ -1,5 +1,7 @@
 #include 
 #include 
+#include 
+#include 
 
 std::mutex contended_mutex;
 

diff  --git 
a/lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/Makefile 
b/lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/Makefile
index ee0d4690d834..4b3467bc4e82 100644
--- a/lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/Makefile
+++ b/lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/Makefile
@@ -1,4 +1,5 @@
 CXX_SOURCES := locking.cpp
+CXXFLAGS_EXTRAS := -std=c++11
 ENABLE_THREADS := YES
 
 include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/locking.cpp 
b/lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/locking.cpp
index fab3aa8c5635..8288a668fe86 100644
--- 
a/lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/locking.cpp
+++ 
b/lldb/packages/Python/lldbsuite/test/lang/c/step_over_no_deadlock/locking.cpp
@@ -1,5 +1,7 @@
 #include 
 #include 
+#include 
+#include 
 
 std::mutex contended_mutex;
 



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

[Lldb-commits] [PATCH] D69309: Support template instantiation in the expression evaluator

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

In D69309#1763454 , @v.g.vassilev 
wrote:

> https://reviews.llvm.org/D41416 could be relevant. I am not an expert but I 
> think when reading the DWARF you could only register 'lazy' specializations 
> which will be imported only when really required.


https://reviews.llvm.org/D41416 is indeed relevant, but it has not been landed. 
I suffer from the same problem you do - lldb needs all the specializations, but 
providing those takes too much time. Are you still planning to land your 
granular lazy specialization change?


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

https://reviews.llvm.org/D69309



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

[Lldb-commits] [PATCH] D69309: Support template instantiation in the expression evaluator

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

In D69309#1752738 , @friss wrote:

> Basically, today the debug info will describe an entity named "Foo". The 
> accelerator tables all reference this name. So when Clang asks us if we know 
> "Foo" (which is what happens when instantiating), we fail to find the right 
> instantiations. The consensus of the above discussion was that we should 
> change the debug info to have "Foo" as the name of any instantiation, with a 
> child DIE describing the template arguments. Just doing this in the compiler 
> causes test failures in LLDB, so there's some work to do in LLDB to support 
> this.


Frederic, you say that "doing this in the compiler causes test failures in 
LLDB", which implies you have tried adding the template in the compiler. Do you 
have that compiler patch lying around so that we could have a look at what can 
be done on the lldb side?

I agree that a good long term fix is to have "Foo" as an entity in DWARF, 
although for backwards compatibility it might be better if the "Foo" template 
just contained references to the instantiations rather than having them as 
children.


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

https://reviews.llvm.org/D69309



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

[Lldb-commits] [lldb] 3c6554b - [lldb] Fix unused variable warning in ThreadPlanStepRange.cpp

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-17T08:53:06+01:00
New Revision: 3c6554be2e3c1500f825df5f7c186f2e58d6a33a

URL: 
https://github.com/llvm/llvm-project/commit/3c6554be2e3c1500f825df5f7c186f2e58d6a33a
DIFF: 
https://github.com/llvm/llvm-project/commit/3c6554be2e3c1500f825df5f7c186f2e58d6a33a.diff

LOG: [lldb] Fix unused variable warning in ThreadPlanStepRange.cpp

This was added in 434905b97d961531286d4b49c7ee1969f7cbea0e.
Remove it to fix the compiler warnings for this.

Added: 


Modified: 
lldb/source/Target/ThreadPlanStepRange.cpp

Removed: 




diff  --git a/lldb/source/Target/ThreadPlanStepRange.cpp 
b/lldb/source/Target/ThreadPlanStepRange.cpp
index a22337deaed5..1db29652aa8b 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -331,7 +331,6 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
   else {
 Target &target = GetThread().GetProcess()->GetTarget();
 const bool ignore_calls = GetKind() == eKindStepOverRange;
-bool found_calls;
 uint32_t branch_index =
 instructions->GetIndexOfNextBranchInstruction(pc_index, target,
   ignore_calls, 



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