[Lldb-commits] [PATCH] D67803: [lldb] Fix that importing decls in a TagDecl end up in wrong declaration context (partly reverts D61333)

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: friss, martong.
Herald added subscribers: lldb-commits, JDevlieghere, abidh, christof, 
rnkovacs, aprantl.
Herald added a project: LLDB.

In D61333  we dropped some code from 
ClangASTSource that checks if imported declarations
ended up in the right DeclContext. While this code wasn't tested by the test 
suite (or better, it was hit
by the test suite but we didn't have any checks that were affected) and the 
code seems pointless
(as usually Decls should end up in the right DeclContext), it actually broke 
the data formatters in LLDB
and causes a bunch of obscure bugs where structs suddenly miss all their 
members. The first report we got about
this was that printing a std::map doesn't work anymore when simply doing "expr 
m" (m is the std::map).

This patch reverts D61333  partly and 
reintroduces the check in a more stricter way (we actually check now that
we *move* the Decl and it is in a single DeclContext). This should fix all the 
problems we currently have until
we figure out how to properly fix the underlying issues. I changed the order of 
some std::map formatter tests
which is currently the most reliable way to test this problem (it's a tricky 
setup, see description below).

Fixes rdar://55502701 and rdar://55129537

--

Some more explanation what is actually going on and what is going wrong:

The situation we have is that if we have a `std::map m` and do a `expr m`, we 
end up seeing an empty map
(even if `m` has elements). The reason for this is that our data formatter sees 
that std::pair has no
members. However, `frame var m` works just fine (and fixes all following `expr 
m` calls).

The reason for why `expr` breaks std::map is that we actually copy the std::map 
nodes in two steps in the
three ASTContexts that are involved: The debug information ASTContext (D-AST), 
the expression ASTContext
we created for the current expression (E-AST) and the persistent ASTContext we 
use for our $variables (P-AST).

When doing `expr m` we do a minimal import of `std::map` from D-AST to E-AST 
just do the type checking/codegen.
This copies std::map itself and does a minimal.import of `std::pair` 
(that is, we don't actually import
the `first` and `second` members as we don't need them for anything). After the 
expression is done, we take
the expression result and copy it from E-AST to P-AST. This imports the E-AST's 
`std::pair` into P-AST which still
has no `first` and `second` as they are still undeserialized. Once we are in 
P-AST, the data formatter tries to
inspect `std::map` (and also `std::pair` as that's what the elements are) and 
it asks for the `std::pair` members.
We see that `std::pair` has undeserialized members and go to the 
ExternalASTSource to ask for them. However,
P-ASTs ExternalASTSource points to D-AST (and not E-AST, which `std::pair` came 
from). It can't point to E-AST
as that is only temporary and already gone (and also doesn't actually contain 
all decls we have in P-AST).

So we go to D-AST to get the `std::pair` members. The ASTImporter is asked to 
copy over `std::pair` members
and first checks if `std::pair` is already in P-AST. However, it only finds the 
std::pair we got from E-AST, so it
can't use it's map of already imported declarations and does a comparison 
between the `std::pair` decls we have
Because the ASTImporter thinks they are different declarations, it creates a 
second `std::pair` and fills in the
members `first` and `second` into the second `std::pair`. However, the data 
formatter is looking at the first
`std::pair` which still has no members as they are in the other decl. Now we 
pretend we have no declarations
and just print an empty map as a fallback.

The hack we had before fixed this issue by moving `first` and `second` to the 
first declaration which makes
the formatters happy as they can now see the members in the DeclContext they 
are querying.

Obviously this is a temporary patch until we get a real fix but I'm not sure 
what's the best way to fix this.
Implementing that the ClassTemplateSpecializationDecl actually understands that 
the two std::pair's are the same
decl fixes the issue, but this doesn't fix the bug for all declarations. My 
preferred solution would be to
complete all declarations in E-AST before they get moved to P-AST (as we anyway 
have to do this from what I can
tell), but that might have unintended side-effects and not sure what's the best 
way to implement this.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67803

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/sour

[Lldb-commits] [PATCH] D67803: [lldb] Fix that importing decls in a TagDecl end up in wrong declaration context (partly reverts D61333)

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I plan to match this soon-ish as this is essentially just a revert.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67803



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

[Lldb-commits] [PATCH] D61478: Move decl completion out of the ASTImporterDelegate and document it [NFC]

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 220965.
teemperor marked 4 inline comments as done.
teemperor added a comment.

- Renamed class and related variables to reflect that it is completing TagDecls.


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

https://reviews.llvm.org/D61478

Files:
  lldb/include/lldb/Symbol/ClangASTImporter.h
  lldb/source/Symbol/ClangASTImporter.cpp

Index: lldb/source/Symbol/ClangASTImporter.cpp
===
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -250,6 +250,94 @@
   }
 };
 
+namespace {
+/// Completes all imported TagDecls at the end of the scope.
+///
+/// While in a CompleteTagDeclsScope, every decl that could be completed will
+/// be completed at the end of the scope (including all Decls that are
+/// imported while completing the original Decls).
+class CompleteTagDeclsScope : public ClangASTImporter::NewDeclListener {
+  ClangASTImporter::ImporterDelegateSP m_delegate;
+  // FIXME: Investigate how many decls we usually have in these sets and
+  // see if we can use SmallPtrSet instead here.
+  std::set m_decls_to_complete;
+  std::set m_decls_already_completed;
+  clang::ASTContext *m_dst_ctx;
+  clang::ASTContext *m_src_ctx;
+  ClangASTImporter &importer;
+
+public:
+  /// Constructs a CompleteTagDeclsScope.
+  /// \param importer The ClangASTImporter that we should observe.
+  /// \param dst_ctx The ASTContext to which Decls are imported.
+  /// \param src_ctx The ASTContext from which Decls are imported.
+  explicit CompleteTagDeclsScope(ClangASTImporter &importer,
+clang::ASTContext *dst_ctx,
+clang::ASTContext *src_ctx)
+  : m_delegate(importer.GetDelegate(dst_ctx, src_ctx)), m_dst_ctx(dst_ctx),
+m_src_ctx(src_ctx), importer(importer) {
+m_delegate->SetImportListener(this);
+  }
+
+  virtual ~CompleteTagDeclsScope() {
+ClangASTImporter::ASTContextMetadataSP to_context_md =
+importer.GetContextMetadata(m_dst_ctx);
+
+// Complete all decls we collected until now.
+while (!m_decls_to_complete.empty()) {
+  NamedDecl *decl = *m_decls_to_complete.begin();
+
+  m_decls_already_completed.insert(decl);
+  m_decls_to_complete.erase(decl);
+
+  // We should only complete decls coming from the source context.
+  assert(to_context_md->m_origins[decl].ctx == m_src_ctx);
+
+  Decl *original_decl = to_context_md->m_origins[decl].decl;
+
+  // Complete the decl now.
+  ClangASTContext::GetCompleteDecl(m_src_ctx, original_decl);
+  if (auto *tag_decl = dyn_cast(decl)) {
+if (auto *original_tag_decl = dyn_cast(original_decl)) {
+  if (original_tag_decl->isCompleteDefinition()) {
+m_delegate->ImportDefinitionTo(tag_decl, original_tag_decl);
+tag_decl->setCompleteDefinition(true);
+  }
+}
+
+tag_decl->setHasExternalLexicalStorage(false);
+tag_decl->setHasExternalVisibleStorage(false);
+  } else if (auto *container_decl = dyn_cast(decl)) {
+container_decl->setHasExternalLexicalStorage(false);
+container_decl->setHasExternalVisibleStorage(false);
+  }
+
+  to_context_md->m_origins.erase(decl);
+}
+
+// Stop listening to imported decls. We do this after clearing the
+// Decls we needed to import to catch all Decls they might have pulled in.
+m_delegate->RemoveImportListener();
+  }
+
+  void NewDeclImported(clang::Decl *from, clang::Decl *to) override {
+// Filter out decls that we can't complete later.
+if (!isa(to) && !isa(to))
+  return;
+RecordDecl *from_record_decl = dyn_cast(from);
+// We don't need to completed injected class name decls.
+if (from_record_decl && from_record_decl->isInjectedClassName())
+  return;
+
+NamedDecl *to_named_decl = dyn_cast(to);
+// Check if we already completed this type.
+if (m_decls_already_completed.count(to_named_decl) != 0)
+  return;
+m_decls_to_complete.insert(to_named_decl);
+  }
+};
+} // namespace
+
 lldb::opaque_compiler_type_t
 ClangASTImporter::DeportType(clang::ASTContext *dst_ctx,
  clang::ASTContext *src_ctx,
@@ -263,27 +351,16 @@
 (unsigned long long)type, static_cast(src_ctx),
 static_cast(dst_ctx));
 
-  ImporterDelegateSP delegate_sp(GetDelegate(dst_ctx, src_ctx));
-
-  if (!delegate_sp)
-return nullptr;
-
-  std::set decls_to_deport;
-  std::set decls_already_deported;
-
   DeclContextOverride decl_context_override;
 
-  if (const clang::TagType *tag_type =
-  clang::QualType::getFromOpaquePtr(type)->getAs()) {
-decl_context_override.OverrideAllDeclsFromContainingFunction(
-tag_type->getDecl());
-  }
-
-  delegate_sp->InitDeportWorkQueues(&decls_to_deport, &decls_already_deported);
-
-  lldb::opaque_compiler_type_t result = CopyType(dst_ctx, src_ctx,

[Lldb-commits] [PATCH] D61478: Move decl completion out of the ASTImporterDelegate and document it [NFC]

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added inline comments.



Comment at: lldb/source/Symbol/ClangASTImporter.cpp:250
+/// imported while completing the original Decls).
+class DeportQueueScope : public ClangASTImporter::NewDeclListener {
+  ClangASTImporter::ImporterDelegateSP m_delegate;

martong wrote:
> The verb `deport` is pretty vague in this context for me. Actually, what this 
> class does is importing missing members and methods of classes. Perhaps a 
> better name could be `ImportMembersQueueScope` ?
> I still don't understand why is it needed to import the members in two steps 
> in LLDB: 1. import the class itself without members 2. import the members. So 
> perhaps we could have some documentation about that too here.
Renamed to point out that it's essentially just completing TagDecls (I didn't 
want to specify the members, that seems a bit vague).

And I wish I could document why we need to do this in two steps, but I didn't 
write that code so that's also a mystery to me :) When I'll figure this out 
I'll make a patch.



Comment at: lldb/source/Symbol/ClangASTImporter.cpp:324
+
+NamedDecl *to_named_decl = dyn_cast(to);
+// Check if we already deported this type.

martong wrote:
> Would it make sense to filter out those TagDecls which are already completed?
> E.g.:
> ```
> if (TagDecl *to_tag_decl = dyn_cast(to))
>   if (to_tag_decl->isCompleteDefinition()) // skip tags which are already 
> completed
> return;
> ```
> Or this would not work because there are cases when the tag is completed, but 
> the members are still missing? If that is the case could you please document 
> that here too?
Maybe, but that could make this patch non-NFC :) I can make this as a follow-up.


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

https://reviews.llvm.org/D61478



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

[Lldb-commits] [PATCH] D65942: Disallow implicit conversion from pointers to bool in llvm::toStringRef

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 220983.
teemperor marked an inline comment as done.
teemperor added a comment.

- Add GetNameForLanguageTypeAsRef to save some typing.
- Add comment for why we use enable_if.


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

https://reviews.llvm.org/D65942

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/Symbol/TypeSystem.cpp
  llvm/include/llvm/ADT/StringExtras.h
  llvm/include/llvm/CodeGen/CommandFlags.inc

Index: llvm/include/llvm/CodeGen/CommandFlags.inc
===
--- llvm/include/llvm/CodeGen/CommandFlags.inc
+++ llvm/include/llvm/CodeGen/CommandFlags.inc
@@ -389,7 +389,7 @@
 }
 if (DisableTailCalls.getNumOccurrences() > 0)
   NewAttrs.addAttribute("disable-tail-calls",
-toStringRef(DisableTailCalls));
+toStringRef((bool)DisableTailCalls));
 if (StackRealign)
   NewAttrs.addAttribute("stackrealign");
 
Index: llvm/include/llvm/ADT/StringExtras.h
===
--- llvm/include/llvm/ADT/StringExtras.h
+++ llvm/include/llvm/ADT/StringExtras.h
@@ -48,8 +48,13 @@
   return Result;
 }
 
-/// Construct a string ref from a boolean.
-inline StringRef toStringRef(bool B) { return StringRef(B ? "true" : "false"); }
+/// Construct a StringRef from a bool.
+// Uses enable_if to prevent implicit conversion to bool from pointers, ints, etc.
+template 
+typename std::enable_if::value, StringRef>::type
+toStringRef(T B) {
+  return StringRef(B ? "true" : "false");
+}
 
 /// Construct a string ref from an array ref of unsigned chars.
 inline StringRef toStringRef(ArrayRef Input) {
Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -237,7 +237,7 @@
   }
   error = llvm::make_error(
   "TypeSystem for language " +
-  llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+  Language::GetNameForLanguageTypeAsRef(language) +
   " doesn't exist",
   llvm::inconvertibleErrorCode());
   return std::move(error);
@@ -254,7 +254,7 @@
 }
 error = llvm::make_error(
 "TypeSystem for language " +
-llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+Language::GetNameForLanguageTypeAsRef(language) +
 " doesn't exist",
 llvm::inconvertibleErrorCode());
 return std::move(error);
@@ -264,7 +264,7 @@
 if (!can_create) {
   error = llvm::make_error(
   "Unable to find type system for language " +
-  llvm::toStringRef(Language::GetNameForLanguageType(language)),
+  Language::GetNameForLanguageTypeAsRef(language),
   llvm::inconvertibleErrorCode());
 } else {
   // Cache even if we get a shared pointer that contains a null type system
@@ -277,7 +277,7 @@
   }
   error = llvm::make_error(
   "TypeSystem for language " +
-  llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+  Language::GetNameForLanguageTypeAsRef(language) +
   " doesn't exist",
   llvm::inconvertibleErrorCode());
 }
@@ -306,7 +306,7 @@
   }
   error = llvm::make_error(
   "TypeSystem for language " +
-  llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+  Language::GetNameForLanguageTypeAsRef(language) +
   " doesn't exist",
   llvm::inconvertibleErrorCode());
   return std::move(error);
@@ -323,7 +323,7 @@
 }
 error = llvm::make_error(
 "TypeSystem for language " +
-llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+Language::GetNameForLanguageTypeAsRef(language) +
 " doesn't exist",
 llvm::inconvertibleErrorCode());
 return std::move(error);
@@ -333,7 +333,7 @@
 if (!can_create) {
   error = llvm::make_error(
   "Unable to find type system for language " +
-  llvm::toStringRef(Language::GetNameForLanguageType(language)),
+  Language::GetNameForLanguageTypeAsRef(language),
   llvm::inconvertibleErrorCode());
 } else {
   // Cache even if we get a shared pointer that contains a null type system
@@ -346,7 +346,7 @@
   }
   error = llvm::make_error(
   "TypeSystem for language " +
-  llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+  Language::GetNameForLanguageTypeAsRef(language) +
   " doesn't exist",
   llvm::inconvertibleErrorCode());
 }
Index: lldb/include/lldb/Target/Language.h
===
--- lldb/incl

[Lldb-commits] [PATCH] D65942: Disallow implicit conversion from pointers to bool in llvm::toStringRef

[Raphael Isemann via Phabricator via lldb-commits]

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



Comment at: lldb/source/Symbol/TypeSystem.cpp:331
   "TypeSystem for language " +
-  llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+  llvm::StringRef(Language::GetNameForLanguageType(language)) +
   " doesn't exist",

dblaikie wrote:
> Perhaps Language::GetNameForLanguageType should return StringRef to start 
> with?
> 
> (& guessing there should be an lldb test case demonstrating this fix?)
The problem is that this is used in LLDB's SB API which returns a const char* 
(and we can't change that). So we can't return a StringRef as that doesn't 
convert back to const char * (and we can't go over std::string or anything like 
that as the API doesn't transfer ownership back to the caller). I added an 
alternative method to save some typing (and maybe we can go fully StringRef in 
the future if we ever change the SB API strings).

And we don't test log message content in LLDB. Also the log message only 
happens on systems with unsupported type system (like MIPS or something like 
that). And we don't have any such bot that would even run the test.


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

https://reviews.llvm.org/D65942



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

[Lldb-commits] [PATCH] D61565: Ignore generated @import statements in the expression evaluator

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 220987.
teemperor marked an inline comment as done.
teemperor edited the summary of this revision.

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

https://reviews.llvm.org/D61565

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
@@ -23,6 +23,9 @@
 
 class ClangExpressionSourceCode : public ExpressionSourceCode {
 public:
+  /// The file name we use for the wrapper code that we inject before
+  /// the user expression.
+  static const llvm::StringRef g_prefix_file_name;
   static const char *g_expression_prefix;
 
   static ClangExpressionSourceCode *CreateWrapped(llvm::StringRef filename,
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -29,9 +29,12 @@
 
 using namespace lldb_private;
 
+#define PREFIX_NAME ""
+
+const llvm::StringRef ClangExpressionSourceCode::g_prefix_file_name = 
PREFIX_NAME;
+
 const char *ClangExpressionSourceCode::g_expression_prefix =
-R"(
-#line 1 ""
+"#line 1 \"" PREFIX_NAME R"("
 #ifndef offsetof
 #define offsetof(t, d) __builtin_offsetof(t, d)
 #endif
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -105,16 +105,26 @@
 class ClangExpressionParser::LLDBPreprocessorCallbacks : public PPCallbacks {
   ClangModulesDeclVendor &m_decl_vendor;
   ClangPersistentVariables &m_persistent_vars;
+  clang::SourceManager &m_source_mgr;
   StreamString m_error_stream;
   bool m_has_errors = false;
 
 public:
   LLDBPreprocessorCallbacks(ClangModulesDeclVendor &decl_vendor,
-ClangPersistentVariables &persistent_vars)
-  : m_decl_vendor(decl_vendor), m_persistent_vars(persistent_vars) {}
+ClangPersistentVariables &persistent_vars,
+clang::SourceManager &source_mgr)
+  : m_decl_vendor(decl_vendor), m_persistent_vars(persistent_vars),
+m_source_mgr(source_mgr) {}
 
   void moduleImport(SourceLocation import_location, clang::ModuleIdPath path,
 const clang::Module * /*null*/) override {
+// Ignore modules that are imported in the wrapper code as these are not
+// loaded by the user.
+llvm::StringRef filename =
+m_source_mgr.getPresumedLoc(import_location).getFilename();
+if (filename == ClangExpressionSourceCode::g_prefix_file_name)
+  return;
+
 SourceModule module;
 
 for (const std::pair &component : path)
@@ -572,8 +582,8 @@
 llvm::cast(
 target_sp->GetPersistentExpressionStateForLanguage(
 lldb::eLanguageTypeC));
-std::unique_ptr pp_callbacks(
-new LLDBPreprocessorCallbacks(*decl_vendor, *clang_persistent_vars));
+std::unique_ptr pp_callbacks(new LLDBPreprocessorCallbacks(
+*decl_vendor, *clang_persistent_vars, m_compiler->getSourceManager()));
 m_pp_callbacks =
 static_cast(pp_callbacks.get());
 m_compiler->getPreprocessor().addPPCallbacks(std::move(pp_callbacks));


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
@@ -23,6 +23,9 @@
 
 class ClangExpressionSourceCode : public ExpressionSourceCode {
 public:
+  /// The file name we use for the wrapper code that we inject before
+  /// the user expression.
+  static const llvm::StringRef g_prefix_file_name;
   static const char *g_expression_prefix;
 
   static ClangExpressionSourceCode *CreateWrapped(llvm::StringRef filename,
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -29,9 +29,12 @@
 
 using namespace lldb_private;
 
+#define PREFIX_NAME ""
+
+const llvm::StringRef ClangExpressionSourceCode::g_pref

[Lldb-commits] [PATCH] D61565: Ignore generated @import statements in the expression evaluator

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

- Rebased and some minor refactoring to prevent having multiple places with the 
duplicated wrapper file name.


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

https://reviews.llvm.org/D61565



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

[Lldb-commits] [lldb] r372381 - [lldb][NFC] Remove unused include in TestLineEntry.cpp

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Fri Sep 20 03:30:38 2019
New Revision: 372381

URL: http://llvm.org/viewvc/llvm-project?rev=372381&view=rev
Log:
[lldb][NFC] Remove unused include in TestLineEntry.cpp

Modified:
lldb/trunk/unittests/Symbol/TestLineEntry.cpp

Modified: lldb/trunk/unittests/Symbol/TestLineEntry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Symbol/TestLineEntry.cpp?rev=372381&r1=372380&r2=372381&view=diff
==
--- lldb/trunk/unittests/Symbol/TestLineEntry.cpp (original)
+++ lldb/trunk/unittests/Symbol/TestLineEntry.cpp Fri Sep 20 03:30:38 2019
@@ -22,7 +22,6 @@
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/SymbolContext.h"
-#include "lldb/Utility/StreamString.h"
 
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Program.h"


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

[Lldb-commits] [PATCH] D67589: Fix crash on SBCommandReturnObject & assignment

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D67589#1674640 , @jankratochvil 
wrote:

> Maybe http://lldb.llvm.org/resources/sbapi.html could say that and it would 
> be much more clear.


That's probably a good idea, though I would still keep the list of restrictions 
spelled out as ABI can be broken in a lot of subtle and unobvious ways (at 
least to a person who has never tried to maintain a stable abi).

> 
> 
>>   lldb_private::CommandReturnObjectImpl {
>> bool owned;
>> std::unique_ptr m_opaque_up;
>>   };
> 
> Is this a request to rework this patch this way? If so isn't it safer / more 
> clear to do it rather this way?
> 
>   lldb_private::CommandReturnObjectImpl {
> bool owned;
> lldb_private::CommandReturnObject *m_opaque_ptr;
> ~CommandReturnObjectImpl() { if (owned) delete m_opaque_ptr; }
>   };

I think I would like that better than the swap trick. Since you're now inside a 
pimpl, you can replace the two members with a llvm::PointerIntPair, if you are 
so inclined.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67589



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

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

What behavior does this change exactly? This is not immediately obvious to me, 
and probably anyone else not intimately familiar with the stop reason machinery 
(= everyone except Jim, probably). Could you add a test case for this (which I 
am hoping will also shed some light on what is this patch doing)?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67776



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

[Lldb-commits] [PATCH] D67583: Fix swig python package path

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I could be off mark here, but I do recall some people trying to get this code 
to work in environments where python and lldb(llvm) are configured with 
different libdirs (so LLVM_LIBDIR_SUFFIX in llvm, and `configure --prefix=X` in 
python). Is that the case you're running into here?

If that is something we want to support (personally, I would very much like to 
just declare that "unsupported", but maybe we don't have that luxury). Then 
we'll need to be super careful about distinguishing the _llvm_ libdir name 
(i.e. the place where liblldb and friends should go to) from the _python_ 
libdir (the destination for the site-packages stuff). I think the code already 
partially tries to do that, but I am not sure if it is really correct.

Since I'm already complaining about python paths, I'll add that the way we 
currently compute the python path is wrong for cross-compilation. (Because it 
will pick up the path from the host python, not the target one.) If we're going 
to be changing something here, it would be nice to fix that too (which, I hope, 
will also allow us avoid needing to keep multiple places in sync).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67583



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

[Lldb-commits] [PATCH] D67789: bugfix: File::GetWaitableHandle() should call fileno()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for the fix. This sounds like it should be unit testable. Could add a 
test for that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67789



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

[Lldb-commits] [PATCH] D61478: Move decl completion out of the ASTImporterDelegate and document it [NFC]

[Gabor Marton via Phabricator via lldb-commits]

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

Looks good to me, thanks!




Comment at: lldb/source/Symbol/ClangASTImporter.cpp:324
+
+NamedDecl *to_named_decl = dyn_cast(to);
+// Check if we already deported this type.

teemperor wrote:
> martong wrote:
> > Would it make sense to filter out those TagDecls which are already 
> > completed?
> > E.g.:
> > ```
> > if (TagDecl *to_tag_decl = dyn_cast(to))
> >   if (to_tag_decl->isCompleteDefinition()) // skip tags which are already 
> > completed
> > return;
> > ```
> > Or this would not work because there are cases when the tag is completed, 
> > but the members are still missing? If that is the case could you please 
> > document that here too?
> Maybe, but that could make this patch non-NFC :) I can make this as a 
> follow-up.
Okay.



Comment at: lldb/source/Symbol/ClangASTImporter.cpp:328
+RecordDecl *from_record_decl = dyn_cast(from);
+// We don't need to completed injected class name decls.
+if (from_record_decl && from_record_decl->isInjectedClassName())

typo: "to completed" -> "to complete"


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

https://reviews.llvm.org/D61478



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

[Lldb-commits] [PATCH] D67792: bugfix: llvm_private::File::GetStream() can fail if m_options == 0

[Pavel Labath via Phabricator via lldb-commits]

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

It doesn't look like this will work on windows (I find no trace of fcntl in the 
windows docs). Even if we find a way to guess the open mode on windows somehow, 
I am not convinced that is the right way forward here. I would rather we find a 
way to fix the api so that it is harder to use incorrectly, then trying to 
guess what should happen when the api is used in a potentially incorrect way.

What is original reason you needed this patch for? Could we just return a null 
FILE* if the user didn't call SetOptions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67792



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

[Lldb-commits] [lldb] r372385 - Move decl completion out of the ASTImporterDelegate and document it [NFC]

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Fri Sep 20 05:52:55 2019
New Revision: 372385

URL: http://llvm.org/viewvc/llvm-project?rev=372385&view=rev
Log:
Move decl completion out of the ASTImporterDelegate and document it [NFC]

Summary:
The ASTImporterDelegate is currently responsible for both recording and also 
completing
types. This patch moves the actual completion and recording code outside the 
ASTImporterDelegate
to reduce the amount of responsibilities the ASTImporterDelegate has to fulfill.

As I anyway had to touch the code when moving I also documented and refactored 
most of it
(e.g. no more asserts that we call the deporting start/end function always as a 
pair).

Note that I had to make the ASTImporterDelegate and it's related functions 
public now so that
I can move out the functionality in another class (that doesn't need to be in 
the header).

Reviewers: shafik, aprantl, martong, a.sidorin

Reviewed By: martong

Subscribers: rnkovacs, lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
lldb/trunk/source/Symbol/ClangASTImporter.cpp

Modified: lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTImporter.h?rev=372385&r1=372384&r2=372385&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ClangASTImporter.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTImporter.h Fri Sep 20 05:52:55 2019
@@ -210,7 +210,7 @@ public:
   void ForgetDestination(clang::ASTContext *dst_ctx);
   void ForgetSource(clang::ASTContext *dst_ctx, clang::ASTContext *src_ctx);
 
-private:
+public:
   struct DeclOrigin {
 DeclOrigin() : ctx(nullptr), decl(nullptr) {}
 
@@ -235,23 +235,29 @@ private:
 
   typedef std::map OriginMap;
 
+  /// Listener interface used by the ASTImporterDelegate to inform other code
+  /// about decls that have been imported the first time.
+  struct NewDeclListener {
+virtual ~NewDeclListener() = default;
+/// A decl has been imported for the first time.
+virtual void NewDeclImported(clang::Decl *from, clang::Decl *to) = 0;
+  };
+
   /// ASTImporter that intercepts and records the import process of the
   /// underlying ASTImporter.
   ///
   /// This class updates the map from declarations to their original
-  /// declarations and can record and complete declarations that have been
-  /// imported in a certain interval.
+  /// declarations and can record declarations that have been imported in a
+  /// certain interval.
   ///
   /// When intercepting a declaration import, the ASTImporterDelegate uses the
   /// CxxModuleHandler to replace any missing or malformed declarations with
   /// their counterpart from a C++ module.
-  class ASTImporterDelegate : public clang::ASTImporter {
-  public:
+  struct ASTImporterDelegate : public clang::ASTImporter {
 ASTImporterDelegate(ClangASTImporter &master, clang::ASTContext 
*target_ctx,
 clang::ASTContext *source_ctx)
 : clang::ASTImporter(*target_ctx, master.m_file_manager, *source_ctx,
  master.m_file_manager, true /*minimal*/),
-  m_decls_to_deport(nullptr), m_decls_already_deported(nullptr),
   m_master(master), m_source_ctx(source_ctx) {
   setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
 }
@@ -287,43 +293,32 @@ private:
   }
 };
 
-  protected:
-llvm::Expected ImportImpl(clang::Decl *From) override;
-
-  public:
-// A call to "InitDeportWorkQueues" puts the delegate into deport mode.
-// In deport mode, every copied Decl that could require completion is
-// recorded and placed into the decls_to_deport set.
-//
-// A call to "ExecuteDeportWorkQueues" completes all the Decls that
-// are in decls_to_deport, adding any Decls it sees along the way that it
-// hasn't already deported.  It proceeds until decls_to_deport is empty.
-//
-// These calls must be paired.  Leaving a delegate in deport mode or trying
-// to start deport delegate with a new pair of queues will result in an
-// assertion failure.
-
-void
-InitDeportWorkQueues(std::set *decls_to_deport,
- std::set *decls_already_deported);
-void ExecuteDeportWorkQueues();
-
 void ImportDefinitionTo(clang::Decl *to, clang::Decl *from);
 
 void Imported(clang::Decl *from, clang::Decl *to) override;
 
 clang::Decl *GetOriginalDecl(clang::Decl *To) override;
 
+void SetImportListener(NewDeclListener *listener) {
+  assert(m_new_decl_listener == nullptr && "Already attached a listener?");
+  m_new_decl_listener = listener;
+}
+void RemoveImportListener() { m_new_decl_listener = nullptr; }
+
+  protected:
+llvm::Expected ImportImpl(clang::Decl *From) override;
+
+  private:
 /// Decls we should ignore when mapping dec

[Lldb-commits] [PATCH] D61478: Move decl completion out of the ASTImporterDelegate and document it [NFC]

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372385: Move decl completion out of the ASTImporterDelegate 
and document it [NFC] (authored by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61478?vs=220965&id=221007#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61478

Files:
  lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
  lldb/trunk/source/Symbol/ClangASTImporter.cpp

Index: lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
@@ -210,7 +210,7 @@
   void ForgetDestination(clang::ASTContext *dst_ctx);
   void ForgetSource(clang::ASTContext *dst_ctx, clang::ASTContext *src_ctx);
 
-private:
+public:
   struct DeclOrigin {
 DeclOrigin() : ctx(nullptr), decl(nullptr) {}
 
@@ -235,23 +235,29 @@
 
   typedef std::map OriginMap;
 
+  /// Listener interface used by the ASTImporterDelegate to inform other code
+  /// about decls that have been imported the first time.
+  struct NewDeclListener {
+virtual ~NewDeclListener() = default;
+/// A decl has been imported for the first time.
+virtual void NewDeclImported(clang::Decl *from, clang::Decl *to) = 0;
+  };
+
   /// ASTImporter that intercepts and records the import process of the
   /// underlying ASTImporter.
   ///
   /// This class updates the map from declarations to their original
-  /// declarations and can record and complete declarations that have been
-  /// imported in a certain interval.
+  /// declarations and can record declarations that have been imported in a
+  /// certain interval.
   ///
   /// When intercepting a declaration import, the ASTImporterDelegate uses the
   /// CxxModuleHandler to replace any missing or malformed declarations with
   /// their counterpart from a C++ module.
-  class ASTImporterDelegate : public clang::ASTImporter {
-  public:
+  struct ASTImporterDelegate : public clang::ASTImporter {
 ASTImporterDelegate(ClangASTImporter &master, clang::ASTContext *target_ctx,
 clang::ASTContext *source_ctx)
 : clang::ASTImporter(*target_ctx, master.m_file_manager, *source_ctx,
  master.m_file_manager, true /*minimal*/),
-  m_decls_to_deport(nullptr), m_decls_already_deported(nullptr),
   m_master(master), m_source_ctx(source_ctx) {
   setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
 }
@@ -287,43 +293,32 @@
   }
 };
 
-  protected:
-llvm::Expected ImportImpl(clang::Decl *From) override;
-
-  public:
-// A call to "InitDeportWorkQueues" puts the delegate into deport mode.
-// In deport mode, every copied Decl that could require completion is
-// recorded and placed into the decls_to_deport set.
-//
-// A call to "ExecuteDeportWorkQueues" completes all the Decls that
-// are in decls_to_deport, adding any Decls it sees along the way that it
-// hasn't already deported.  It proceeds until decls_to_deport is empty.
-//
-// These calls must be paired.  Leaving a delegate in deport mode or trying
-// to start deport delegate with a new pair of queues will result in an
-// assertion failure.
-
-void
-InitDeportWorkQueues(std::set *decls_to_deport,
- std::set *decls_already_deported);
-void ExecuteDeportWorkQueues();
-
 void ImportDefinitionTo(clang::Decl *to, clang::Decl *from);
 
 void Imported(clang::Decl *from, clang::Decl *to) override;
 
 clang::Decl *GetOriginalDecl(clang::Decl *To) override;
 
+void SetImportListener(NewDeclListener *listener) {
+  assert(m_new_decl_listener == nullptr && "Already attached a listener?");
+  m_new_decl_listener = listener;
+}
+void RemoveImportListener() { m_new_decl_listener = nullptr; }
+
+  protected:
+llvm::Expected ImportImpl(clang::Decl *From) override;
+
+  private:
 /// Decls we should ignore when mapping decls back to their original
 /// ASTContext. Used by the CxxModuleHandler to mark declarations that
 /// were created from the 'std' C++ module to prevent that the Importer
 /// tries to sync them with the broken equivalent in the debug info AST.
 std::set m_decls_to_ignore;
-std::set *m_decls_to_deport;
-std::set *m_decls_already_deported;
 ClangASTImporter &m_master;
 clang::ASTContext *m_source_ctx;
 CxxModuleHandler *m_std_handler = nullptr;
+/// The currently attached listener.
+NewDeclListener *m_new_decl_listener = nullptr;
   };
 
   typedef std::shared_ptr ImporterDelegateSP;
@@ -389,6 +384,7 @@
 }
   }
 
+public:
   DeclOrigin GetDeclOrigin(const clang::Decl *decl);
 
   clang::FileManager m_

[Lldb-commits] [PATCH] D67803: [lldb] Fix that importing decls in a TagDecl end up in wrong declaration context (partly reverts D61333)

[Gabor Marton via Phabricator via lldb-commits]

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

Thanks for the thorough explanation about the different ASTContexts in LLDB.
The hack is indeed hideous, but seems good to me... for now until the real 
solution is born.

> So we go to D-AST to get the std::pair members. The ASTImporter is asked to 
> copy over std::pair members
>  and first checks if std::pair is already in P-AST. However, it only finds 
> the std::pair we got from E-AST, so it
>  can't use it's map of already imported declarations and does a comparison 
> between the std::pair decls we have
>  Because the ASTImporter thinks they are different declarations, it creates a 
> second std::pair

Note that LLDB uses the lenient ODR violation handling strategy 
(`ASTImporter::ODRHandlingType::Liberal`).
With the other, stricter ODR violation handling strategy, when the to be 
imported std::pair turns out to be nonequivalent with the existing one we would 
get an error instead of a new decl.
Would it make sense to change the odr handling strategy before the formatter 
turns to the ExternalASTSource?
It would not solve this particular issue, but perhaps could make discovering 
bugs like this easier.

> My preferred solution would be to complete all declarations in E-AST before 
> they get moved to P-AST

That sounds reasonable to me.

> When doing expr m we do a minimal import of std::map from D-AST to E-AST just 
> do the type checking/codegen.

There are some cases, when codegen do need the completed tagdecl, not just the 
skeleton we get via minimal import. So, there must be cases when in the E-AST 
we have a full, completed type, right? What would happen if we imported 
everything as a full declaration in the first place, instead of  using the 
minimal import? Could that work?
Perhaps it is even more intrusive then your preferred solution, but could 
greatly reduce the import related code and complexity in LLDB.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67803



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

[Lldb-commits] [PATCH] D67793: new api class: SBFile

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This is an interesting undertaking. There's a lot of confusion about FILE* in 
lldb, and it would be great to clean some of that stuff up.

The api seems reasonably straight-forward. The one thing that struck me is that 
you're using unique_ptrs for the pimpled objects. That would mean the SBFile 
objects are non-copyable, which is generally not a good thing, as python really 
likes to copy things. Looking ahead at your patchset, it looks like I am onto 
something, as you're then trying to implement shared semantics inside 
lldb_private::File. That doesn't sound like the most fortunate solution to me, 
as most of the lldb_private::File users don't need that functionality. I am 
wondering if it wouldn't be better to use a shared_ptr in SBFile, and implement 
any extra sharing semantics you need at the SB level. Also, this needs a bunch 
of tests.

From a higher level perspective, it would be good to have a couple of 
paragraphs saying what exactly is the end goal here (what new apis will you 
introduce, how will they work, etc.). I can sort of guess some of that from 
looking at the patchset, but it would be nice to have that written down, so 
that we know we're on the same page. This looks like it is a sufficiently big 
of a change that it would deserve a quick RFC email to lldb-dev.




Comment at: lldb/source/API/SBFile.cpp:20-26
+void SBFile::SetStream(FILE *file, bool transfer_ownership) {
+m_opaque_up = std::make_unique(file, transfer_ownership);
+}
+
+void SBFile::SetDescriptor(int fd, bool transfer_owndership) {
+m_opaque_up = std::make_unique(fd, transfer_owndership);
+}

I think it would be better if these were constructors instead of member 
functions. That way you might be able to get rid of the all the `if 
(!m_opaque_up) {` checks as the File member will always be initialized.



Comment at: lldb/source/API/SBFile.cpp:21
+void SBFile::SetStream(FILE *file, bool transfer_ownership) {
+m_opaque_up = std::make_unique(file, transfer_ownership);
+}

It looks like you need to reformat this file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793



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

[Lldb-commits] [PATCH] D67803: [lldb] Fix that importing decls in a TagDecl end up in wrong declaration context (partly reverts D61333)

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

BTW, I tried to access the bug reports rdar://55502701 and rdar://55129537, but 
could not.
I tried at openradar, but there the search for the ID was not successful.
I wonder if there is a publicly accessible (for non Apple employees) URL for 
these bugs?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67803



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

[Lldb-commits] [PATCH] D67831: [lldb] Get the TargetAPI lock in SBProcess::IsInstrumentationRuntimePresen

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: jingham.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.

We should get the TargetAPI lock here to prevent the process of being destroyed 
while we are in the function. Thanks Jim for explaining what's going on.

Fixes rdar://54424754


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67831

Files:
  lldb/source/API/SBProcess.cpp


Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1180,6 +1180,9 @@
   if (!process_sp)
 return false;
 
+  std::lock_guard guard(
+  process_sp->GetTarget().GetAPIMutex());
+
   InstrumentationRuntimeSP runtime_sp =
   process_sp->GetInstrumentationRuntime(type);
 


Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1180,6 +1180,9 @@
   if (!process_sp)
 return false;
 
+  std::lock_guard guard(
+  process_sp->GetTarget().GetAPIMutex());
+
   InstrumentationRuntimeSP runtime_sp =
   process_sp->GetInstrumentationRuntime(type);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

The flag m_abnormal_stop_was_expected was added in the commit 
 to avoid stop on crash because attach 
always stops
with a SIGSTOP on OSX. I faced the same issue with SIGTRAP and SIGSTOP sent by 
a gdb-remote stub on some commands. This patch filters out such harmless 
signals instead of setting the flag everywhere in commands.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67776



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

[Lldb-commits] [PATCH] D67792: bugfix: llvm_private::File::GetStream() can fail if m_options == 0

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

In D67792#1676556 , @labath wrote:

> It doesn't look like this will work on windows (I find no trace of fcntl in 
> the windows docs). Even if we find a way to guess the open mode on windows 
> somehow, I am not convinced that is the right way forward here. I would 
> rather we find a way to fix the api so that it is harder to use incorrectly, 
> then trying to guess what should happen when the api is used in a potentially 
> incorrect way.
>
> What is original reason you needed this patch for? Could we just return a 
> null FILE* if the user didn't call SetOptions?


While I was writing tests for my python files branch, I first tested the simple 
case of creating a SBFile with a descriptor, and found to my surprise that 
almost nothing in LLDB could actually write to that file, because everything 
expected a FILE* stream and it couldn't make one.

Should I just add an options argument to SetDescriptor instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67792



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

[Lldb-commits] [PATCH] D67793: new api class: SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

In D67793#1676592 , @labath wrote:

> The api seems reasonably straight-forward. The one thing that struck me is 
> that you're using unique_ptrs for the pimpled objects. That would mean the 
> SBFile objects are non-copyable, which is generally not a good thing, as 
> python really likes to copy things. Looking ahead at your patchset, it looks 
> like I am onto something, as you're then trying to implement shared semantics 
> inside lldb_private::File. That doesn't sound like the most fortunate 
> solution to me, as most of the lldb_private::File users don't need that 
> functionality. I am wondering if it wouldn't be better to use a shared_ptr in 
> SBFile, and implement any extra sharing semantics you need at the SB level. 
> Also, this needs a bunch of tests.


That was my initial thought as well, to use a shared_ptr here, and not have any 
sharing at the lldb_private::File level. However, I found I couldn't do it 
that way.   lldb_private::File doesn't really have the semantics of a file 
object, it has the semantics of a //variable//, which can point to a file 
object.Users of lldb_private::File expect to be able to Close() a file and 
re-open it as something else.   They expect to be able to embed File 
immediately in other objects.They expect to be able to make an empty File 
and check it with IsValid(). I thought it would be best to let them keep 
using File in that way, so later in the queue I make File copyable and add 
FileOps class to manage the sharing between copied files.The other way to 
do it would be to track down every use of lldb_private::File that isn't through 
a pointer and convert it to shared_ptr instead.Then SBFile could just 
be another shared_ptr.   Should I do it that way?

As far as python copying things goes, that's not a problem.   SWIG will always 
allocate a SBFile on the heap and copy the pointers to it, it doesn't need an 
actual C++ copy constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793



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

[Lldb-commits] [PATCH] D67760: [lldb] Decouple importing the std C++ module from the way the program is compiled

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I understand the motivation, just a few questions about the approach:

- Can we make it such that *if* the program was compiled with -gmodules LLDB is 
using the DWARF module info as authoritative source instead of using this 
heuristic?
- Is there a path towards supporting the following scenario?
  1. User compiles a program that contains `@import Foo;` using a custom -DBAR 
define, which affects how Foo is built on the command line.
  2. LLDB reads the fact the Foo was imported with -DBAR from DWARF and makes 
it available in the expression evaluator.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67760



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

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

+1 on a test case. Other than that this seems like a good improvement to the 
way we filter stop reasons.




Comment at: source/Interpreter/CommandInterpreter.cpp:2164
+StopReason reason = stop_info->GetStopReason();
+if (eStopReasonException == reason || eStopReasonInstrumentation == reason)
+  return true;

I'm curious why you swapped the two operands. Is there a benefit in 
`eStopReasonException == reason` compared to `reason == eStopReasonException`? 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67776



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

[Lldb-commits] [PATCH] D67792: bugfix: llvm_private::File::GetStream() can fail if m_options == 0

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 221047.
lawrence_danna added a comment.

Added options to SetDescriptor instead of using ::fcntl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67792

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Core/StreamFile.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4299,9 +4299,10 @@
   IOHandlerProcessSTDIO(Process *process, int write_fd)
   : IOHandler(process->GetTarget().GetDebugger(),
   IOHandler::Type::ProcessIO),
-m_process(process), m_write_file(write_fd, false) {
+m_process(process),
+m_write_file(write_fd, File::eOpenOptionWrite, false) {
 m_pipe.CreateNew(false);
-m_read_file.SetDescriptor(GetInputFD(), false);
+m_read_file.SetDescriptor(GetInputFD(), File::eOpenOptionRead, false);
   }
 
   ~IOHandlerProcessSTDIO() override = default;
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1043,9 +1043,9 @@
   file.Close();
   // We don't own the file descriptor returned by this function, make sure the
   // File object knows about that.
-  file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), false);
   PythonString py_mode = GetAttributeValue("mode").AsType();
-  file.SetOptions(PythonFile::GetOptionsFromMode(py_mode.GetString()));
+  auto options = PythonFile::GetOptionsFromMode(py_mode.GetString());
+  file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), options, false);
   return file.IsValid();
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -537,7 +537,7 @@
   int err = -1;
   int save_errno = 0;
   if (fd >= 0) {
-File file(fd, true);
+File file(fd, 0, true);
 Status error = file.Close();
 err = 0;
 save_errno = error.GetError();
@@ -568,7 +568,7 @@
   }
 
   std::string buffer(count, 0);
-  File file(fd, false);
+  File file(fd, File::eOpenOptionRead, false);
   Status error = file.Read(static_cast(&buffer[0]), count, offset);
   const ssize_t bytes_read = error.Success() ? count : -1;
   const int save_errno = error.GetError();
@@ -600,7 +600,7 @@
 if (packet.GetChar() == ',') {
   std::string buffer;
   if (packet.GetEscapedBinaryData(buffer)) {
-File file(fd, false);
+File file(fd, File::eOpenOptionWrite, false);
 size_t count = buffer.size();
 Status error =
 file.Write(static_cast(&buffer[0]), count, offset);
Index: lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -425,11 +425,16 @@
   }
 }
 Status posix_error;
+int oflag = file_action->GetActionArgument();
 int created_fd =
-open(file_spec.GetPath().c_str(), file_action->GetActionArgument(),
- S_IRUSR | S_IWUSR);
+open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR);
 if (created_fd >= 0) {
-  file.SetDescriptor(created_fd, true);
+  uint32_t file_options = 0;
+  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
+file_options |= File::eOpenOptionRead;
+  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
+file_options |= File::eOpenOptionWrite;
+  file.SetDescriptor(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] forKey:key];
   return error; // Success
 } else {
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/Expressio

[Lldb-commits] [PATCH] D67791: dotest.py: bugfix: test filters with -f do not work on Python3

[Jonas Devlieghere via Phabricator via lldb-commits]

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

I would prefer to update the unittest2 dependency instead, but I tried doing so 
and it appears to be less straightforward than I'd hoped. I have added it to my 
todo-list, but this is fine in the meantime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67791



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

[Lldb-commits] [lldb] r372411 - Doxygenify comments.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Fri Sep 20 10:15:57 2019
New Revision: 372411

URL: http://llvm.org/viewvc/llvm-project?rev=372411&view=rev
Log:
Doxygenify comments.

Modified:
lldb/trunk/include/lldb/Symbol/Variable.h
lldb/trunk/source/Symbol/Variable.cpp

Modified: lldb/trunk/include/lldb/Symbol/Variable.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/Variable.h?rev=372411&r1=372410&r2=372411&view=diff
==
--- lldb/trunk/include/lldb/Symbol/Variable.h (original)
+++ lldb/trunk/include/lldb/Symbol/Variable.h Fri Sep 20 10:15:57 2019
@@ -26,15 +26,14 @@ class Variable : public UserID, public s
 public:
   typedef RangeVector RangeList;
 
-  // Constructors and Destructors
-  Variable(lldb::user_id_t uid, const char *name,
-   const char
-   *mangled, // The mangled or fully qualified name of the 
variable.
-   const lldb::SymbolFileTypeSP &symfile_type_sp,
-   lldb::ValueType scope, SymbolContextScope *owner_scope,
-   const RangeList &scope_range, Declaration *decl,
-   const DWARFExpression &location, bool external, bool artificial,
-   bool static_member = false);
+  /// Constructors and Destructors.
+  ///
+  /// \param mangled The mangled or fully qualified name of the variable.
+  Variable(lldb::user_id_t uid, const char *name, const char *mangled,
+   const lldb::SymbolFileTypeSP &symfile_type_sp, lldb::ValueType 
scope,
+   SymbolContextScope *owner_scope, const RangeList &scope_range,
+   Declaration *decl, const DWARFExpression &location, bool external,
+   bool artificial, bool static_member = false);
 
   virtual ~Variable();
 
@@ -50,11 +49,11 @@ public:
 
   SymbolContextScope *GetSymbolContextScope() const { return m_owner_scope; }
 
-  // Since a variable can have a basename "i" and also a mangled named
-  // "_ZN12_GLOBAL__N_11iE" and a demangled mangled name "(anonymous
-  // namespace)::i", this function will allow a generic match function that can
-  // be called by commands and expression parsers to make sure we match
-  // anything we come across.
+  /// Since a variable can have a basename "i" and also a mangled named
+  /// "_ZN12_GLOBAL__N_11iE" and a demangled mangled name "(anonymous
+  /// namespace)::i", this function will allow a generic match function that 
can
+  /// be called by commands and expression parsers to make sure we match
+  /// anything we come across.
   bool NameMatches(ConstString name) const;
 
   bool NameMatches(const RegularExpression ®ex) const;
@@ -107,26 +106,34 @@ public:
   CompilerDecl GetDecl();
 
 protected:
-  ConstString m_name; // The basename of the variable (no namespaces)
-  Mangled m_mangled;  // The mangled name of the variable
-  lldb::SymbolFileTypeSP m_symfile_type_sp; // The type pointer of the variable
-// (int, struct, class, etc)
-  lldb::ValueType m_scope;  // global, parameter, local
-  SymbolContextScope
-  *m_owner_scope; // The symbol file scope that this variable was defined 
in
-  RangeList m_scope_range; // The list of ranges inside the owner's scope where
-   // this variable is valid
-  Declaration m_declaration;  // Declaration location for this item.
-  DWARFExpression m_location; // The location of this variable that can be fed
-  // to DWARFExpression::Evaluate()
-  uint8_t m_external : 1, // Visible outside the containing compile unit?
-  m_artificial : 1, // Non-zero if the variable is not explicitly declared
-// in source
-  m_loc_is_const_data : 1, // The m_location expression contains the
-   // constant variable value data, not a DWARF
-   // location
-  m_static_member : 1; // Non-zero if variable is static member of a class
-   // or struct.
+  /// The basename of the variable (no namespaces).
+  ConstString m_name;
+  /// The mangled name of the variable.
+  Mangled m_mangled;
+  /// The type pointer of the variable (int, struct, class, etc)
+  /// global, parameter, local.
+  lldb::SymbolFileTypeSP m_symfile_type_sp;
+  lldb::ValueType m_scope;
+  /// The symbol file scope that this variable was defined in
+  SymbolContextScope *m_owner_scope;
+  /// The list of ranges inside the owner's scope where this variable
+  /// is valid.
+  RangeList m_scope_range;
+  /// Declaration location for this item.
+  Declaration m_declaration;
+  /// The location of this variable that can be fed to
+  /// DWARFExpression::Evaluate().
+  DWARFExpression m_location;
+  /// Visible outside the containing compile unit?
+  unsigned m_external : 1;
+  /// Non-zero if the variable is not explicitly declared in source.
+  unsigned m_artificial : 1;
+  /// The m_location expression contains the constant variable va

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha updated this revision to Diff 221058.
tatyana-krasnukha added a comment.

Added a test


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67776

Files:
  include/lldb/Interpreter/CommandInterpreter.h
  include/lldb/Interpreter/CommandReturnObject.h
  packages/Python/lldbsuite/test/driver/batch_mode/TestBatchMode.py
  source/Commands/CommandObjectProcess.cpp
  source/Interpreter/CommandInterpreter.cpp
  source/Interpreter/CommandReturnObject.cpp

Index: packages/Python/lldbsuite/test/driver/batch_mode/TestBatchMode.py
===
--- packages/Python/lldbsuite/test/driver/batch_mode/TestBatchMode.py
+++ packages/Python/lldbsuite/test/driver/batch_mode/TestBatchMode.py
@@ -78,6 +78,35 @@
 import pexpect
 child.expect(pexpect.EOF)
 
+@expectedFlakeyFreeBSD("llvm.org/pr25172 fails rarely on the buildbot")
+def test_batch_mode_launch_stop_at_entry(self):
+"""Test that the lldb driver's batch mode works correctly."""
+self.build()
+
+exe = self.getBuildArtifact("a.out")
+
+# Launch with the option '--stop-at-entry' may stop with a signal (usually SIGSTOP)
+# that should be suppressed since it doesn't imply a crash and
+# this is not a reason to exit batch mode.
+extra_args = ['-b',
+'-o', 'process launch --stop-at-entry',
+'-o', 'continue',
+]
+self.launch(executable=exe, extra_args=extra_args)
+child = self.child
+
+# Check that the process has been launched:
+child.expect("Process ([0-9]+) launched:")
+# We should have continued:
+child.expect_exact("continue")
+# The App should have not have crashed:
+child.expect_exact("Got there on time and it did not crash.")
+
+# Then lldb should exit.
+child.expect_exact("exited")
+import pexpect
+child.expect(pexpect.EOF)
+
 def closeVictim(self):
 if self.victim is not None:
 self.victim.close()
Index: source/Interpreter/CommandReturnObject.cpp
===
--- source/Interpreter/CommandReturnObject.cpp
+++ source/Interpreter/CommandReturnObject.cpp
@@ -33,8 +33,7 @@
 
 CommandReturnObject::CommandReturnObject()
 : m_out_stream(), m_err_stream(), m_status(eReturnStatusStarted),
-  m_did_change_process_state(false), m_interactive(true),
-  m_abnormal_stop_was_expected(false) {}
+  m_did_change_process_state(false), m_interactive(true) {}
 
 CommandReturnObject::~CommandReturnObject() {}
 
Index: source/Interpreter/CommandInterpreter.cpp
===
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -63,8 +63,10 @@
 #include "lldb/Utility/Args.h"
 
 #include "lldb/Target/Process.h"
+#include "lldb/Target/StopInfo.h"
 #include "lldb/Target/TargetList.h"
 #include "lldb/Target/Thread.h"
+#include "lldb/Target/UnixSignals.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -2141,6 +2143,39 @@
   return platform_sp;
 }
 
+bool CommandInterpreter::DidProcessStopAbnormally() const {
+  TargetSP target_sp = m_debugger.GetTargetList().GetSelectedTarget();
+  if (!target_sp)
+return false;
+
+  ProcessSP process_sp(target_sp->GetProcessSP());
+  if (!process_sp)
+return false;
+
+  if (eStateStopped != process_sp->GetState())
+return false;
+
+  for (const auto &thread_sp : process_sp->GetThreadList().Threads()) {
+StopInfoSP stop_info = thread_sp->GetStopInfo();
+if (!stop_info)
+  return false;
+
+StopReason reason = stop_info->GetStopReason();
+if (eStopReasonException == reason || eStopReasonInstrumentation == reason)
+  return true;
+
+if (eStopReasonSignal == reason) {
+  const auto stop_signal = static_cast(stop_info->GetValue());
+  UnixSignalsSP signals = process_sp->GetUnixSignals();
+  if (!signals || !signals->GetShouldSuppress(stop_signal))
+// The signal is not of known suppressable signals, so it is abnormal.
+return true;
+}
+  }
+
+  return false;
+}
+
 void CommandInterpreter::HandleCommands(const StringList &commands,
 ExecutionContext *override_context,
 CommandInterpreterRunOptions &options,
@@ -2251,38 +2286,22 @@
 }
 
 // Also check for "stop on crash here:
-bool should_stop = false;
-if (tmp_result.GetDidChangeProcessState() && options.GetStopOnCrash()) {
-  TargetSP target_sp(m_debugger.GetTargetList().GetSelectedTarget());
-  if (target_sp) {
-ProcessSP process_sp(target_sp->GetProcessSP());
-if (process_sp) {
-  for (ThreadSP thread_sp : process_sp->GetThreadList().Threads()) {
-

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added inline comments.



Comment at: source/Interpreter/CommandInterpreter.cpp:2164
+StopReason reason = stop_info->GetStopReason();
+if (eStopReasonException == reason || eStopReasonInstrumentation == reason)
+  return true;

JDevlieghere wrote:
> I'm curious why you swapped the two operands. Is there a benefit in 
> `eStopReasonException == reason` compared to `reason == 
> eStopReasonException`? 
This technique prevents unintended assignment instead of comparison since 
left-hand value is constant. 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67776



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

[Lldb-commits] [PATCH] D67760: [lldb] Decouple importing the std C++ module from the way the program is compiled

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

This patch is *only* for getting the `std` module working on as many projects 
as possible. The initial plan about using -gmodules and then looking at the 
imported modules as expressed in DWARF is still what we will do for modules in 
general (and it will then have precedence over this approach if user's have 
fully modularized and migrated their projects to `-gmodules`). So clear yes to 
both of your questions. -gmodules support will be added when I get around to 
update D61606 , but getting this into a state 
where we can ship it is the priority for now :)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67760



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

[Lldb-commits] [PATCH] D67792: File::SetDescriptor() should require options

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

Do we still need `SetOptions` after this? Are there cases where the value needs 
to change after construction? If not I would consider removing that function. 
Otherwise this LGTM if Pavel doesn't have any other concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67792



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

[Lldb-commits] [PATCH] D67774: [Mangle] Add flag to asm labels to disable global prefixing

[Vedant Kumar via Phabricator via lldb-commits]

vsk updated this revision to Diff 221066.
vsk retitled this revision from "[Mangle] Check ExternalASTSource before adding 
prefix to asm label names" to "[Mangle] Add flag to asm labels to disable 
global prefixing".
vsk edited the summary of this revision.
vsk added a comment.

Thanks for your feedback. I've added a flag to asm labels to disable global 
prefixing. I've tried to minimize the behavior change in this patch -- it seems 
to me that additional cleanups can happen in follow-ups.


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

https://reviews.llvm.org/D67774

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/AST/Mangle.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/unittests/AST/DeclTest.cpp
  lldb/source/Symbol/ClangASTContext.cpp

Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -8300,8 +8300,8 @@
 cxx_method_decl->addAttr(clang::UsedAttr::CreateImplicit(*getASTContext()));
 
   if (mangled_name != nullptr) {
-cxx_method_decl->addAttr(
-clang::AsmLabelAttr::CreateImplicit(*getASTContext(), mangled_name));
+cxx_method_decl->addAttr(clang::AsmLabelAttr::CreateImplicit(
+*getASTContext(), mangled_name, /*literal=*/true));
   }
 
   // Populate the method decl with parameter decls
Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -10,12 +10,16 @@
 //
 //===--===//
 
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Mangle.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
 
 using namespace clang::ast_matchers;
 using namespace clang::tooling;
+using namespace clang;
 
 TEST(Decl, CleansUpAPValues) {
   MatchFinder Finder;
@@ -56,3 +60,49 @@
   "constexpr _Complex __uint128_t c = 0x;",
   Args));
 }
+
+TEST(Decl, AsmLabelAttr) {
+  // Create two method decls: `f` and `g`.
+  StringRef Code = R"(
+struct S {
+  void f() {}
+  void g() {}
+};
+  )";
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-target", "i386-apple-darwin"});
+  ASTContext &Ctx = AST->getASTContext();
+  DiagnosticsEngine &Diags = AST->getDiagnostics();
+  SourceManager &SM = AST->getSourceManager();
+  FileID MainFileID = SM.getMainFileID();
+
+  // Find the method decls within the AST.
+  SmallVector Decls;
+  AST->findFileRegionDecls(MainFileID, Code.find('{'), 0, Decls);
+  ASSERT_TRUE(Decls.size() == 1);
+  CXXRecordDecl *DeclS = cast(Decls[0]);
+  NamedDecl *DeclF = *DeclS->method_begin();
+  NamedDecl *DeclG = *(++DeclS->method_begin());
+
+  // Attach asm labels to the decls: one literal, and one subject to global
+  // prefixing.
+  DeclF->addAttr(::new (Ctx) AsmLabelAttr(Ctx, SourceLocation(), "foo",
+  /*LiteralLabel=*/true));
+  DeclG->addAttr(::new (Ctx) AsmLabelAttr(Ctx, SourceLocation(), "goo",
+  /*LiteralLabel=*/false));
+
+  // Mangle the decl names.
+  std::string MangleF, MangleG;
+  MangleContext *MC = ItaniumMangleContext::create(Ctx, Diags);
+  {
+llvm::raw_string_ostream OS(MangleF);
+MC->mangleName(DeclF, OS);
+  }
+  {
+llvm::raw_string_ostream OS(MangleG);
+MC->mangleName(DeclG, OS);
+  }
+
+  ASSERT_TRUE(0 == MangleF.compare("foo"));
+  ASSERT_TRUE(0 == MangleG.compare("\x01goo"));
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2766,6 +2766,8 @@
 
   if (AsmLabelAttr *NewA = New->getAttr()) {
 if (AsmLabelAttr *OldA = Old->getAttr()) {
+  assert(OldA->getLiteralLabel() == NewA->getLiteralLabel() &&
+ "Redeclaration of asm label changes label kind");
   if (OldA->getLabel() != NewA->getLabel()) {
 // This redeclaration changes __asm__ label.
 Diag(New->getLocation(), diag::err_different_asm_label);
@@ -6983,8 +6985,8 @@
   }
 }
 
-NewVD->addAttr(::new (Context)
-   AsmLabelAttr(Context, SE->getStrTokenLoc(0), Label));
+NewVD->addAttr(::new (Context) AsmLabelAttr(Context, SE->getStrTokenLoc(0),
+Label, /*LiteralLabel=*/false));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
@@ -8882,8 +8884,9 @@
   if (Expr *E = (Expr*) D.getAsmLabel()) {
 // The parser guarantees this is a string.
 StringLiteral *SE = cast(E);
-NewFD->addAttr(::new (Context) AsmLabelAttr(Context, SE->getStrTokenLoc(0),
-

[Lldb-commits] [PATCH] D67583: Fix swig python package path

[Haibo Huang via Phabricator via lldb-commits]

hhb added a comment.

In D67583#1675889 , @ZeGentzy wrote:

> In D67583#1675714 , @hhb wrote:
>
> > My theory now is that your python has a different implementation of 
> > get_python_lib(). After all we should not guess what python would do. I'm 
> > making a change.
> >  If you still interested in testing that, you can fix the path in a.py...
>
>
> Oh, derp, I completely missed that. I get `(True, 
> '/src/out/./lib/python3.7/site-packages/lldb', '')` for python and `(True, 
> '/src/out/./lib/python2.7/site-packages/lldb', '')` for python2.
>
> EDIT:
>
>   $ cat a.py   
>   import sys
>   
> sys.path.append('/home/gentz/Documents/aur/gfx/backups/second/llvm-git-gentz/llvm-git-gentz/src/llvm-project32/lldb/scripts/Python')
>   import finishSwigPythonLLDB
>   vDictArgs = {"--srcRoot": "/src/llvm-project/lldb",
>"--targetDir": "/src/out/./lib32",
>"--cfgBldDir": "/src/out/tools/lldb/scripts",
>"--prefix": "/src/out",
>"--cmakeBuildConfiguration": ".",
>"--lldbLibDir": "lib32",
>"-m": None,
>   }
>   print(finishSwigPythonLLDB.get_framework_python_dir(vDictArgs))
>


That's really... weird... I don't know how your files end up in the lib32 
directory. Anyway this change is reverted in https://reviews.llvm.org/D67781.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67583



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

[Lldb-commits] [PATCH] D67583: Fix swig python package path

[Haibo Huang via Phabricator via lldb-commits]

hhb added a comment.

In D67583#1676531 , @labath wrote:

> I could be off mark here, but I do recall some people trying to get this code 
> to work in environments where python and lldb(llvm) are configured with 
> different libdirs (so LLVM_LIBDIR_SUFFIX in llvm, and `configure --prefix=X` 
> in python). Is that the case you're running into here?
>
> If that is something we want to support (personally, I would very much like 
> to just declare that "unsupported", but maybe we don't have that luxury). 
> Then we'll need to be super careful about distinguishing the _llvm_ libdir 
> name (i.e. the place where liblldb and friends should go to) from the 
> _python_ libdir (the destination for the site-packages stuff). I think the 
> code already partially tries to do that, but I am not sure if it is really 
> correct.
>
> Since I'm already complaining about python paths, I'll add that the way we 
> currently compute the python path is wrong for cross-compilation. (Because it 
> will pick up the path from the host python, not the target one.) If we're 
> going to be changing something here, it would be nice to fix that too (which, 
> I hope, will also allow us avoid needing to keep multiple places in sync).


I also realized that this is totally wrong for cross-compilation. We are doing 
cross compilation to windows with MinGW so I need to fix that.

Actually I don't know why we need get_python_lib(). From what I can see now, 
the 3 paths should match:

1. finishSwigPythonLLDB.py write necessary files to 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/distutils.sysconfig.get_python_lib()

2. lldb/scripts/CMakeLists.txt copies them to lib${LLVM_LIBDIR_SUFFIX} (if not 
xcode)

3. ScriptInterpreterPython.cpp adds the right path into PYTHONPATH.

As long as these 3 paths matches, we are good. Then why not simply put 
everything into ${liblldb path}/site-packages for all platforms?

(I'm gonna make a change to test that. But let me know if anything is obviously 
wrong.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67583



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

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: packages/Python/lldbsuite/test/driver/batch_mode/TestBatchMode.py:83
+def test_batch_mode_launch_stop_at_entry(self):
+"""Test that the lldb driver's batch mode works correctly."""
+self.build()

Update the comment?



Comment at: source/Interpreter/CommandInterpreter.cpp:2164
+StopReason reason = stop_info->GetStopReason();
+if (eStopReasonException == reason || eStopReasonInstrumentation == reason)
+  return true;

tatyana-krasnukha wrote:
> JDevlieghere wrote:
> > I'm curious why you swapped the two operands. Is there a benefit in 
> > `eStopReasonException == reason` compared to `reason == 
> > eStopReasonException`? 
> This technique prevents unintended assignment instead of comparison since 
> left-hand value is constant. 
Ah, because the `StopReason` is not const. Fair enough, thanks for the 
explanation. I don't think I've seen that in other parts of LLDB but I'm not 
opposed to it. 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67776



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

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

[Jim Ingham via Phabricator via lldb-commits]

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

This is all about the decision in "batch" mode for what constitutes a stop by 
the program that should cause batch mode to return the control to the user.

I don't think anybody would want to stop for SIGSTOP and SIGINT if the program 
under the debugger can recover.

However, __builtin_trap on most UNIX systems results in a SIGTRAP.  It's not at 
all uncommon to use __builtin_trap to signal a fatal condition in a library 
(for instance Swift uses this on array overflow and for a number of other error 
conditions.)  It has the benefit over "abort" that it doesn't push a bunch of 
stack frames onto the stack before dying, which makes crash analysis easier.  
This use means we certainly want batch mode to stop for SIGTRAP.

If the SIGTRAP is for a breakpoint, lldb will convert the incoming signal event 
into a breakpoint-hit event, which doesn't cause the batch mode to stop.

So maybe we don't actually need to have "should suppress" be true for SIGTRAP.  
We should be able to distinguish between traps that were really in the code of 
the binary and traps lldb inserted, and make the decision based on that.  That 
was just copied from gdb, which kept the breakpoint SIGTRAP's raw.

I can't tell how real breakpoint SIGTRAPs work through lldb because on MacOS 
because don't come in as SIGTRAP's (they come in as Mach exceptions).  It would 
be worth seeing how this works for Linux hosts.

But in any case, a SIGTRAP that doesn't have an underlying breakpoint should 
definitely cause batch mode to stop.  Under what circumstances did you need to 
suppress it?




Comment at: source/Interpreter/CommandInterpreter.cpp:2164
+StopReason reason = stop_info->GetStopReason();
+if (eStopReasonException == reason || eStopReasonInstrumentation == reason)
+  return true;

tatyana-krasnukha wrote:
> JDevlieghere wrote:
> > I'm curious why you swapped the two operands. Is there a benefit in 
> > `eStopReasonException == reason` compared to `reason == 
> > eStopReasonException`? 
> This technique prevents unintended assignment instead of comparison since 
> left-hand value is constant. 
We don't use that convention anywhere else in lldb.  Introducing it piecemeal 
just makes the code harder to read, and I personally find this convention makes 
code generally harder to parse in my head.  


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67776



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

[Lldb-commits] [PATCH] D67789: bugfix: File::GetWaitableHandle() should call fileno()

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 221082.
lawrence_danna added a comment.
Herald added a subscriber: mgorny.

added a unit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67789

Files:
  lldb/source/Host/common/File.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/FileTest.cpp


Index: lldb/unittests/Host/FileTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/FileTest.cpp
@@ -0,0 +1,36 @@
+//===-- FileTest.cpp *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/File.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(File, GetWaitableHandleFileno) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+
+  llvm::SmallString<128> name;
+  int fd;
+  llvm::sys::fs::createTemporaryFile(llvm::Twine(Info->test_case_name()) + "-" 
+
+ Info->name(),
+ "test", fd, name);
+  llvm::FileRemover remover(name);
+  EXPECT_GE(fd, 0);
+
+  FILE *stream = fdopen(fd, "r");
+  EXPECT_TRUE(stream);
+
+  File file(stream, true);
+  EXPECT_EQ(file.GetWaitableHandle(), fd);
+}
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -11,6 +11,7 @@
   SocketTest.cpp
   SocketTestUtilities.cpp
   TaskPoolTest.cpp
+  FileTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -91,7 +91,7 @@
   return kInvalidDescriptor;
 }
 
-IOObject::WaitableHandle File::GetWaitableHandle() { return m_descriptor; }
+IOObject::WaitableHandle File::GetWaitableHandle() { return GetDescriptor(); }
 
 void File::SetDescriptor(int fd, bool transfer_ownership) {
   if (IsValid())


Index: lldb/unittests/Host/FileTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/FileTest.cpp
@@ -0,0 +1,36 @@
+//===-- FileTest.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/File.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(File, GetWaitableHandleFileno) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+
+  llvm::SmallString<128> name;
+  int fd;
+  llvm::sys::fs::createTemporaryFile(llvm::Twine(Info->test_case_name()) + "-" +
+ Info->name(),
+ "test", fd, name);
+  llvm::FileRemover remover(name);
+  EXPECT_GE(fd, 0);
+
+  FILE *stream = fdopen(fd, "r");
+  EXPECT_TRUE(stream);
+
+  File file(stream, true);
+  EXPECT_EQ(file.GetWaitableHandle(), fd);
+}
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -11,6 +11,7 @@
   SocketTest.cpp
   SocketTestUtilities.cpp
   TaskPoolTest.cpp
+  FileTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -91,7 +91,7 @@
   return kInvalidDescriptor;
 }
 
-IOObject::WaitableHandle File::GetWaitableHandle() { return m_descriptor; }
+IOObject::WaitableHandle File::GetWaitableHandle() { return GetDescriptor(); }
 
 void File::SetDescriptor(int fd, bool transfer_ownership) {
   if (IsValid())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67792: File::SetDescriptor() should require options

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 221093.
lawrence_danna edited the summary of this revision.
lawrence_danna added a comment.

removed SetOptions()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67792

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Core/StreamFile.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4299,9 +4299,10 @@
   IOHandlerProcessSTDIO(Process *process, int write_fd)
   : IOHandler(process->GetTarget().GetDebugger(),
   IOHandler::Type::ProcessIO),
-m_process(process), m_write_file(write_fd, false) {
+m_process(process),
+m_write_file(write_fd, File::eOpenOptionWrite, false) {
 m_pipe.CreateNew(false);
-m_read_file.SetDescriptor(GetInputFD(), false);
+m_read_file.SetDescriptor(GetInputFD(), File::eOpenOptionRead, false);
   }
 
   ~IOHandlerProcessSTDIO() override = default;
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1043,9 +1043,9 @@
   file.Close();
   // We don't own the file descriptor returned by this function, make sure the
   // File object knows about that.
-  file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), false);
   PythonString py_mode = GetAttributeValue("mode").AsType();
-  file.SetOptions(PythonFile::GetOptionsFromMode(py_mode.GetString()));
+  auto options = PythonFile::GetOptionsFromMode(py_mode.GetString());
+  file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), options, false);
   return file.IsValid();
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -537,7 +537,7 @@
   int err = -1;
   int save_errno = 0;
   if (fd >= 0) {
-File file(fd, true);
+File file(fd, 0, true);
 Status error = file.Close();
 err = 0;
 save_errno = error.GetError();
@@ -568,7 +568,7 @@
   }
 
   std::string buffer(count, 0);
-  File file(fd, false);
+  File file(fd, File::eOpenOptionRead, false);
   Status error = file.Read(static_cast(&buffer[0]), count, offset);
   const ssize_t bytes_read = error.Success() ? count : -1;
   const int save_errno = error.GetError();
@@ -600,7 +600,7 @@
 if (packet.GetChar() == ',') {
   std::string buffer;
   if (packet.GetEscapedBinaryData(buffer)) {
-File file(fd, false);
+File file(fd, File::eOpenOptionWrite, false);
 size_t count = buffer.size();
 Status error =
 file.Write(static_cast(&buffer[0]), count, offset);
Index: lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -425,11 +425,16 @@
   }
 }
 Status posix_error;
+int oflag = file_action->GetActionArgument();
 int created_fd =
-open(file_spec.GetPath().c_str(), file_action->GetActionArgument(),
- S_IRUSR | S_IWUSR);
+open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR);
 if (created_fd >= 0) {
-  file.SetDescriptor(created_fd, true);
+  uint32_t file_options = 0;
+  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
+file_options |= File::eOpenOptionRead;
+  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
+file_options |= File::eOpenOptionWrite;
+  file.SetDescriptor(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] forKey:key];
   return error; // Success
 } else {
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/

[Lldb-commits] [PATCH] D67760: [lldb] Decouple importing the std C++ module from the way the program is compiled

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/include/lldb/Symbol/CompileUnit.h:228
 
+  /// Apply a lambda to each external module referenced by this compilation
+  /// unit. Recursively also descends into the referenced external modules

What does "external module" mean in this context? Can we be specific about 
lldb::Module vs clang::Module?



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp:442
+  std::string full_msg = "[C++ module config] " + msg;
+  LLDB_LOG(log, full_msg.c_str());
+  return CppModuleConfiguration();

if you use `LLDB_LOG(log, "[C++ module config] %s ", msg.c_str());` we only pay 
the cost for concatenating if logging is on.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:1
+//===-- CppModuleConfiguration.cpp --*- C++ 
-*-===//
+//

the `-*- C++ -*-` only makes sense for `.h` files were it is ambiguous whether 
they contain C or C++ code.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:46
+  // we should remove that '/bits' suffix to get the actual include directory.
+  if (dir.endswith("/usr/include/bits"))
+dir.consume_back("/bits");

llvm::sys::path::append to avoid hardcoding path separators?



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:65
+if (!analyzeFile(f))
+  break;
+


```
if (!std::all_of(supported_files.begin(), supported_files.end(), analyzeFile))
  return;
```
?



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:68
+  // Calculate the resource directory for LLDB.
+  m_resource_inc = GetClangResourceDir().GetPath() + "/include";
+

sys::path::append



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h:28
+/// True iff this path hasn't been set yet.
+bool m_first = true;
+

micro-optimization: may use less space if the two bools are grouped together


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67760



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

[Lldb-commits] [PATCH] D65942: Disallow implicit conversion from pointers to bool in llvm::toStringRef

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: lldb/source/Symbol/TypeSystem.cpp:331
   "TypeSystem for language " +
-  llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+  llvm::StringRef(Language::GetNameForLanguageType(language)) +
   " doesn't exist",

teemperor wrote:
> dblaikie wrote:
> > Perhaps Language::GetNameForLanguageType should return StringRef to start 
> > with?
> > 
> > (& guessing there should be an lldb test case demonstrating this fix?)
> The problem is that this is used in LLDB's SB API which returns a const char* 
> (and we can't change that). So we can't return a StringRef as that doesn't 
> convert back to const char * (and we can't go over std::string or anything 
> like that as the API doesn't transfer ownership back to the caller). I added 
> an alternative method to save some typing (and maybe we can go fully 
> StringRef in the future if we ever change the SB API strings).
> 
> And we don't test log message content in LLDB. Also the log message only 
> happens on systems with unsupported type system (like MIPS or something like 
> that). And we don't have any such bot that would even run the test.
Those both seem a bit problematic... 

Is there no API migration strategy in the SB API? Introducing new versions of 
functions and deprecating old ones?

And testing - clearly this code was buggy and worth fixing, so it ought to be 
worth testing. Any chance of unit testing/API-level testing any of this?


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

https://reviews.llvm.org/D65942



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

[Lldb-commits] [PATCH] D67856: [LLDB] Fix compilation for MinGW, remove redundant class name on inline member

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: asmith, labath, hhb.
Herald added a subscriber: JDevlieghere.
Herald added a project: LLDB.

This fixes build errors like these:

  NativeRegisterContextWindows.h:22:33: error: extra qualification on member 
'NativeRegisterContextWindows'
NativeRegisterContextWindows::NativeRegisterContextWindows(
~~^


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67856

Files:
  lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.h


Index: lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.h
===
--- lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.h
+++ lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.h
@@ -19,7 +19,7 @@
 
 class NativeRegisterContextWindows : public NativeRegisterContextRegisterInfo {
 public:
-  NativeRegisterContextWindows::NativeRegisterContextWindows(
+  NativeRegisterContextWindows(
   NativeThreadProtocol &native_thread,
   RegisterInfoInterface *reg_info_interface_p);
 


Index: lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.h
===
--- lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.h
+++ lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.h
@@ -19,7 +19,7 @@
 
 class NativeRegisterContextWindows : public NativeRegisterContextRegisterInfo {
 public:
-  NativeRegisterContextWindows::NativeRegisterContextWindows(
+  NativeRegisterContextWindows(
   NativeThreadProtocol &native_thread,
   RegisterInfoInterface *reg_info_interface_p);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r372424 - [lldb] Process formatters in reverse-chronological order

[Jan Kratochvil via lldb-commits]

Author: jankratochvil
Date: Fri Sep 20 13:19:18 2019
New Revision: 372424

URL: http://llvm.org/viewvc/llvm-project?rev=372424&view=rev
Log:
[lldb] Process formatters in reverse-chronological order

If one reverts D66398 then the TestDataFormatterStdList does fail - as the C++
formatters are initialized in the opposite order. But the current state of
trunk does not mind the order for C++ formatters.

It is using now a single std::vector as suggested by Pavel Labath.

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

Modified:
lldb/trunk/include/lldb/DataFormatters/FormattersContainer.h
lldb/trunk/include/lldb/Utility/RegularExpression.h

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py

Modified: lldb/trunk/include/lldb/DataFormatters/FormattersContainer.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/DataFormatters/FormattersContainer.h?rev=372424&r1=372423&r2=372424&view=diff
==
--- lldb/trunk/include/lldb/DataFormatters/FormattersContainer.h (original)
+++ lldb/trunk/include/lldb/DataFormatters/FormattersContainer.h Fri Sep 20 
13:19:18 2019
@@ -65,7 +65,7 @@ template  class FormatMap {
 public:
   typedef typename ValueType::SharedPointer ValueSP;
-  typedef std::map MapType;
+  typedef std::vector> MapType;
   typedef typename MapType::iterator MapIterator;
   typedef std::function 
ForEachCallback;
 
@@ -79,20 +79,22 @@ public:
   entry->GetRevision() = 0;
 
 std::lock_guard guard(m_map_mutex);
-m_map[std::move(name)] = entry;
+Delete(name);
+m_map.emplace_back(std::move(name), std::move(entry));
 if (listener)
   listener->Changed();
   }
 
   bool Delete(const KeyType &name) {
 std::lock_guard guard(m_map_mutex);
-MapIterator iter = m_map.find(name);
-if (iter == m_map.end())
-  return false;
-m_map.erase(iter);
-if (listener)
-  listener->Changed();
-return true;
+for (MapIterator iter = m_map.begin(); iter != m_map.end(); ++iter)
+  if (iter->first == name) {
+m_map.erase(iter);
+if (listener)
+  listener->Changed();
+return true;
+  }
+return false;
   }
 
   void Clear() {
@@ -104,11 +106,12 @@ public:
 
   bool Get(const KeyType &name, ValueSP &entry) {
 std::lock_guard guard(m_map_mutex);
-MapIterator iter = m_map.find(name);
-if (iter == m_map.end())
-  return false;
-entry = iter->second;
-return true;
+for (const auto &pos : m_map)
+  if (pos.first == name) {
+entry = pos.second;
+return true;
+  }
+return false;
   }
 
   void ForEach(ForEachCallback callback) {
@@ -126,29 +129,17 @@ public:
 
   ValueSP GetValueAtIndex(size_t index) {
 std::lock_guard guard(m_map_mutex);
-MapIterator iter = m_map.begin();
-MapIterator end = m_map.end();
-while (index > 0) {
-  iter++;
-  index--;
-  if (end == iter)
-return ValueSP();
-}
-return iter->second;
+if (index >= m_map.size())
+  return ValueSP();
+return m_map[index].second;
   }
 
   // If caller holds the mutex we could return a reference without copy ctor.
   KeyType GetKeyAtIndex(size_t index) {
 std::lock_guard guard(m_map_mutex);
-MapIterator iter = m_map.begin();
-MapIterator end = m_map.end();
-while (index > 0) {
-  iter++;
-  index--;
-  if (end == iter)
-return KeyType();
-}
-return iter->first;
+if (index >= m_map.size())
+  return {};
+return m_map[index].first;
   }
 
 protected:
@@ -171,8 +162,8 @@ protected:
 public:
   typedef typename BackEndType::MapType MapType;
   typedef typename MapType::iterator MapIterator;
-  typedef typename MapType::key_type MapKeyType;
-  typedef typename MapType::mapped_type MapValueType;
+  typedef KeyType MapKeyType;
+  typedef std::shared_ptr MapValueType;
   typedef typename BackEndType::ForEachCallback ForEachCallback;
   typedef typename std::shared_ptr>
   SharedPointer;
@@ -294,7 +285,8 @@ protected:
 RegularExpression *dummy) {
 llvm::StringRef key_str = key.GetStringRef();
 std::lock_guard guard(m_format_map.mutex());
-for (const auto &pos : m_format_map.map()) {
+// Patterns are matched in reverse-chronological order.
+for (const auto &pos : llvm::reverse(m_format_map.map())) {
   const RegularExpression ®ex = pos.first;
   if (regex.Execute(key_str)) {
 value = pos.second;

Modified: lldb/trunk/include/lldb/Utility/RegularExpression.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/RegularExpression.h?rev=372424&r1=372423&r2=372424&view=diff
==
--- lldb/trunk/include/lldb/Utility/RegularExpression.h (original)
+++ lldb/trunk/include/lldb/Utility/RegularExpression.h Fri Sep 20 1

[Lldb-commits] [PATCH] D67857: [LLDB] Include lldb/Host/windows/windows.h on any windows target

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: lanza, hhb, labath.
Herald added subscribers: JDevlieghere, abidh.
Herald added a project: LLDB.

This fixes MinGW builds, that otherwise fail due to use of undeclared 
`GetLastError()` and `ERROR_OPERATION_ABORTED`.

The use of `GetLastError()` was added in SVN r366520 (within an `#ifdef 
_WIN32`), while the existing include of `lldb/Host/windows/windows.h` was 
within `#ifdef _MSC_VER`. Change the include ifdef to `#ifdef _WIN32`.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67857

Files:
  lldb/source/Core/IOHandler.cpp


Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -52,7 +52,7 @@
 
 #include "llvm/ADT/StringRef.h"
 
-#ifdef _MSC_VER
+#ifdef _WIN32
 #include "lldb/Host/windows/windows.h"
 #endif
 


Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -52,7 +52,7 @@
 
 #include "llvm/ADT/StringRef.h"
 
-#ifdef _MSC_VER
+#ifdef _WIN32
 #include "lldb/Host/windows/windows.h"
 #endif
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D66654: 2/2: Process formatters in reverse-chronological order

[Jan Kratochvil via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL372424: [lldb] Process formatters in reverse-chronological 
order (authored by jankratochvil, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66654?vs=220219&id=221100#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66654

Files:
  lldb/trunk/include/lldb/DataFormatters/FormattersContainer.h
  lldb/trunk/include/lldb/Utility/RegularExpression.h
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py

Index: lldb/trunk/include/lldb/Utility/RegularExpression.h
===
--- lldb/trunk/include/lldb/Utility/RegularExpression.h
+++ lldb/trunk/include/lldb/Utility/RegularExpression.h
@@ -78,10 +78,6 @@
   /// otherwise.
   llvm::Error GetError() const;
 
-  bool operator<(const RegularExpression &rhs) const {
-return GetText() < rhs.GetText();
-  }
-
   bool operator==(const RegularExpression &rhs) const {
 return GetText() == rhs.GetText();
   }
Index: lldb/trunk/include/lldb/DataFormatters/FormattersContainer.h
===
--- lldb/trunk/include/lldb/DataFormatters/FormattersContainer.h
+++ lldb/trunk/include/lldb/DataFormatters/FormattersContainer.h
@@ -65,7 +65,7 @@
 template  class FormatMap {
 public:
   typedef typename ValueType::SharedPointer ValueSP;
-  typedef std::map MapType;
+  typedef std::vector> MapType;
   typedef typename MapType::iterator MapIterator;
   typedef std::function ForEachCallback;
 
@@ -79,20 +79,22 @@
   entry->GetRevision() = 0;
 
 std::lock_guard guard(m_map_mutex);
-m_map[std::move(name)] = entry;
+Delete(name);
+m_map.emplace_back(std::move(name), std::move(entry));
 if (listener)
   listener->Changed();
   }
 
   bool Delete(const KeyType &name) {
 std::lock_guard guard(m_map_mutex);
-MapIterator iter = m_map.find(name);
-if (iter == m_map.end())
-  return false;
-m_map.erase(iter);
-if (listener)
-  listener->Changed();
-return true;
+for (MapIterator iter = m_map.begin(); iter != m_map.end(); ++iter)
+  if (iter->first == name) {
+m_map.erase(iter);
+if (listener)
+  listener->Changed();
+return true;
+  }
+return false;
   }
 
   void Clear() {
@@ -104,11 +106,12 @@
 
   bool Get(const KeyType &name, ValueSP &entry) {
 std::lock_guard guard(m_map_mutex);
-MapIterator iter = m_map.find(name);
-if (iter == m_map.end())
-  return false;
-entry = iter->second;
-return true;
+for (const auto &pos : m_map)
+  if (pos.first == name) {
+entry = pos.second;
+return true;
+  }
+return false;
   }
 
   void ForEach(ForEachCallback callback) {
@@ -126,29 +129,17 @@
 
   ValueSP GetValueAtIndex(size_t index) {
 std::lock_guard guard(m_map_mutex);
-MapIterator iter = m_map.begin();
-MapIterator end = m_map.end();
-while (index > 0) {
-  iter++;
-  index--;
-  if (end == iter)
-return ValueSP();
-}
-return iter->second;
+if (index >= m_map.size())
+  return ValueSP();
+return m_map[index].second;
   }
 
   // If caller holds the mutex we could return a reference without copy ctor.
   KeyType GetKeyAtIndex(size_t index) {
 std::lock_guard guard(m_map_mutex);
-MapIterator iter = m_map.begin();
-MapIterator end = m_map.end();
-while (index > 0) {
-  iter++;
-  index--;
-  if (end == iter)
-return KeyType();
-}
-return iter->first;
+if (index >= m_map.size())
+  return {};
+return m_map[index].first;
   }
 
 protected:
@@ -171,8 +162,8 @@
 public:
   typedef typename BackEndType::MapType MapType;
   typedef typename MapType::iterator MapIterator;
-  typedef typename MapType::key_type MapKeyType;
-  typedef typename MapType::mapped_type MapValueType;
+  typedef KeyType MapKeyType;
+  typedef std::shared_ptr MapValueType;
   typedef typename BackEndType::ForEachCallback ForEachCallback;
   typedef typename std::shared_ptr>
   SharedPointer;
@@ -294,7 +285,8 @@
 RegularExpression *dummy) {
 llvm::StringRef key_str = key.GetStringRef();
 std::lock_guard guard(m_format_map.mutex());
-for (const auto &pos : m_format_map.map()) {
+// Patterns are matched in reverse-chronological order.
+for (const auto &pos : llvm::reverse(m_format_map.map())) {
   const RegularExpression ®ex = pos.first;
   if (regex.Execute(key_str)) {
 value = pos.second;
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-advanced/Tes

[Lldb-commits] [PATCH] D67856: [LLDB] Fix compilation for MinGW, remove redundant class name on inline member

[Davide Italiano via Phabricator via lldb-commits]

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

If this doesn't break clang and gcc, fine.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67856



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

[Lldb-commits] [PATCH] D67856: [LLDB] Fix compilation for MinGW, remove redundant class name on inline member

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D67856#1677356 , @davide wrote:

> If this doesn't break clang and gcc, fine.


FWIW, it was with clang that I ran into it, haven't tested building lldb with 
gcc.

But the issue can be tested with a snippet like this:

  class Foo {
  public: 
  Foo::Foo() {}
  };

With MSVC, this passes fine without any warnings. With clang-cl (or plain clang 
with an msvc target), it builds but warns (`-Wmicrosoft-extra-qualification`), 
with gcc it's an error (that can be waived with `-fpermissive`), and with clang 
a non-msvc target, it's an unrecoverable error.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67856



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

[Lldb-commits] [PATCH] D67858: [LLDB] Check for the GCC/MinGW compatible arch defines for windows, in addition to MSVC defines

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: compnerd, hhb, labath.
Herald added subscribers: JDevlieghere, abidh.
Herald added a project: LLDB.

This matches how it is done in all other similar ifdefs throughout lldb (modulo 
inconsistencies between `_M_AMD64` and `_M_X64`); both MSVC and GCC style arch 
defines are checked together.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67858

Files:
  lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp


Index: lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
@@ -21,9 +21,9 @@
 #include "TargetThreadWindows.h"
 
 // TODO support _M_ARM and _M_ARM64
-#if defined(_M_AMD64)
+#if defined(__x86_64__) || defined(_M_AMD64)
 #include "x64/RegisterContextWindows_x64.h"
-#elif defined(_M_IX86)
+#elif defined(__i386__) || defined(_M_IX86)
 #include "x86/RegisterContextWindows_x86.h"
 #endif
 
@@ -77,7 +77,7 @@
 break;
 
   case llvm::Triple::x86:
-#if defined(_M_IX86)
+#if defined(__i386__) || defined(_M_IX86)
 m_thread_reg_ctx_sp.reset(
 new RegisterContextWindows_x86(*this, concrete_frame_idx));
 #else
@@ -86,7 +86,7 @@
 break;
 
   case llvm::Triple::x86_64:
-#if defined(_M_AMD64)
+#if defined(__x86_64__) || defined(_M_AMD64)
 m_thread_reg_ctx_sp.reset(
 new RegisterContextWindows_x64(*this, concrete_frame_idx));
 #else
Index: lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
@@ -84,7 +84,7 @@
   case 1:
   case 2:
   case 4:
-#if defined(_M_AMD64)
+#if defined(__x86_64__) || defined(_M_AMD64)
   case 8:
 #endif
 break;


Index: lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
@@ -21,9 +21,9 @@
 #include "TargetThreadWindows.h"
 
 // TODO support _M_ARM and _M_ARM64
-#if defined(_M_AMD64)
+#if defined(__x86_64__) || defined(_M_AMD64)
 #include "x64/RegisterContextWindows_x64.h"
-#elif defined(_M_IX86)
+#elif defined(__i386__) || defined(_M_IX86)
 #include "x86/RegisterContextWindows_x86.h"
 #endif
 
@@ -77,7 +77,7 @@
 break;
 
   case llvm::Triple::x86:
-#if defined(_M_IX86)
+#if defined(__i386__) || defined(_M_IX86)
 m_thread_reg_ctx_sp.reset(
 new RegisterContextWindows_x86(*this, concrete_frame_idx));
 #else
@@ -86,7 +86,7 @@
 break;
 
   case llvm::Triple::x86_64:
-#if defined(_M_AMD64)
+#if defined(__x86_64__) || defined(_M_AMD64)
 m_thread_reg_ctx_sp.reset(
 new RegisterContextWindows_x64(*this, concrete_frame_idx));
 #else
Index: lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
@@ -84,7 +84,7 @@
   case 1:
   case 2:
   case 4:
-#if defined(_M_AMD64)
+#if defined(__x86_64__) || defined(_M_AMD64)
   case 8:
 #endif
 break;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67856: [LLDB] Fix compilation for MinGW, remove redundant class name on inline member

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Just keep an eye on the bots as they might start whining
http://green.lab.llvm.org/green/

otherwise, just go ahead.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67856



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

[Lldb-commits] [PATCH] D67859: [LLDB] Use the Windows SOCKET type on all windows targets, not only MSVC

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: hhb, labath, amccarth.
Herald added a subscriber: JDevlieghere.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67859

Files:
  lldb/include/lldb/Host/Socket.h


Index: lldb/include/lldb/Host/Socket.h
===
--- lldb/include/lldb/Host/Socket.h
+++ lldb/include/lldb/Host/Socket.h
@@ -31,7 +31,7 @@
 
 namespace lldb_private {
 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 typedef SOCKET NativeSocket;
 #else
 typedef int NativeSocket;


Index: lldb/include/lldb/Host/Socket.h
===
--- lldb/include/lldb/Host/Socket.h
+++ lldb/include/lldb/Host/Socket.h
@@ -31,7 +31,7 @@
 
 namespace lldb_private {
 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 typedef SOCKET NativeSocket;
 #else
 typedef int NativeSocket;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D66398: 2/2: Fix `TestDataFormatterStdList` regression

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

"jankratochvil added a reverting change" - that is not true, Differential got 
somehow confused by the commit text there talking about this patch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66398



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

[Lldb-commits] [PATCH] D67860: [LLDB] Use LLVM_FALLTHROUGH instead of a custom comment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: asmith, hhb, labath.
Herald added a subscriber: JDevlieghere.
Herald added a project: LLDB.

This fixes a warning when built with Clang instead of MSVC.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67860

Files:
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp


Index: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
@@ -479,7 +479,7 @@
   return ExceptionResult::BreakInDebugger;
 }
 
-// Fall through
+LLVM_FALLTHROUGH;
   default:
 LLDB_LOG(log,
  "Debugger thread reported exception {0:x} at address {1:x} "


Index: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
@@ -479,7 +479,7 @@
   return ExceptionResult::BreakInDebugger;
 }
 
-// Fall through
+LLVM_FALLTHROUGH;
   default:
 LLDB_LOG(log,
  "Debugger thread reported exception {0:x} at address {1:x} "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67861: [LLDB] Check for the __MINGW32__ define instead of __MINGW64

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: hhb, xen2, labath.
Herald added subscribers: JDevlieghere, abidh.
Herald added a project: LLDB.

`__MINGW64__` is only defined for 64 bit targets, and the define without 
trailing underscores is never defined automatically (neither by clang nor by 
gcc), while `__MINGW32__` is defined for any MinGW target, both 32 and 64 bit.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67861

Files:
  lldb/source/Host/windows/Windows.cpp


Index: lldb/source/Host/windows/Windows.cpp
===
--- lldb/source/Host/windows/Windows.cpp
+++ lldb/source/Host/windows/Windows.cpp
@@ -48,7 +48,7 @@
   size_t buflen;
   va_list ap2;
 
-#if defined(_MSC_VER) || defined(__MINGW64)
+#if defined(_MSC_VER) || defined(__MINGW32__)
   ap2 = ap;
   len = _vscprintf(fmt, ap2);
 #else


Index: lldb/source/Host/windows/Windows.cpp
===
--- lldb/source/Host/windows/Windows.cpp
+++ lldb/source/Host/windows/Windows.cpp
@@ -48,7 +48,7 @@
   size_t buflen;
   va_list ap2;
 
-#if defined(_MSC_VER) || defined(__MINGW64)
+#if defined(_MSC_VER) || defined(__MINGW32__)
   ap2 = ap;
   len = _vscprintf(fmt, ap2);
 #else
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67583: Fix swig python package path

[Hal Gentz via Phabricator via lldb-commits]

ZeGentzy added a comment.

In D67583#1676531 , @labath wrote:

> I could be off mark here, but I do recall some people trying to get this code 
> to work in environments where python and lldb(llvm) are configured with 
> different libdirs (so LLVM_LIBDIR_SUFFIX in llvm, and `configure --prefix=X` 
> in python). Is that the case you're running into here?


I think not. I'm cross compiling llvm, getting it to build a 32-bit llvm for my 
64-bit host, so I can install it along side my 64-bit llvm and use it with my 
32-bit mesa.

I'm building two pythons, python2s, llvms and mesas, one for 32-bits and one 
for 64-bits.

My 32-bit python2, the one in use by llvm for it's 32-bit builds, was 
configured with the following options:

  --prefix=/usr 
  --enable-shared
  --with-threads
  --enable-optimizations
  --with-lto
  --enable-ipv6
  --with-system-expat
  --with-dbmliborder=gdbm:ndbm
  --with-system-ffi
  --without-ensurepip
  --libdir=/usr/lib32
  --libexecdir=/usr/lib32
  --includedir="/usr/lib32/python${_pybasever}/include"
  --exec_prefix="/usr/lib32/python${_pybasever}/"
  --bindir=/usr/bin
  --sbindir=/usr/bin
  --with-suffix='-32'

Python:

  --prefix=/usr 
  --enable-shared
  --with-threads
  --with-computed-gotos
  --enable-optimizations
  --with-lto
  --enable-ipv6
  --with-system-expat
  --with-dbmliborder=gdbm:ndbm
  --with-system-ffi
  --with-system-libmpdec
  --enable-loadable-sqlite-extensions
  --without-ensurepip
  --libdir=/usr/lib32
  --libexecdir=/usr/lib32
  --includedir=/usr/lib32/python${_pybasever}m/include
  --exec_prefix=/usr/lib32/python${_pybasever}/
  --bindir=/usr/bin
  --sbindir=/usr/bin
  --with-suffix='-32'

This is done so my 32-bit python stuff does not conflict with the 64-bit python 
stuff.

---

Actually, thinking about it, I should have probably run your scripts with the 
32-bit pythons, as those are what llvm uses:

  [root@GENTZ-PC-NEW2 scripts]# python a.py
  (True, '/src/out/./lib/python3.7/site-packages/lldb', '')
  [root@GENTZ-PC-NEW2 scripts]# python-32 a.py
  (True, '/src/out/./lib32/python3.7/site-packages/lldb', '')
  [root@GENTZ-PC-NEW2 scripts]# python2-32 a.py
  (True, '/src/out/./lib32/python2.7/site-packages/lldb', '')
  [root@GENTZ-PC-NEW2 scripts]# python2 a.py
  (True, '/src/out/./lib/python2.7/site-packages/lldb', '')
  [root@GENTZ-PC-NEW2 scripts]# file /usr/lib[0-9]*/python*/ 
  /usr/lib32/python2.7/:  directory
  /usr/lib32/python3.7/:  directory
  /usr/lib32/python3.7m/: directory
  /usr/lib64/python2.7/:  directory
  /usr/lib64/python3.7/:  directory


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67583



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

[Lldb-commits] [PATCH] D67863: [LLDB] Cast -1 (as invalid socket) to the socket type before comparing

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: hhb, labath, compnerd.
Herald added subscribers: JDevlieghere, abidh.
Herald added a project: LLDB.

This silences warnings about comparison of integers between unsigned long long 
(which is what the Windows SOCKET type is) and signed int when building in 
MinGW mode.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67863

Files:
  lldb/source/Host/common/Socket.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -93,7 +93,7 @@
 } else {
   listen(sockfd, 5);
   socklen_t clilen = sizeof(cli_addr);
-  newsockfd = llvm::sys::RetryAfterSignal(-1, accept,
+  newsockfd = llvm::sys::RetryAfterSignal((SOCKET) -1, accept,
   sockfd, (struct sockaddr *)&cli_addr, &clilen);
   if (newsockfd < 0)
 if (g_vsc.log)
Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -476,10 +476,10 @@
   if (!child_processes_inherit) {
 flags |= SOCK_CLOEXEC;
   }
-  NativeSocket fd = llvm::sys::RetryAfterSignal(-1, ::accept4,
+  NativeSocket fd = llvm::sys::RetryAfterSignal((NativeSocket) -1, ::accept4,
   sockfd, addr, addrlen, flags);
 #else
-  NativeSocket fd = llvm::sys::RetryAfterSignal(-1, ::accept,
+  NativeSocket fd = llvm::sys::RetryAfterSignal((NativeSocket) -1, ::accept,
   sockfd, addr, addrlen);
 #endif
   if (fd == kInvalidSocketValue)


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -93,7 +93,7 @@
 } else {
   listen(sockfd, 5);
   socklen_t clilen = sizeof(cli_addr);
-  newsockfd = llvm::sys::RetryAfterSignal(-1, accept,
+  newsockfd = llvm::sys::RetryAfterSignal((SOCKET) -1, accept,
   sockfd, (struct sockaddr *)&cli_addr, &clilen);
   if (newsockfd < 0)
 if (g_vsc.log)
Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -476,10 +476,10 @@
   if (!child_processes_inherit) {
 flags |= SOCK_CLOEXEC;
   }
-  NativeSocket fd = llvm::sys::RetryAfterSignal(-1, ::accept4,
+  NativeSocket fd = llvm::sys::RetryAfterSignal((NativeSocket) -1, ::accept4,
   sockfd, addr, addrlen, flags);
 #else
-  NativeSocket fd = llvm::sys::RetryAfterSignal(-1, ::accept,
+  NativeSocket fd = llvm::sys::RetryAfterSignal((NativeSocket) -1, ::accept,
   sockfd, addr, addrlen);
 #endif
   if (fd == kInvalidSocketValue)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67862: [LLDB] Use SetErrorStringWithFormatv for cases that use LLVM style format strings

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: asmith, hhb, labath, compnerd.
Herald added a subscriber: JDevlieghere.
Herald added a project: LLDB.

SetErrorStringWithFormat only supports normal printf style format strings.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67862

Files:
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp


Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -170,9 +170,9 @@
 else
   LLDB_LOG(log, "Detaching process error: {0}", error);
   } else {
-error.SetErrorStringWithFormat("error: process {0} in state = {1}, but "
-   "cannot detach it in this state.",
-   GetID(), private_state);
+error.SetErrorStringWithFormatv("error: process {0} in state = {1}, but "
+"cannot detach it in this state.",
+GetID(), private_state);
 LLDB_LOG(log, "error: {0}", error);
   }
   return error;
Index: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
@@ -177,9 +177,9 @@
 else
   LLDB_LOG(log, "Detaching process error: {0}", error);
   } else {
-error.SetErrorStringWithFormat("error: process {0} in state = {1}, but "
-   "cannot detach it in this state.",
-   GetID(), state);
+error.SetErrorStringWithFormatv("error: process {0} in state = {1}, but "
+"cannot detach it in this state.",
+GetID(), state);
 LLDB_LOG(log, "error: {0}", error);
   }
   return error;


Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -170,9 +170,9 @@
 else
   LLDB_LOG(log, "Detaching process error: {0}", error);
   } else {
-error.SetErrorStringWithFormat("error: process {0} in state = {1}, but "
-   "cannot detach it in this state.",
-   GetID(), private_state);
+error.SetErrorStringWithFormatv("error: process {0} in state = {1}, but "
+"cannot detach it in this state.",
+GetID(), private_state);
 LLDB_LOG(log, "error: {0}", error);
   }
   return error;
Index: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
@@ -177,9 +177,9 @@
 else
   LLDB_LOG(log, "Detaching process error: {0}", error);
   } else {
-error.SetErrorStringWithFormat("error: process {0} in state = {1}, but "
-   "cannot detach it in this state.",
-   GetID(), state);
+error.SetErrorStringWithFormatv("error: process {0} in state = {1}, but "
+"cannot detach it in this state.",
+GetID(), state);
 LLDB_LOG(log, "error: {0}", error);
   }
   return error;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67793: new api class: SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 221113.
lawrence_danna added a comment.

added some tests, rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793

Files:
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBError.h
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Host/File.h
  lldb/include/lldb/lldb-forward.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/interface/SBFile.i
  lldb/scripts/lldb.swig
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBFile.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -951,7 +951,8 @@
 
 PythonFile::PythonFile(const char *path, const char *mode) {
   lldb_private::File file;
-  FileSystem::Instance().Open(file, FileSpec(path), GetOptionsFromMode(mode));
+  FileSystem::Instance().Open(file, FileSpec(path),
+  File::GetOptionsFromMode(mode));
   Reset(file, mode);
 }
 
@@ -1019,23 +1020,6 @@
 #endif
 }
 
-uint32_t PythonFile::GetOptionsFromMode(llvm::StringRef mode) {
-  if (mode.empty())
-return 0;
-
-  return llvm::StringSwitch(mode.str())
-  .Case("r", File::eOpenOptionRead)
-  .Case("w", File::eOpenOptionWrite)
-  .Case("a", File::eOpenOptionWrite | File::eOpenOptionAppend |
- File::eOpenOptionCanCreate)
-  .Case("r+", File::eOpenOptionRead | File::eOpenOptionWrite)
-  .Case("w+", File::eOpenOptionRead | File::eOpenOptionWrite |
-  File::eOpenOptionCanCreate | File::eOpenOptionTruncate)
-  .Case("a+", File::eOpenOptionRead | File::eOpenOptionWrite |
-  File::eOpenOptionAppend | File::eOpenOptionCanCreate)
-  .Default(0);
-}
-
 bool PythonFile::GetUnderlyingFile(File &file) const {
   if (!IsValid())
 return false;
@@ -1044,7 +1028,7 @@
   // We don't own the file descriptor returned by this function, make sure the
   // File object knows about that.
   PythonString py_mode = GetAttributeValue("mode").AsType();
-  auto options = PythonFile::GetOptionsFromMode(py_mode.GetString());
+  auto options = File::GetOptionsFromMode(py_mode.GetString());
   file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), options, false);
   return file.IsValid();
 }
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -68,6 +68,22 @@
   return nullptr;
 }
 
+uint32_t File::GetOptionsFromMode(llvm::StringRef mode) {
+  if (mode.empty())
+return 0;
+  return llvm::StringSwitch(mode.str())
+  .Case("r", File::eOpenOptionRead)
+  .Case("w", File::eOpenOptionWrite)
+  .Case("a", File::eOpenOptionWrite | File::eOpenOptionAppend |
+ File::eOpenOptionCanCreate)
+  .Case("r+", File::eOpenOptionRead | File::eOpenOptionWrite)
+  .Case("w+", File::eOpenOptionRead | File::eOpenOptionWrite |
+  File::eOpenOptionCanCreate | File::eOpenOptionTruncate)
+  .Case("a+", File::eOpenOptionRead | File::eOpenOptionWrite |
+  File::eOpenOptionAppend | File::eOpenOptionCanCreate)
+  .Default(0);
+}
+
 int File::kInvalidDescriptor = -1;
 FILE *File::kInvalidStream = nullptr;
 
@@ -158,9 +174,14 @@
 
 Status File::Close() {
   Status error;
-  if (StreamIsValid() && m_own_stream) {
-if (::fclose(m_stream) == EOF)
-  error.SetErrorToErrno();
+  if (StreamIsValid()) {
+if (m_own_stream) {
+  if (::fclose(m_stream) == EOF)
+error.SetErrorToErrno();
+} else {
+  if (::fflush(m_stream) == EOF)
+error.SetErrorToErrno();
+}
   }
 
   if (DescriptorIsValid() && m_should_close_fd) {
Index: lldb/source/API/SBFile.cpp
===
--- /dev/null
+++ lldb/source/API/SBFile.cpp
@@ -0,0 +1,76 @@
+//===-- SBFile.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/API/SBFile.h"
+#include "lldb/API/SBError.h"
+#include "lldb/Host/File.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+SBFile::~SBFile() {}
+
+SBFile::SBFile() {}
+
+void SBFile::SetStream(FILE *file, bool transfer_ownership) {
+  m_opaque_up = std::make_unique(file, trans

[Lldb-commits] [PATCH] D67863: [LLDB] Cast -1 (as invalid socket) to the socket type before comparing

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.



Comment at: lldb/source/Host/common/Socket.cpp:479
   }
-  NativeSocket fd = llvm::sys::RetryAfterSignal(-1, ::accept4,
+  NativeSocket fd = llvm::sys::RetryAfterSignal((NativeSocket) -1, ::accept4,
   sockfd, addr, addrlen, flags);

Could you use C++ style casts please?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67863



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

[Lldb-commits] [PATCH] D67866: [lldb] Unify python site-packages path

[Haibo Huang via Phabricator via lldb-commits]

hhb created this revision.
hhb added a reviewer: labath.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.

Before this change, the procedure of installing and loading python
modules are as follow:

1. finishSwigPythonLLDB.py writes necessary files to

${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/distutils.sysconfig.get_python_lib()

2. lldb/scripts/CMakeLists.txt installs them to lib${LLVM_LIBDIR_SUFFIX}

3. ScriptInterpreterPython.cpp adds the right path into PYTHONPATH.

(XCode is using a different path but that's not changed here.)

The issue is that get_python_lib() is platform dependent. It is
difficult to guess the right path in cmake. And it is impossible to be
right when cross compiling.

This change updates finishSwigPythonLLDB.py to output to ${liblldb_build_dir}.
Since cmake will always install them into lib${LLVM_LIBDIR_SUFFIX} , this change
Also updates ScriptInterpreterPython.cpp to load from
${liblldb_path}/../lib${LLVM_LIBDIR_SUFFIX}/site-packages.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67866

Files:
  lldb/CMakeLists.txt
  lldb/scripts/CMakeLists.txt
  lldb/scripts/Python/finishSwigPythonLLDB.py
  lldb/scripts/finishSwigWrapperClasses.py
  lldb/scripts/get_relative_lib_dir.py
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -48,8 +48,7 @@
 
 protected:
   static void ComputePythonDirForApple(llvm::SmallVectorImpl &path);
-  static void ComputePythonDirForPosix(llvm::SmallVectorImpl &path);
-  static void ComputePythonDirForWindows(llvm::SmallVectorImpl &path);
+  static void ComputePythonDir(llvm::SmallVectorImpl &path);
 };
 } // namespace lldb_private
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -28,6 +28,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/DataFormatters/TypeSummary.h"
+#include "lldb/Host/Config.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
@@ -302,35 +303,22 @@
   auto rend = llvm::sys::path::rend(path_ref);
   auto framework = std::find(rbegin, rend, "LLDB.framework");
   if (framework == rend) {
-ComputePythonDirForPosix(path);
+ComputePythonDir(path);
 return;
   }
   path.resize(framework - rend);
   llvm::sys::path::append(path, style, "LLDB.framework", "Resources", "Python");
 }
 
-void ScriptInterpreterPython::ComputePythonDirForPosix(
+void ScriptInterpreterPython::ComputePythonDir(
 llvm::SmallVectorImpl &path) {
-  auto style = llvm::sys::path::Style::posix;
-#if defined(LLDB_PYTHON_RELATIVE_LIBDIR)
-  // Build the path by backing out of the lib dir, then building with whatever
-  // the real python interpreter uses.  (e.g. lib for most, lib64 on RHEL
-  // x86_64).
-  llvm::sys::path::remove_filename(path, style);
-  llvm::sys::path::append(path, style, LLDB_PYTHON_RELATIVE_LIBDIR);
+#if defined(_WIN32)
+auto style = llvm::sys::path::Style::windows;
 #else
-  llvm::sys::path::append(path, style,
-  "python" + llvm::Twine(PY_MAJOR_VERSION) + "." +
-  llvm::Twine(PY_MINOR_VERSION),
-  "site-packages");
+auto style = llvm::sys::path::Style::posix;
 #endif
-}
-
-void ScriptInterpreterPython::ComputePythonDirForWindows(
-llvm::SmallVectorImpl &path) {
-  auto style = llvm::sys::path::Style::windows;
   llvm::sys::path::remove_filename(path, style);
-  llvm::sys::path::append(path, style, "lib", "site-packages");
+  llvm::sys::path::append(path, style, "lib" LLDB_LIBDIR_SUFFIX, "site-packages");
 
   // This will be injected directly through FileSpec.GetDirectory().SetString(),
   // so we need to normalize manually.
@@ -347,10 +335,8 @@
 
 #if defined(__APPLE__)
 ComputePythonDirForApple(path);
-#elif defined(_WIN32)
-ComputePythonDirForWindows(path);
 #else
-ComputePythonDirForPosix(path);
+ComputePythonDir(path);
 #endif
 spec.GetDirectory().SetString(path);
 return spec;
Index: lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
===
--- lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
+++ lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
@@ -1,16 +1,3 @@
-if (NOT CMAKE

[Lldb-commits] [PATCH] D67863: [LLDB] Cast -1 (as invalid socket) to the socket type before comparing

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo updated this revision to Diff 221122.
mstorsjo added a comment.

Use static_cast.


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

https://reviews.llvm.org/D67863

Files:
  lldb/source/Host/common/Socket.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -93,8 +93,9 @@
 } else {
   listen(sockfd, 5);
   socklen_t clilen = sizeof(cli_addr);
-  newsockfd = llvm::sys::RetryAfterSignal(-1, accept,
-  sockfd, (struct sockaddr *)&cli_addr, &clilen);
+  newsockfd =
+  llvm::sys::RetryAfterSignal(static_cast(-1), accept, sockfd,
+  (struct sockaddr *)&cli_addr, &clilen);
   if (newsockfd < 0)
 if (g_vsc.log)
   *g_vsc.log << "error: accept (" << strerror(errno) << ")"
Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -476,11 +476,11 @@
   if (!child_processes_inherit) {
 flags |= SOCK_CLOEXEC;
   }
-  NativeSocket fd = llvm::sys::RetryAfterSignal(-1, ::accept4,
-  sockfd, addr, addrlen, flags);
+  NativeSocket fd = llvm::sys::RetryAfterSignal(
+  static_cast(-1), ::accept4, sockfd, addr, addrlen, flags);
 #else
-  NativeSocket fd = llvm::sys::RetryAfterSignal(-1, ::accept,
-  sockfd, addr, addrlen);
+  NativeSocket fd = llvm::sys::RetryAfterSignal(
+  static_cast(-1), ::accept, sockfd, addr, addrlen);
 #endif
   if (fd == kInvalidSocketValue)
 SetLastError(error);


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -93,8 +93,9 @@
 } else {
   listen(sockfd, 5);
   socklen_t clilen = sizeof(cli_addr);
-  newsockfd = llvm::sys::RetryAfterSignal(-1, accept,
-  sockfd, (struct sockaddr *)&cli_addr, &clilen);
+  newsockfd =
+  llvm::sys::RetryAfterSignal(static_cast(-1), accept, sockfd,
+  (struct sockaddr *)&cli_addr, &clilen);
   if (newsockfd < 0)
 if (g_vsc.log)
   *g_vsc.log << "error: accept (" << strerror(errno) << ")"
Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -476,11 +476,11 @@
   if (!child_processes_inherit) {
 flags |= SOCK_CLOEXEC;
   }
-  NativeSocket fd = llvm::sys::RetryAfterSignal(-1, ::accept4,
-  sockfd, addr, addrlen, flags);
+  NativeSocket fd = llvm::sys::RetryAfterSignal(
+  static_cast(-1), ::accept4, sockfd, addr, addrlen, flags);
 #else
-  NativeSocket fd = llvm::sys::RetryAfterSignal(-1, ::accept,
-  sockfd, addr, addrlen);
+  NativeSocket fd = llvm::sys::RetryAfterSignal(
+  static_cast(-1), ::accept, sockfd, addr, addrlen);
 #endif
   if (fd == kInvalidSocketValue)
 SetLastError(error);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67866: [lldb] Unify python site-packages path

[Haibo Huang via Phabricator via lldb-commits]

hhb updated this revision to Diff 221123.
hhb added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67866

Files:
  lldb/CMakeLists.txt
  lldb/scripts/CMakeLists.txt
  lldb/scripts/Python/finishSwigPythonLLDB.py
  lldb/scripts/finishSwigWrapperClasses.py
  lldb/scripts/get_relative_lib_dir.py
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -48,8 +48,7 @@
 
 protected:
   static void ComputePythonDirForApple(llvm::SmallVectorImpl &path);
-  static void ComputePythonDirForPosix(llvm::SmallVectorImpl &path);
-  static void ComputePythonDirForWindows(llvm::SmallVectorImpl &path);
+  static void ComputePythonDir(llvm::SmallVectorImpl &path);
 };
 } // namespace lldb_private
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -28,6 +28,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/DataFormatters/TypeSummary.h"
+#include "lldb/Host/Config.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
@@ -302,35 +303,22 @@
   auto rend = llvm::sys::path::rend(path_ref);
   auto framework = std::find(rbegin, rend, "LLDB.framework");
   if (framework == rend) {
-ComputePythonDirForPosix(path);
+ComputePythonDir(path);
 return;
   }
   path.resize(framework - rend);
   llvm::sys::path::append(path, style, "LLDB.framework", "Resources", "Python");
 }
 
-void ScriptInterpreterPython::ComputePythonDirForPosix(
+void ScriptInterpreterPython::ComputePythonDir(
 llvm::SmallVectorImpl &path) {
-  auto style = llvm::sys::path::Style::posix;
-#if defined(LLDB_PYTHON_RELATIVE_LIBDIR)
-  // Build the path by backing out of the lib dir, then building with whatever
-  // the real python interpreter uses.  (e.g. lib for most, lib64 on RHEL
-  // x86_64).
-  llvm::sys::path::remove_filename(path, style);
-  llvm::sys::path::append(path, style, LLDB_PYTHON_RELATIVE_LIBDIR);
+#if defined(_WIN32)
+auto style = llvm::sys::path::Style::windows;
 #else
-  llvm::sys::path::append(path, style,
-  "python" + llvm::Twine(PY_MAJOR_VERSION) + "." +
-  llvm::Twine(PY_MINOR_VERSION),
-  "site-packages");
+auto style = llvm::sys::path::Style::posix;
 #endif
-}
-
-void ScriptInterpreterPython::ComputePythonDirForWindows(
-llvm::SmallVectorImpl &path) {
-  auto style = llvm::sys::path::Style::windows;
   llvm::sys::path::remove_filename(path, style);
-  llvm::sys::path::append(path, style, "lib", "site-packages");
+  llvm::sys::path::append(path, style, "lib" LLDB_LIBDIR_SUFFIX, "site-packages");
 
   // This will be injected directly through FileSpec.GetDirectory().SetString(),
   // so we need to normalize manually.
@@ -347,10 +335,8 @@
 
 #if defined(__APPLE__)
 ComputePythonDirForApple(path);
-#elif defined(_WIN32)
-ComputePythonDirForWindows(path);
 #else
-ComputePythonDirForPosix(path);
+ComputePythonDir(path);
 #endif
 spec.GetDirectory().SetString(path);
 return spec;
Index: lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
===
--- lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
+++ lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
@@ -1,16 +1,3 @@
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
-  # Call a python script to gather the arch-specific libdir for
-  # modules like the lldb module.
-  execute_process(
-COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/../../../../scripts/get_relative_lib_dir.py
-RESULT_VARIABLE get_libdir_status
-OUTPUT_VARIABLE relative_libdir
-)
-  if (get_libdir_status EQUAL 0)
-add_definitions(-DLLDB_PYTHON_RELATIVE_LIBDIR="${relative_libdir}")
-  endif()
-endif()
-
 add_lldb_library(lldbPluginScriptInterpreterPython PLUGIN
   PythonDataObjects.cpp
   PythonExceptionState.cpp
Index: lldb/scripts/get_relative_lib_dir.py
===
--- lldb/scripts/get_relative_lib_dir.py
+++ /dev/null
@@ -1,44 +0,0 @@
-import distutils.sysconfig
-import os
-import platform
-import re
-import sys
-
-
-def get_python_r

[Lldb-commits] [PATCH] D67774: [Mangle] Add flag to asm labels to disable global prefixing

[John McCall via Phabricator via lldb-commits]

rjmccall added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:725
   let Spellings = [Keyword<"asm">, Keyword<"__asm__">];
-  let Args = [StringArgument<"Label">];
+  let Args = [StringArgument<"Label">, BoolArgument<"LiteralLabel">];
   let SemaHandler = 0;

Does this actually change how it can be written in source?  I think it might be 
a good idea to do that in the long run, but let's not in this patch.

Also something really needs to document what this is actually for.



Comment at: clang/lib/AST/Mangle.cpp:137
+(!Source || Source->UseGlobalPrefixInAsmLabelMangle()))
   Out << '\01'; // LLVM IR Marker for __asm("foo")
 

rjmccall wrote:
> This is one of those bugs where looking at the code really reveals a lot of 
> problematic behavior.
> 
> `AsmLabelAttr` is generally assumed to carry a literal label, i.e. it should 
> not have the global prefix added to it.  We should not be changing that 
> assumption globally just based on whether an `ExternalASTSource` has been set 
> up; that's going to break in any sort of non-uniform situation, e.g. if an 
> LLDB user writes a declaration with an asm label, or if we import a Clang 
> module where a declaration has an asm label.  Either `ExternalASTSource`s 
> should be putting the real (prefixed) symbol name in the `AsmLabelAttr`, or 
> they should be making `AsmLabelAttr`s that are different somehow, e.g. by 
> giving `AsmLabelAttr` a flag (which doesn't need to be spellable in the 
> source language) saying whether the label is literal or subject to global 
> prefixing.
> 
> Separately, it seems to me that if the `AsmLabelAttr` is literal but the 
> label happens to include the global prefix, we should strip the prefix from 
> the label and not add a `\01` so that we get a consistent symbol name in LLVM.
Please check for whether the label is literal first before pulling out the 
global prefix.



Comment at: clang/lib/Sema/SemaDecl.cpp:2770
+  assert(OldA->getLiteralLabel() == NewA->getLiteralLabel() &&
+ "Redeclaration of asm label changes label kind");
   if (OldA->getLabel() != NewA->getLabel()) {

This is definitely not a safe assumption; for one, we might eventually allow 
this to be spelled in source, and for another, you can redeclare something from 
the global context in LLDB and therefore get this kind of mismatch.  Anyway, I 
don't think you need the assumption here; just add a method to `AsmLabelAttr` 
that checks for equality in a way that compensates for the 
inclusion/suppression of the global prefix.


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

https://reviews.llvm.org/D67774



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

[Lldb-commits] [lldb] r372441 - dotest.py: bugfix: test filters with -f do not work on Python3

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Fri Sep 20 16:41:29 2019
New Revision: 372441

URL: http://llvm.org/viewvc/llvm-project?rev=372441&view=rev
Log:
dotest.py: bugfix: test filters with -f do not work on Python3

dotest -f does not work on Python3.

The name types.UnboundMethodType was an alias for types.MethodType in
2.7, but it does not exist in python3. MethodType works in both.

Also the actual type returned from SomeClass.some_method in python3
will be types.Function, not MethodType.

Patch by: Lawrence D'Anna

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

Modified:
lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py

Modified: lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py?rev=372441&r1=372440&r2=372441&view=diff
==
--- lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py 
(original)
+++ lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py Fri Sep 
20 16:41:29 2019
@@ -117,7 +117,7 @@ class TestLoader(unittest.TestLoader):
 return self.loadTestsFromModule(obj)
 elif isinstance(obj, type) and issubclass(obj, unittest.TestCase):
 return self.loadTestsFromTestCase(obj)
-elif (isinstance(obj, types.UnboundMethodType) and
+elif (isinstance(obj, (types.MethodType, types.FunctionType)) and
   isinstance(parent, type) and
   issubclass(parent, case.TestCase)):
 return self.suiteClass([parent(obj.__name__)])


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

[Lldb-commits] [lldb] r372442 - prepare_binding_Python: print readable errors if SWIG fails

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Fri Sep 20 16:41:32 2019
New Revision: 372442

URL: http://llvm.org/viewvc/llvm-project?rev=372442&view=rev
Log:
prepare_binding_Python: print readable errors if SWIG fails

When swig fails, all the errors are squished onto one line with \n
quoting. It's very hard to read. This will print them out in a more
reasonable format.

Patch by: Lawrence D'Anna

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

Modified:
lldb/trunk/scripts/Python/prepare_binding_Python.py

Modified: lldb/trunk/scripts/Python/prepare_binding_Python.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/prepare_binding_Python.py?rev=372442&r1=372441&r2=372442&view=diff
==
--- lldb/trunk/scripts/Python/prepare_binding_Python.py (original)
+++ lldb/trunk/scripts/Python/prepare_binding_Python.py Fri Sep 20 16:41:32 2019
@@ -231,11 +231,13 @@ def do_swig_rebuild(options, dependency_
 swig_stdout, swig_stderr = process.communicate()
 return_code = process.returncode
 if return_code != 0:
+swig_stdout = swig_stdout.decode('utf8', errors='replace').rstrip()
+swig_stderr = swig_stderr.decode('utf8', errors='replace').rstrip()
+swig_stdout = re.sub(r'^(?=.)', 'stdout: ', swig_stdout, 
flags=re.MULTILINE)
+swig_stderr = re.sub(r'^(?=.)', 'stderr: ', swig_stderr, 
flags=re.MULTILINE)
 logging.error(
-"swig failed with error code %d: stdout=%s, stderr=%s",
-return_code,
-swig_stdout,
-swig_stderr)
+"swig failed with error code %d\n%s%s",
+return_code, swig_stdout, swig_stderr)
 logging.error(
 "command line:\n%s", ' '.join(command))
 sys.exit(return_code)


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

[Lldb-commits] [PATCH] D67869: [ABISysV] Fix regression for Simulator and MacABI

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, xiaobai.
Herald added a subscriber: abidh.
Herald added a project: LLDB.

The ABISysV ABI was refactored in r364216 to support the Windows ABI for 
x86_64. In particular it changed `ABISysV_x86_64::CreateInstance` to switch on 
the OS type. This breaks debugging MacABI apps as well as apps in the 
simulator. This adds back the necessary cases.

We have a test internally that exercises this code path but some MacABI parts 
are still missing upstream.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67869

Files:
  lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp


Index: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
===
--- lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
+++ lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
@@ -222,16 +222,28 @@
 ABISysV_x86_64::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec 
&arch) {
   const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
   const llvm::Triple::OSType os_type = arch.GetTriple().getOS();
+  const llvm::Triple::EnvironmentType os_env = 
arch.GetTriple().getEnvironment();
   if (arch_type == llvm::Triple::x86_64) {
 switch(os_type) {
-  case llvm::Triple::OSType::MacOSX:
-  case llvm::Triple::OSType::Linux:
+  case llvm::Triple::OSType::IOS:
+  case llvm::Triple::OSType::TvOS:
+  case llvm::Triple::OSType::WatchOS:
+switch(os_env) {
+  case llvm::Triple::EnvironmentType::Simulator:
+  case llvm::Triple::EnvironmentType::UnknownEnvironment:
+  case llvm::Triple::EnvironmentType::MacABI:
+return ABISP(new ABISysV_x86_64(process_sp));
+  default:
+return ABISP();
+}
   case llvm::Triple::OSType::FreeBSD:
+  case llvm::Triple::OSType::Linux:
+  case llvm::Triple::OSType::MacOSX:
   case llvm::Triple::OSType::NetBSD:
   case llvm::Triple::OSType::Solaris:
   case llvm::Triple::OSType::UnknownOS:
 return ABISP(new ABISysV_x86_64(process_sp));
-  default: 
+  default:
 return ABISP();
 }
   }


Index: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
===
--- lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
+++ lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
@@ -222,16 +222,28 @@
 ABISysV_x86_64::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
   const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
   const llvm::Triple::OSType os_type = arch.GetTriple().getOS();
+  const llvm::Triple::EnvironmentType os_env = arch.GetTriple().getEnvironment();
   if (arch_type == llvm::Triple::x86_64) {
 switch(os_type) {
-  case llvm::Triple::OSType::MacOSX:
-  case llvm::Triple::OSType::Linux:
+  case llvm::Triple::OSType::IOS:
+  case llvm::Triple::OSType::TvOS:
+  case llvm::Triple::OSType::WatchOS:
+switch(os_env) {
+  case llvm::Triple::EnvironmentType::Simulator:
+  case llvm::Triple::EnvironmentType::UnknownEnvironment:
+  case llvm::Triple::EnvironmentType::MacABI:
+return ABISP(new ABISysV_x86_64(process_sp));
+  default:
+return ABISP();
+}
   case llvm::Triple::OSType::FreeBSD:
+  case llvm::Triple::OSType::Linux:
+  case llvm::Triple::OSType::MacOSX:
   case llvm::Triple::OSType::NetBSD:
   case llvm::Triple::OSType::Solaris:
   case llvm::Triple::OSType::UnknownOS:
 return ABISP(new ABISysV_x86_64(process_sp));
-  default: 
+  default:
 return ABISP();
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67790: prepare_binding_Python: print readable errors if SWIG fails

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372442: prepare_binding_Python: print readable errors if 
SWIG fails (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67790?vs=220940&id=221136#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67790

Files:
  lldb/trunk/scripts/Python/prepare_binding_Python.py


Index: lldb/trunk/scripts/Python/prepare_binding_Python.py
===
--- lldb/trunk/scripts/Python/prepare_binding_Python.py
+++ lldb/trunk/scripts/Python/prepare_binding_Python.py
@@ -231,11 +231,13 @@
 swig_stdout, swig_stderr = process.communicate()
 return_code = process.returncode
 if return_code != 0:
+swig_stdout = swig_stdout.decode('utf8', errors='replace').rstrip()
+swig_stderr = swig_stderr.decode('utf8', errors='replace').rstrip()
+swig_stdout = re.sub(r'^(?=.)', 'stdout: ', swig_stdout, 
flags=re.MULTILINE)
+swig_stderr = re.sub(r'^(?=.)', 'stderr: ', swig_stderr, 
flags=re.MULTILINE)
 logging.error(
-"swig failed with error code %d: stdout=%s, stderr=%s",
-return_code,
-swig_stdout,
-swig_stderr)
+"swig failed with error code %d\n%s%s",
+return_code, swig_stdout, swig_stderr)
 logging.error(
 "command line:\n%s", ' '.join(command))
 sys.exit(return_code)


Index: lldb/trunk/scripts/Python/prepare_binding_Python.py
===
--- lldb/trunk/scripts/Python/prepare_binding_Python.py
+++ lldb/trunk/scripts/Python/prepare_binding_Python.py
@@ -231,11 +231,13 @@
 swig_stdout, swig_stderr = process.communicate()
 return_code = process.returncode
 if return_code != 0:
+swig_stdout = swig_stdout.decode('utf8', errors='replace').rstrip()
+swig_stderr = swig_stderr.decode('utf8', errors='replace').rstrip()
+swig_stdout = re.sub(r'^(?=.)', 'stdout: ', swig_stdout, flags=re.MULTILINE)
+swig_stderr = re.sub(r'^(?=.)', 'stderr: ', swig_stderr, flags=re.MULTILINE)
 logging.error(
-"swig failed with error code %d: stdout=%s, stderr=%s",
-return_code,
-swig_stdout,
-swig_stderr)
+"swig failed with error code %d\n%s%s",
+return_code, swig_stdout, swig_stderr)
 logging.error(
 "command line:\n%s", ' '.join(command))
 sys.exit(return_code)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67791: dotest.py: bugfix: test filters with -f do not work on Python3

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372441: dotest.py: bugfix: test filters with -f do not work 
on Python3 (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67791?vs=220941&id=221135#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67791

Files:
  lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py


Index: lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py
===
--- lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py
+++ lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py
@@ -117,7 +117,7 @@
 return self.loadTestsFromModule(obj)
 elif isinstance(obj, type) and issubclass(obj, unittest.TestCase):
 return self.loadTestsFromTestCase(obj)
-elif (isinstance(obj, types.UnboundMethodType) and
+elif (isinstance(obj, (types.MethodType, types.FunctionType)) and
   isinstance(parent, type) and
   issubclass(parent, case.TestCase)):
 return self.suiteClass([parent(obj.__name__)])


Index: lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py
===
--- lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py
+++ lldb/trunk/third_party/Python/module/unittest2/unittest2/loader.py
@@ -117,7 +117,7 @@
 return self.loadTestsFromModule(obj)
 elif isinstance(obj, type) and issubclass(obj, unittest.TestCase):
 return self.loadTestsFromTestCase(obj)
-elif (isinstance(obj, types.UnboundMethodType) and
+elif (isinstance(obj, (types.MethodType, types.FunctionType)) and
   isinstance(parent, type) and
   issubclass(parent, case.TestCase)):
 return self.suiteClass([parent(obj.__name__)])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Multithread support

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

mgorny added a comment.

Nevermind that long report, it actually turned out to be a bug in NetBSD kernel 
that resulted in the thread not being resumed.


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

https://reviews.llvm.org/D64647



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